Skip to content

Commit 0efc599

Browse files
committed
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>
1 parent dde915c commit 0efc599

2 files changed

Lines changed: 75 additions & 0 deletions

File tree

include/linux/string.h

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,55 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count,
422422
memcpy(dest, src, strnlen(src, min(_src_len, _dest_len))); \
423423
} while (0)
424424

425+
/**
426+
* memtostr - Copy a possibly non-NUL-term string to a NUL-term string
427+
* @dest: Pointer to destination NUL-terminates string
428+
* @src: Pointer to character array (likely marked as __nonstring)
429+
*
430+
* This is a replacement for strncpy() uses where the source is not
431+
* a NUL-terminated string.
432+
*
433+
* Note that sizes of @dest and @src must be known at compile-time.
434+
*/
435+
#define memtostr(dest, src) do { \
436+
const size_t _dest_len = __builtin_object_size(dest, 1); \
437+
const size_t _src_len = __builtin_object_size(src, 1); \
438+
const size_t _src_chars = strnlen(src, _src_len); \
439+
const size_t _copy_len = min(_dest_len - 1, _src_chars); \
440+
\
441+
BUILD_BUG_ON(!__builtin_constant_p(_dest_len) || \
442+
!__builtin_constant_p(_src_len) || \
443+
_dest_len == 0 || _dest_len == (size_t)-1 || \
444+
_src_len == 0 || _src_len == (size_t)-1); \
445+
memcpy(dest, src, _copy_len); \
446+
dest[_copy_len] = '\0'; \
447+
} while (0)
448+
449+
/**
450+
* memtostr_pad - Copy a possibly non-NUL-term string to a NUL-term string
451+
* with NUL padding in the destination
452+
* @dest: Pointer to destination NUL-terminates string
453+
* @src: Pointer to character array (likely marked as __nonstring)
454+
*
455+
* This is a replacement for strncpy() uses where the source is not
456+
* a NUL-terminated string.
457+
*
458+
* Note that sizes of @dest and @src must be known at compile-time.
459+
*/
460+
#define memtostr_pad(dest, src) do { \
461+
const size_t _dest_len = __builtin_object_size(dest, 1); \
462+
const size_t _src_len = __builtin_object_size(src, 1); \
463+
const size_t _src_chars = strnlen(src, _src_len); \
464+
const size_t _copy_len = min(_dest_len - 1, _src_chars); \
465+
\
466+
BUILD_BUG_ON(!__builtin_constant_p(_dest_len) || \
467+
!__builtin_constant_p(_src_len) || \
468+
_dest_len == 0 || _dest_len == (size_t)-1 || \
469+
_src_len == 0 || _src_len == (size_t)-1); \
470+
memcpy(dest, src, _copy_len); \
471+
memset(&dest[_copy_len], 0, _dest_len - _copy_len); \
472+
} while (0)
473+
425474
/**
426475
* memset_after - Set a value after a struct member to the end of a struct
427476
*

lib/string_kunit.c

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,31 @@ static void string_test_strlcat(struct kunit *test)
524524
KUNIT_EXPECT_STREQ(test, dest, "fourABE");
525525
}
526526

527+
static void string_test_memtostr(struct kunit *test)
528+
{
529+
char nonstring[7] = { 'a', 'b', 'c', 'd', 'e', 'f', 'g' };
530+
char nonstring_small[3] = { 'a', 'b', 'c' };
531+
char dest[sizeof(nonstring) + 1];
532+
533+
/* Copy in a non-NUL-terminated string into exactly right-sized dest. */
534+
KUNIT_EXPECT_EQ(test, sizeof(dest), sizeof(nonstring) + 1);
535+
memset(dest, 'X', sizeof(dest));
536+
memtostr(dest, nonstring);
537+
KUNIT_EXPECT_STREQ(test, dest, "abcdefg");
538+
memset(dest, 'X', sizeof(dest));
539+
memtostr(dest, nonstring_small);
540+
KUNIT_EXPECT_STREQ(test, dest, "abc");
541+
KUNIT_EXPECT_EQ(test, dest[7], 'X');
542+
543+
memset(dest, 'X', sizeof(dest));
544+
memtostr_pad(dest, nonstring);
545+
KUNIT_EXPECT_STREQ(test, dest, "abcdefg");
546+
memset(dest, 'X', sizeof(dest));
547+
memtostr_pad(dest, nonstring_small);
548+
KUNIT_EXPECT_STREQ(test, dest, "abc");
549+
KUNIT_EXPECT_EQ(test, dest[7], '\0');
550+
}
551+
527552
static struct kunit_case string_test_cases[] = {
528553
KUNIT_CASE(string_test_memset16),
529554
KUNIT_CASE(string_test_memset32),
@@ -543,6 +568,7 @@ static struct kunit_case string_test_cases[] = {
543568
KUNIT_CASE(string_test_strcat),
544569
KUNIT_CASE(string_test_strncat),
545570
KUNIT_CASE(string_test_strlcat),
571+
KUNIT_CASE(string_test_memtostr),
546572
{}
547573
};
548574

0 commit comments

Comments
 (0)