string.h: Introduce memtostr() and memtostr_pad()

Another ambiguous use of strncpy() is to copy from strings that may not
be NUL-terminated. These cases depend on having the destination buffer
be explicitly larger than the source buffer's maximum size, having
the size of the copy exactly match the source buffer's maximum size,
and for the destination buffer to get explicitly NUL terminated.

This usually happens when parsing protocols or hardware character arrays
that are not guaranteed to be NUL-terminated. The code pattern is
effectively this:

	char dest[sizeof(src) + 1];

	strncpy(dest, src, sizeof(src));
	dest[sizeof(dest) - 1] = '\0';

In practice it usually looks like:

struct from_hardware {
	...
	char name[HW_NAME_SIZE] __nonstring;
	...
};

	struct from_hardware *p = ...;
	char name[HW_NAME_SIZE + 1];

	strncpy(name, p->name, HW_NAME_SIZE);
	name[NW_NAME_SIZE] = '\0';

This cannot be replaced with:

	strscpy(name, p->name, sizeof(name));

because p->name is smaller and not NUL-terminated, so FORTIFY will
trigger when strnlen(p->name, sizeof(name)) is used. And it cannot be
replaced with:

	strscpy(name, p->name, sizeof(p->name));

because then "name" may contain a 1 character early truncation of
p->name.

Provide an unambiguous interface for converting a maybe not-NUL-terminated
string to a NUL-terminated string, with compile-time buffer size checking
so that it can never fail at runtime: memtostr() and memtostr_pad(). Also
add KUnit tests for both.

Link: https://lore.kernel.org/r/20240410023155.2100422-1-keescook@chromium.org
Signed-off-by: Kees Cook <keescook@chromium.org>

diff --git a/include/linux/string.h b/include/linux/string.h
index 9ba8b4597009c961a7895742d6e8299b142434a7..86aa6cd351675c2349a2ef37b95f35ab6757bf1d 100644 (file)
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -422,6 +422,55 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count,
        memcpy(dest, src, strnlen(src, min(_src_len, _dest_len)));      \
 } while (0)
 
+/**
+ * memtostr - Copy a possibly non-NUL-term string to a NUL-term string
+ * @dest: Pointer to destination NUL-terminates string
+ * @src: Pointer to character array (likely marked as __nonstring)
+ *
+ * This is a replacement for strncpy() uses where the source is not
+ * a NUL-terminated string.
+ *
+ * Note that sizes of @dest and @src must be known at compile-time.
+ */
+#define memtostr(dest, src)    do {                                    \
+       const size_t _dest_len = __builtin_object_size(dest, 1);        \
+       const size_t _src_len = __builtin_object_size(src, 1);          \
+       const size_t _src_chars = strnlen(src, _src_len);               \
+       const size_t _copy_len = min(_dest_len - 1, _src_chars);        \
+                                                                       \
+       BUILD_BUG_ON(!__builtin_constant_p(_dest_len) ||                \
+                    !__builtin_constant_p(_src_len) ||                 \
+                    _dest_len == 0 || _dest_len == (size_t)-1 ||       \
+                    _src_len == 0 || _src_len == (size_t)-1);          \
+       memcpy(dest, src, _copy_len);                                   \
+       dest[_copy_len] = '\0';                                         \
+} while (0)
+
+/**
+ * memtostr_pad - Copy a possibly non-NUL-term string to a NUL-term string
+ *                with NUL padding in the destination
+ * @dest: Pointer to destination NUL-terminates string
+ * @src: Pointer to character array (likely marked as __nonstring)
+ *
+ * This is a replacement for strncpy() uses where the source is not
+ * a NUL-terminated string.
+ *
+ * Note that sizes of @dest and @src must be known at compile-time.
+ */
+#define memtostr_pad(dest, src)                do {                            \
+       const size_t _dest_len = __builtin_object_size(dest, 1);        \
+       const size_t _src_len = __builtin_object_size(src, 1);          \
+       const size_t _src_chars = strnlen(src, _src_len);               \
+       const size_t _copy_len = min(_dest_len - 1, _src_chars);        \
+                                                                       \
+       BUILD_BUG_ON(!__builtin_constant_p(_dest_len) ||                \
+                    !__builtin_constant_p(_src_len) ||                 \
+                    _dest_len == 0 || _dest_len == (size_t)-1 ||       \
+                    _src_len == 0 || _src_len == (size_t)-1);          \
+       memcpy(dest, src, _copy_len);                                   \
+       memset(&dest[_copy_len], 0, _dest_len - _copy_len);             \
+} while (0)
+
 /**
  * memset_after - Set a value after a struct member to the end of a struct
  *

diff --git a/lib/string_kunit.c b/lib/string_kunit.c
index de4eae91403fb229c0872b4076374f83e3c47fd2..fadad46c1b05fda8f7d0ef350c7ac77cd7ea5d9b 100644 (file)
--- a/lib/string_kunit.c
+++ b/lib/string_kunit.c
@@ -524,6 +524,31 @@ static void string_test_strlcat(struct kunit *test)
        KUNIT_EXPECT_STREQ(test, dest, "fourABE");
 }
 
+static void string_test_memtostr(struct kunit *test)
+{
+       char nonstring[7] = { 'a', 'b', 'c', 'd', 'e', 'f', 'g' };
+       char nonstring_small[3] = { 'a', 'b', 'c' };
+       char dest[sizeof(nonstring) + 1];
+
+       /* Copy in a non-NUL-terminated string into exactly right-sized dest. */
+       KUNIT_EXPECT_EQ(test, sizeof(dest), sizeof(nonstring) + 1);
+       memset(dest, 'X', sizeof(dest));
+       memtostr(dest, nonstring);
+       KUNIT_EXPECT_STREQ(test, dest, "abcdefg");
+       memset(dest, 'X', sizeof(dest));
+       memtostr(dest, nonstring_small);
+       KUNIT_EXPECT_STREQ(test, dest, "abc");
+       KUNIT_EXPECT_EQ(test, dest[7], 'X');
+
+       memset(dest, 'X', sizeof(dest));
+       memtostr_pad(dest, nonstring);
+       KUNIT_EXPECT_STREQ(test, dest, "abcdefg");
+       memset(dest, 'X', sizeof(dest));
+       memtostr_pad(dest, nonstring_small);
+       KUNIT_EXPECT_STREQ(test, dest, "abc");
+       KUNIT_EXPECT_EQ(test, dest[7], '\0');
+}
+
 static struct kunit_case string_test_cases[] = {
        KUNIT_CASE(string_test_memset16),
        KUNIT_CASE(string_test_memset32),
@@ -543,6 +568,7 @@ static struct kunit_case string_test_cases[] = {
        KUNIT_CASE(string_test_strcat),
        KUNIT_CASE(string_test_strncat),
        KUNIT_CASE(string_test_strlcat),
+       KUNIT_CASE(string_test_memtostr),
        {}
 };