aboutsummaryrefslogtreecommitdiff
path: root/lib
diff options
context:
space:
mode:
authorKees Cook2024-04-09 19:31:50 -0700
committerKees Cook2024-04-24 08:57:09 -0700
commit0efc5990bca540b8d438fda23db3a72efa733eb0 (patch)
treea906f4892c15306be2b88ed4a307d100386607b1 /lib
parentdde915c5cba1fe49e980efe72662d9bc2a6b7ffd (diff)
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>
Diffstat (limited to 'lib')
-rw-r--r--lib/string_kunit.c26
1 files changed, 26 insertions, 0 deletions
diff --git a/lib/string_kunit.c b/lib/string_kunit.c
index de4eae91403f..fadad46c1b05 100644
--- 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),
{}
};