Message ID | 20201020164707.30402-2-laniel_francis@privacyrequired.com |
---|---|
State | New |
Headers | show |
Series | Fix inefficiences and rename nla_strlcpy | expand |
On Tue, Oct 20, 2020 at 06:47:05PM +0200, laniel_francis@privacyrequired.com wrote: > From: Francis Laniel <laniel_francis@privacyrequired.com> > > Before this commit, nla_strlcpy first memseted dst to 0 then wrote src into it. > This is inefficient because bytes whom number is less than src length are written > twice. > > This patch solves this issue by first writing src into dst then fill dst with > 0's. > Note that, in the case where src length is higher than dst, only 0 is written. > Otherwise there are as many 0's written to fill dst. > > For example, if src is "foo\0" and dst is 5 bytes long, the result will be: > 1. "fooGG" after memcpy (G means garbage). > 2. "foo\0\0" after memset. > > Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com> Looks good! (If there are future versions of this series, I think you can drop the RFC part...) Reviewed-by: Kees Cook <keescook@chromium.org> -- Kees Cook
diff --git a/lib/nlattr.c b/lib/nlattr.c index 74019c8ebf6b..07156e581997 100644 --- a/lib/nlattr.c +++ b/lib/nlattr.c @@ -731,8 +731,9 @@ size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize) if (dstsize > 0) { size_t len = (srclen >= dstsize) ? dstsize - 1 : srclen; - memset(dst, 0, dstsize); memcpy(dst, src, len); + /* Zero pad end of dst. */ + memset(dst + len, 0, dstsize - len); } return srclen;