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.

2 files changed, 50 insertions, 27 deletions

diff --git a/src/libnm-platform/nm-netlink.c b/src/libnm-platform/nm-netlink.c
index fc70422642..de042f8e90 100644
--- a/src/libnm-platform/nm-netlink.c
+++ b/src/libnm-platform/nm-netlink.c
@@ -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;
-
-    /* - 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. */
-
-    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;
-    }
+    size_t      cpylen;
+
+    /* 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).
+     */
 
-    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
diff --git a/src/libnm-platform/nm-netlink.h b/src/libnm-platform/nm-netlink.h
index 2919d432d8..117525f944 100644
--- a/src/libnm-platform/nm-netlink.h
+++ b/src/libnm-platform/nm-netlink.h
@@ -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);