platform/netlink: cleanup nla_strlcpy() to not wipe remaining buffer

strlcpy()/g_strlcpy() has a well understood behavior. nla_strlcpy()
did not behave like that. Instead, it also used to always wipe the
remainder of the string, similar to what strncpy() would do.

True, if we do

  nla_strlcpy(obj->link.name, tb[IFLA_IFNAME], IFNAMSIZ);

then we might want to clear the remainder and don't care about the
overhead of writing up to 14 bytes unnecessarily... However, actually
all callers of nla_strlcpy() either operate on a buffer that is already
pre-inialized with zero, or they really don't care about the
uninitialized memory after the string. So this was nowhere the desired
behavior.

Change nla_strlcpy() to not wipe the remainder of the buffer, so it behaves
mostly like strlcpy()/g_strlcpy() and as one would expect.

Add nla_strlcpy_wipe(), which on top of it also clears the remaining
buffer. In that aspect, it bears some similarities with strncpy(), but it
differs in other regards from strncpy (always NUL terminating and
returning the srclen). Yes, the name nla_strlcpy_wipe() is maybe
unfamiliar to the user, but it really is like nla_strlcpy() with the
addition to clear the buffer. That seems simple enough to understand
based on the name.

Note that all existing callers of nla_strlcpy() do not care about
clearing the memory, and the change in behavior is fine for them.
This commit is contained in:
Thomas Haller 2023-02-17 11:04:32 +01:00
parent d73a5d692b
commit 6f854ecaeb
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728
2 changed files with 49 additions and 26 deletions

View file

@ -488,44 +488,52 @@ nlmsg_put(struct nl_msg *n,
}
size_t
nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
_nla_strlcpy_full(char *dst, const struct nlattr *nla, size_t dstsize, gboolean wipe_remainder)
{
const char *src;
const char *src = NULL;
size_t srclen;
size_t len;
size_t cpylen;
/* - Always writes @dstsize bytes to @dst
* - Copies the first non-NUL characters to @dst.
* Any characters after the first NUL bytes in @nla are ignored.
* - If the string @nla is longer than @dstsize, the string
* gets truncated. @dst will always be NUL terminated. */
/* Behaves like strlcpy():
*
* - returns the length of the string in nla (how much it wanted to copy).
* - will always NUL terminate dst (unless dstsize is zero).
* - if @wipe_remainder, the remaining bytes after the string are set to NUL,
* similar to what strncpy() would do. Otherwise the bytes are undefined.
* - nla is not required to contain a NUL terminated string (unlike nla_get_string()).
* - the function copies the bytes up to the first NUL character in nla.
* any remainder in nla is ignored.
* - nla may be NULL, which is treated the same as an empty string (copying zero bytes).
*/
if (G_UNLIKELY(dstsize <= 1)) {
if (dstsize == 1)
dst[0] = '\0';
if (nla && (srclen = nla_len(nla)) > 0)
return strnlen(nla_data(nla), srclen);
return 0;
}
nm_assert(dst);
nm_assert(dstsize == 0 || dst);
if (nla) {
srclen = nla_len(nla);
if (srclen > 0) {
src = nla_data(nla);
srclen = strnlen(src, srclen);
if (srclen > 0) {
len = NM_MIN(dstsize - 1, srclen);
memcpy(dst, src, len);
memset(&dst[len], 0, dstsize - len);
return srclen;
}
}
} else
srclen = 0;
if (dstsize == 0) {
/* we cannot NUL terminate. This is potentially dangerous, maybe
* we should assert against this case. */
return srclen;
}
memset(dst, 0, dstsize);
return 0;
cpylen = NM_MIN(dstsize - 1u, srclen);
nm_memcpy(dst, src, cpylen);
if (wipe_remainder) {
/* like strncpy() would do, wipe the rest. */
memset(&dst[cpylen], 0, dstsize - cpylen);
} else
dst[cpylen] = '\0';
return srclen;
}
size_t

View file

@ -252,7 +252,22 @@ nla_get_string(const struct nlattr *nla)
return s;
}
size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize);
size_t
_nla_strlcpy_full(char *dst, const struct nlattr *nla, size_t dstsize, gboolean wipe_remainder);
static inline size_t
nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
{
return _nla_strlcpy_full(dst, nla, dstsize, FALSE);
}
static inline size_t
nla_strlcpy_wipe(char *dst, const struct nlattr *nla, size_t dstsize)
{
/* Behaves exactly like nla_strlcpy(), but (similar to strncpy()) it fills the
* remaining @dstsize bytes with NUL. */
return _nla_strlcpy_full(dst, nla, dstsize, TRUE);
}
size_t nla_memcpy(void *dst, const struct nlattr *nla, size_t dstsize);