From 998b18072ceb0613629c256b409f4d299829c7ec Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 25 Apr 2024 16:06:22 -0700 Subject: kunit/fortify: Fix mismatched kvalloc()/vfree() usage The kv*() family of tests were accidentally freeing with vfree() instead of kvfree(). Use kvfree() instead. Fixes: 9124a2640148 ("kunit/fortify: Validate __alloc_size attribute results") Link: https://lore.kernel.org/r/20240425230619.work.299-kees@kernel.org Signed-off-by: Kees Cook --- lib/fortify_kunit.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'lib/fortify_kunit.c') diff --git a/lib/fortify_kunit.c b/lib/fortify_kunit.c index 493ec02dd5b3..86c1b1a6e2c8 100644 --- a/lib/fortify_kunit.c +++ b/lib/fortify_kunit.c @@ -267,28 +267,28 @@ DEFINE_ALLOC_SIZE_TEST_PAIR(vmalloc) \ checker((expected_pages) * PAGE_SIZE, \ kvmalloc((alloc_pages) * PAGE_SIZE, gfp), \ - vfree(p)); \ + kvfree(p)); \ checker((expected_pages) * PAGE_SIZE, \ kvmalloc_node((alloc_pages) * PAGE_SIZE, gfp, NUMA_NO_NODE), \ - vfree(p)); \ + kvfree(p)); \ checker((expected_pages) * PAGE_SIZE, \ kvzalloc((alloc_pages) * PAGE_SIZE, gfp), \ - vfree(p)); \ + kvfree(p)); \ checker((expected_pages) * PAGE_SIZE, \ kvzalloc_node((alloc_pages) * PAGE_SIZE, gfp, NUMA_NO_NODE), \ - vfree(p)); \ + kvfree(p)); \ checker((expected_pages) * PAGE_SIZE, \ kvcalloc(1, (alloc_pages) * PAGE_SIZE, gfp), \ - vfree(p)); \ + kvfree(p)); \ checker((expected_pages) * PAGE_SIZE, \ kvcalloc((alloc_pages) * PAGE_SIZE, 1, gfp), \ - vfree(p)); \ + kvfree(p)); \ checker((expected_pages) * PAGE_SIZE, \ kvmalloc_array(1, (alloc_pages) * PAGE_SIZE, gfp), \ - vfree(p)); \ + kvfree(p)); \ checker((expected_pages) * PAGE_SIZE, \ kvmalloc_array((alloc_pages) * PAGE_SIZE, 1, gfp), \ - vfree(p)); \ + kvfree(p)); \ \ prev_size = (expected_pages) * PAGE_SIZE; \ orig = kvmalloc(prev_size, gfp); \ -- cgit v1.2.3-59-g8ed1b From a0d6677ec3f1da894cad9df6a962821429828917 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 29 Apr 2024 12:43:39 -0700 Subject: kunit/fortify: Rename tests to use recommended conventions The recommended conventions for KUnit tests is ${module}_test_${what}. Adjust the fortify tests to match. Link: https://lore.kernel.org/r/20240429194342.2421639-1-keescook@chromium.org Signed-off-by: Kees Cook --- lib/fortify_kunit.c | 80 ++++++++++++++++++++++++++--------------------------- 1 file changed, 40 insertions(+), 40 deletions(-) (limited to 'lib/fortify_kunit.c') diff --git a/lib/fortify_kunit.c b/lib/fortify_kunit.c index 86c1b1a6e2c8..5d706529c464 100644 --- a/lib/fortify_kunit.c +++ b/lib/fortify_kunit.c @@ -64,7 +64,7 @@ void fortify_add_kunit_error(int write) kunit_put_resource(resource); } -static void known_sizes_test(struct kunit *test) +static void fortify_test_known_sizes(struct kunit *test) { KUNIT_EXPECT_EQ(test, __compiletime_strlen("88888888"), 8); KUNIT_EXPECT_EQ(test, __compiletime_strlen(array_of_10), 10); @@ -97,7 +97,7 @@ static noinline size_t want_minus_one(int pick) return __compiletime_strlen(str); } -static void control_flow_split_test(struct kunit *test) +static void fortify_test_control_flow_split(struct kunit *test) { KUNIT_EXPECT_EQ(test, want_minus_one(pick), SIZE_MAX); } @@ -173,11 +173,11 @@ static volatile size_t unknown_size = 50; #endif #define DEFINE_ALLOC_SIZE_TEST_PAIR(allocator) \ -static void alloc_size_##allocator##_const_test(struct kunit *test) \ +static void fortify_test_alloc_size_##allocator##_const(struct kunit *test) \ { \ CONST_TEST_BODY(TEST_##allocator); \ } \ -static void alloc_size_##allocator##_dynamic_test(struct kunit *test) \ +static void fortify_test_alloc_size_##allocator##_dynamic(struct kunit *test) \ { \ DYNAMIC_TEST_BODY(TEST_##allocator); \ } @@ -361,7 +361,7 @@ struct fortify_padding { /* Force compiler into not being able to resolve size at compile-time. */ static volatile int unconst; -static void strlen_test(struct kunit *test) +static void fortify_test_strlen(struct kunit *test) { struct fortify_padding pad = { }; int i, end = sizeof(pad.buf) - 1; @@ -384,7 +384,7 @@ static void strlen_test(struct kunit *test) KUNIT_EXPECT_EQ(test, fortify_read_overflows, 1); } -static void strnlen_test(struct kunit *test) +static void fortify_test_strnlen(struct kunit *test) { struct fortify_padding pad = { }; int i, end = sizeof(pad.buf) - 1; @@ -422,7 +422,7 @@ static void strnlen_test(struct kunit *test) KUNIT_EXPECT_EQ(test, fortify_read_overflows, 2); } -static void strcpy_test(struct kunit *test) +static void fortify_test_strcpy(struct kunit *test) { struct fortify_padding pad = { }; char src[sizeof(pad.buf) + 1] = { }; @@ -480,7 +480,7 @@ static void strcpy_test(struct kunit *test) KUNIT_EXPECT_EQ(test, pad.bytes_after, 0); } -static void strncpy_test(struct kunit *test) +static void fortify_test_strncpy(struct kunit *test) { struct fortify_padding pad = { }; char src[] = "Copy me fully into a small buffer and I will overflow!"; @@ -539,7 +539,7 @@ static void strncpy_test(struct kunit *test) KUNIT_EXPECT_EQ(test, pad.bytes_after, 0); } -static void strscpy_test(struct kunit *test) +static void fortify_test_strscpy(struct kunit *test) { struct fortify_padding pad = { }; char src[] = "Copy me fully into a small buffer and I will overflow!"; @@ -596,7 +596,7 @@ static void strscpy_test(struct kunit *test) KUNIT_EXPECT_EQ(test, pad.bytes_after, 0); } -static void strcat_test(struct kunit *test) +static void fortify_test_strcat(struct kunit *test) { struct fortify_padding pad = { }; char src[sizeof(pad.buf) / 2] = { }; @@ -653,7 +653,7 @@ static void strcat_test(struct kunit *test) KUNIT_EXPECT_EQ(test, pad.bytes_after, 0); } -static void strncat_test(struct kunit *test) +static void fortify_test_strncat(struct kunit *test) { struct fortify_padding pad = { }; char src[sizeof(pad.buf)] = { }; @@ -726,7 +726,7 @@ static void strncat_test(struct kunit *test) KUNIT_EXPECT_EQ(test, pad.bytes_after, 0); } -static void strlcat_test(struct kunit *test) +static void fortify_test_strlcat(struct kunit *test) { struct fortify_padding pad = { }; char src[sizeof(pad.buf)] = { }; @@ -811,7 +811,7 @@ static void strlcat_test(struct kunit *test) KUNIT_EXPECT_EQ(test, pad.bytes_after, 0); } -static void memscan_test(struct kunit *test) +static void fortify_test_memscan(struct kunit *test) { char haystack[] = "Where oh where is my memory range?"; char *mem = haystack + strlen("Where oh where is "); @@ -830,7 +830,7 @@ static void memscan_test(struct kunit *test) KUNIT_EXPECT_EQ(test, fortify_read_overflows, 2); } -static void memchr_test(struct kunit *test) +static void fortify_test_memchr(struct kunit *test) { char haystack[] = "Where oh where is my memory range?"; char *mem = haystack + strlen("Where oh where is "); @@ -849,7 +849,7 @@ static void memchr_test(struct kunit *test) KUNIT_EXPECT_EQ(test, fortify_read_overflows, 2); } -static void memchr_inv_test(struct kunit *test) +static void fortify_test_memchr_inv(struct kunit *test) { char haystack[] = "Where oh where is my memory range?"; char *mem = haystack + 1; @@ -869,7 +869,7 @@ static void memchr_inv_test(struct kunit *test) KUNIT_EXPECT_EQ(test, fortify_read_overflows, 2); } -static void memcmp_test(struct kunit *test) +static void fortify_test_memcmp(struct kunit *test) { char one[] = "My mind is going ..."; char two[] = "My mind is going ... I can feel it."; @@ -891,7 +891,7 @@ static void memcmp_test(struct kunit *test) KUNIT_EXPECT_EQ(test, fortify_read_overflows, 2); } -static void kmemdup_test(struct kunit *test) +static void fortify_test_kmemdup(struct kunit *test) { char src[] = "I got Doom running on it!"; char *copy; @@ -951,31 +951,31 @@ static int fortify_test_init(struct kunit *test) } static struct kunit_case fortify_test_cases[] = { - KUNIT_CASE(known_sizes_test), - KUNIT_CASE(control_flow_split_test), - KUNIT_CASE(alloc_size_kmalloc_const_test), - KUNIT_CASE(alloc_size_kmalloc_dynamic_test), - KUNIT_CASE(alloc_size_vmalloc_const_test), - KUNIT_CASE(alloc_size_vmalloc_dynamic_test), - KUNIT_CASE(alloc_size_kvmalloc_const_test), - KUNIT_CASE(alloc_size_kvmalloc_dynamic_test), - KUNIT_CASE(alloc_size_devm_kmalloc_const_test), - KUNIT_CASE(alloc_size_devm_kmalloc_dynamic_test), - KUNIT_CASE(strlen_test), - KUNIT_CASE(strnlen_test), - KUNIT_CASE(strcpy_test), - KUNIT_CASE(strncpy_test), - KUNIT_CASE(strscpy_test), - KUNIT_CASE(strcat_test), - KUNIT_CASE(strncat_test), - KUNIT_CASE(strlcat_test), + KUNIT_CASE(fortify_test_known_sizes), + KUNIT_CASE(fortify_test_control_flow_split), + KUNIT_CASE(fortify_test_alloc_size_kmalloc_const), + KUNIT_CASE(fortify_test_alloc_size_kmalloc_dynamic), + KUNIT_CASE(fortify_test_alloc_size_vmalloc_const), + KUNIT_CASE(fortify_test_alloc_size_vmalloc_dynamic), + KUNIT_CASE(fortify_test_alloc_size_kvmalloc_const), + KUNIT_CASE(fortify_test_alloc_size_kvmalloc_dynamic), + KUNIT_CASE(fortify_test_alloc_size_devm_kmalloc_const), + KUNIT_CASE(fortify_test_alloc_size_devm_kmalloc_dynamic), + KUNIT_CASE(fortify_test_strlen), + KUNIT_CASE(fortify_test_strnlen), + KUNIT_CASE(fortify_test_strcpy), + KUNIT_CASE(fortify_test_strncpy), + KUNIT_CASE(fortify_test_strscpy), + KUNIT_CASE(fortify_test_strcat), + KUNIT_CASE(fortify_test_strncat), + KUNIT_CASE(fortify_test_strlcat), /* skip memset: performs bounds checking on whole structs */ /* skip memcpy: still using warn-and-overwrite instead of hard-fail */ - KUNIT_CASE(memscan_test), - KUNIT_CASE(memchr_test), - KUNIT_CASE(memchr_inv_test), - KUNIT_CASE(memcmp_test), - KUNIT_CASE(kmemdup_test), + KUNIT_CASE(fortify_test_memscan), + KUNIT_CASE(fortify_test_memchr), + KUNIT_CASE(fortify_test_memchr_inv), + KUNIT_CASE(fortify_test_memcmp), + KUNIT_CASE(fortify_test_kmemdup), {} }; -- cgit v1.2.3-59-g8ed1b From 091f79e8de44736a1e677701d67334bba5b749e3 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 29 Apr 2024 12:43:40 -0700 Subject: kunit/fortify: Do not spam logs with fortify WARNs When running KUnit fortify tests, we're already doing precise tracking of which warnings are getting hit. Don't fill the logs with WARNs unless we've been explicitly built with DEBUG enabled. Link: https://lore.kernel.org/r/20240429194342.2421639-2-keescook@chromium.org Signed-off-by: Kees Cook --- lib/fortify_kunit.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'lib/fortify_kunit.c') diff --git a/lib/fortify_kunit.c b/lib/fortify_kunit.c index 5d706529c464..601fa327c5b7 100644 --- a/lib/fortify_kunit.c +++ b/lib/fortify_kunit.c @@ -15,10 +15,17 @@ */ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +/* We don't need to fill dmesg with the fortify WARNs during testing. */ +#ifdef DEBUG +# define FORTIFY_REPORT_KUNIT(x...) __fortify_report(x) +#else +# define FORTIFY_REPORT_KUNIT(x...) do { } while (0) +#endif + /* Redefine fortify_panic() to track failures. */ void fortify_add_kunit_error(int write); #define fortify_panic(func, write, avail, size, retfail) do { \ - __fortify_report(FORTIFY_REASON(func, write), avail, size); \ + FORTIFY_REPORT_KUNIT(FORTIFY_REASON(func, write), avail, size); \ fortify_add_kunit_error(write); \ return (retfail); \ } while (0) -- cgit v1.2.3-59-g8ed1b From 26f812ba75890c48644a31e3cfe3dd9762138968 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 29 Apr 2024 12:43:41 -0700 Subject: kunit/fortify: Add memcpy() tests Add fortify tests for memcpy() and memmove(). This can use a similar method to the fortify_panic() replacement, only we can do it for what was the WARN_ONCE(), which can be redefined. Since this is primarily testing the fortify behaviors of the memcpy() and memmove() defenses, the tests for memcpy() and memmove() are identical. Link: https://lore.kernel.org/r/20240429194342.2421639-3-keescook@chromium.org Signed-off-by: Kees Cook --- include/linux/fortify-string.h | 6 ++- lib/fortify_kunit.c | 85 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 87 insertions(+), 4 deletions(-) (limited to 'lib/fortify_kunit.c') diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h index 6aeebe0a6777..a0bb13825109 100644 --- a/include/linux/fortify-string.h +++ b/include/linux/fortify-string.h @@ -15,10 +15,14 @@ #define FORTIFY_REASON(func, write) (FIELD_PREP(BIT(0), write) | \ FIELD_PREP(GENMASK(7, 1), func)) +/* Overridden by KUnit tests. */ #ifndef fortify_panic # define fortify_panic(func, write, avail, size, retfail) \ __fortify_panic(FORTIFY_REASON(func, write), avail, size) #endif +#ifndef fortify_warn_once +# define fortify_warn_once(x...) WARN_ONCE(x) +#endif #define FORTIFY_READ 0 #define FORTIFY_WRITE 1 @@ -609,7 +613,7 @@ __FORTIFY_INLINE bool fortify_memcpy_chk(__kernel_size_t size, const size_t __q_size = (q_size); \ const size_t __p_size_field = (p_size_field); \ const size_t __q_size_field = (q_size_field); \ - WARN_ONCE(fortify_memcpy_chk(__fortify_size, __p_size, \ + fortify_warn_once(fortify_memcpy_chk(__fortify_size, __p_size, \ __q_size, __p_size_field, \ __q_size_field, FORTIFY_FUNC_ ##op), \ #op ": detected field-spanning write (size %zu) of single %s (size %zu)\n", \ diff --git a/lib/fortify_kunit.c b/lib/fortify_kunit.c index 601fa327c5b7..ef3e4c68b759 100644 --- a/lib/fortify_kunit.c +++ b/lib/fortify_kunit.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 /* - * Runtime test cases for CONFIG_FORTIFY_SOURCE. For testing memcpy(), - * see FORTIFY_MEM_* tests in LKDTM (drivers/misc/lkdtm/fortify.c). + * Runtime test cases for CONFIG_FORTIFY_SOURCE. For additional memcpy() + * testing see FORTIFY_MEM_* tests in LKDTM (drivers/misc/lkdtm/fortify.c). * * For corner cases with UBSAN, try testing with: * @@ -18,8 +18,10 @@ /* We don't need to fill dmesg with the fortify WARNs during testing. */ #ifdef DEBUG # define FORTIFY_REPORT_KUNIT(x...) __fortify_report(x) +# define FORTIFY_WARN_KUNIT(x...) WARN_ONCE(x) #else # define FORTIFY_REPORT_KUNIT(x...) do { } while (0) +# define FORTIFY_WARN_KUNIT(x...) do { } while (0) #endif /* Redefine fortify_panic() to track failures. */ @@ -30,6 +32,14 @@ void fortify_add_kunit_error(int write); return (retfail); \ } while (0) +/* Redefine fortify_warn_once() to track memcpy() failures. */ +#define fortify_warn_once(chk_func, x...) do { \ + bool __result = chk_func; \ + FORTIFY_WARN_KUNIT(__result, x); \ + if (__result) \ + fortify_add_kunit_error(1); \ +} while (0) + #include #include #include @@ -818,6 +828,74 @@ static void fortify_test_strlcat(struct kunit *test) KUNIT_EXPECT_EQ(test, pad.bytes_after, 0); } +/* Check for 0-sized arrays... */ +struct fortify_zero_sized { + unsigned long bytes_before; + char buf[0]; + unsigned long bytes_after; +}; + +#define __fortify_test(memfunc) \ +static void fortify_test_##memfunc(struct kunit *test) \ +{ \ + struct fortify_zero_sized zero = { }; \ + struct fortify_padding pad = { }; \ + char srcA[sizeof(pad.buf) + 2]; \ + char srcB[sizeof(pad.buf) + 2]; \ + size_t len = sizeof(pad.buf) + unconst; \ + \ + memset(srcA, 'A', sizeof(srcA)); \ + KUNIT_ASSERT_EQ(test, srcA[0], 'A'); \ + memset(srcB, 'B', sizeof(srcB)); \ + KUNIT_ASSERT_EQ(test, srcB[0], 'B'); \ + \ + memfunc(pad.buf, srcA, 0 + unconst); \ + KUNIT_EXPECT_EQ(test, pad.buf[0], '\0'); \ + KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0); \ + KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0); \ + memfunc(pad.buf + 1, srcB, 1 + unconst); \ + KUNIT_EXPECT_EQ(test, pad.buf[0], '\0'); \ + KUNIT_EXPECT_EQ(test, pad.buf[1], 'B'); \ + KUNIT_EXPECT_EQ(test, pad.buf[2], '\0'); \ + KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0); \ + KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0); \ + memfunc(pad.buf, srcA, 1 + unconst); \ + KUNIT_EXPECT_EQ(test, pad.buf[0], 'A'); \ + KUNIT_EXPECT_EQ(test, pad.buf[1], 'B'); \ + KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0); \ + KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0); \ + memfunc(pad.buf, srcA, len - 1); \ + KUNIT_EXPECT_EQ(test, pad.buf[1], 'A'); \ + KUNIT_EXPECT_EQ(test, pad.buf[len - 1], '\0'); \ + KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0); \ + KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0); \ + memfunc(pad.buf, srcA, len); \ + KUNIT_EXPECT_EQ(test, pad.buf[1], 'A'); \ + KUNIT_EXPECT_EQ(test, pad.buf[len - 1], 'A'); \ + KUNIT_EXPECT_EQ(test, pad.bytes_after, 0); \ + KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0); \ + KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0); \ + memfunc(pad.buf, srcA, len + 1); \ + KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0); \ + KUNIT_EXPECT_EQ(test, fortify_write_overflows, 1); \ + memfunc(pad.buf + 1, srcB, len); \ + KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0); \ + KUNIT_EXPECT_EQ(test, fortify_write_overflows, 2); \ + \ + /* Reset error counter. */ \ + fortify_write_overflows = 0; \ + /* Copy nothing into nothing: no errors. */ \ + memfunc(zero.buf, srcB, 0 + unconst); \ + KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0); \ + KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0); \ + /* We currently explicitly ignore zero-sized dests. */ \ + memfunc(zero.buf, srcB, 1 + unconst); \ + KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0); \ + KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0); \ +} +__fortify_test(memcpy) +__fortify_test(memmove) + static void fortify_test_memscan(struct kunit *test) { char haystack[] = "Where oh where is my memory range?"; @@ -977,7 +1055,8 @@ static struct kunit_case fortify_test_cases[] = { KUNIT_CASE(fortify_test_strncat), KUNIT_CASE(fortify_test_strlcat), /* skip memset: performs bounds checking on whole structs */ - /* skip memcpy: still using warn-and-overwrite instead of hard-fail */ + KUNIT_CASE(fortify_test_memcpy), + KUNIT_CASE(fortify_test_memmove), KUNIT_CASE(fortify_test_memscan), KUNIT_CASE(fortify_test_memchr), KUNIT_CASE(fortify_test_memchr_inv), -- cgit v1.2.3-59-g8ed1b From 74df22453c51392476117d7330bf02cee6e987cf Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Wed, 1 May 2024 16:29:48 -0700 Subject: kunit/fortify: Fix replaced failure path to unbreak __alloc_size The __alloc_size annotation for kmemdup() was getting disabled under KUnit testing because the replaced fortify_panic macro implementation was using "return NULL" as a way to survive the sanity checking. But having the chance to return NULL invalidated __alloc_size, so kmemdup was not passing the __builtin_dynamic_object_size() tests any more: [23:26:18] [PASSED] fortify_test_alloc_size_kmalloc_const [23:26:19] # fortify_test_alloc_size_kmalloc_dynamic: EXPECTATION FAILED at lib/fortify_kunit.c:265 [23:26:19] Expected __builtin_dynamic_object_size(p, 1) == expected, but [23:26:19] __builtin_dynamic_object_size(p, 1) == -1 (0xffffffffffffffff) [23:26:19] expected == 11 (0xb) [23:26:19] __alloc_size() not working with __bdos on kmemdup("hello there", len, gfp) [23:26:19] [FAILED] fortify_test_alloc_size_kmalloc_dynamic Normal builds were not affected: __alloc_size continued to work there. Use a zero-sized allocation instead, which allows __alloc_size to behave. Fixes: 4ce615e798a7 ("fortify: Provide KUnit counters for failure testing") Fixes: fa4a3f86d498 ("fortify: Add KUnit tests for runtime overflows") Link: https://lore.kernel.org/r/20240501232937.work.532-kees@kernel.org Signed-off-by: Kees Cook --- include/linux/fortify-string.h | 3 ++- lib/fortify_kunit.c | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) (limited to 'lib/fortify_kunit.c') diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h index a0bb13825109..85fc0e6f0f7f 100644 --- a/include/linux/fortify-string.h +++ b/include/linux/fortify-string.h @@ -738,7 +738,8 @@ __FORTIFY_INLINE void *kmemdup(const void * const POS0 p, size_t size, gfp_t gfp if (__compiletime_lessthan(p_size, size)) __read_overflow(); if (p_size < size) - fortify_panic(FORTIFY_FUNC_kmemdup, FORTIFY_READ, p_size, size, NULL); + fortify_panic(FORTIFY_FUNC_kmemdup, FORTIFY_READ, p_size, size, + __real_kmemdup(p, 0, gfp)); return __real_kmemdup(p, size, gfp); } diff --git a/lib/fortify_kunit.c b/lib/fortify_kunit.c index ef3e4c68b759..306522fd0aa2 100644 --- a/lib/fortify_kunit.c +++ b/lib/fortify_kunit.c @@ -1002,19 +1002,19 @@ static void fortify_test_kmemdup(struct kunit *test) /* Out of bounds by 1 byte. */ copy = kmemdup(src, len + 1, GFP_KERNEL); - KUNIT_EXPECT_NULL(test, copy); + KUNIT_EXPECT_PTR_EQ(test, copy, ZERO_SIZE_PTR); KUNIT_EXPECT_EQ(test, fortify_read_overflows, 1); kfree(copy); /* Way out of bounds. */ copy = kmemdup(src, len * 2, GFP_KERNEL); - KUNIT_EXPECT_NULL(test, copy); + KUNIT_EXPECT_PTR_EQ(test, copy, ZERO_SIZE_PTR); KUNIT_EXPECT_EQ(test, fortify_read_overflows, 2); kfree(copy); /* Starting offset causing out of bounds. */ copy = kmemdup(src + 1, len, GFP_KERNEL); - KUNIT_EXPECT_NULL(test, copy); + KUNIT_EXPECT_PTR_EQ(test, copy, ZERO_SIZE_PTR); KUNIT_EXPECT_EQ(test, fortify_read_overflows, 3); kfree(copy); } -- cgit v1.2.3-59-g8ed1b From 7d78a77733552092361239b1d8afaf8412f5dffd Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Wed, 1 May 2024 16:32:02 -0700 Subject: string: Add additional __realloc_size() annotations for "dup" helpers Several other "dup"-style interfaces could use the __realloc_size() attribute. (As a reminder to myself and others: "realloc" is used here instead of "alloc" because the "alloc_size" attribute implies that the memory contents are uninitialized. Since we're copying contents into the resulting allocation, it must use "realloc_size" to avoid confusing the compiler's optimization passes.) Add KUnit test coverage where possible. (KUnit still does not have the ability to manipulate userspace memory.) Reviewed-by: Andy Shevchenko Link: https://lore.kernel.org/r/20240502145218.it.729-kees@kernel.org Signed-off-by: Kees Cook --- include/linux/string.h | 13 ++++++++----- lib/fortify_kunit.c | 26 ++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 5 deletions(-) (limited to 'lib/fortify_kunit.c') diff --git a/include/linux/string.h b/include/linux/string.h index 86aa6cd35167..10e5177bb49c 100644 --- a/include/linux/string.h +++ b/include/linux/string.h @@ -14,8 +14,8 @@ #include extern char *strndup_user(const char __user *, long); -extern void *memdup_user(const void __user *, size_t); -extern void *vmemdup_user(const void __user *, size_t); +extern void *memdup_user(const void __user *, size_t) __realloc_size(2); +extern void *vmemdup_user(const void __user *, size_t) __realloc_size(2); extern void *memdup_user_nul(const void __user *, size_t); /** @@ -27,7 +27,8 @@ extern void *memdup_user_nul(const void __user *, size_t); * Return: an ERR_PTR() on failure. Result is physically * contiguous, to be freed by kfree(). */ -static inline void *memdup_array_user(const void __user *src, size_t n, size_t size) +static inline __realloc_size(2, 3) +void *memdup_array_user(const void __user *src, size_t n, size_t size) { size_t nbytes; @@ -46,7 +47,8 @@ static inline void *memdup_array_user(const void __user *src, size_t n, size_t s * Return: an ERR_PTR() on failure. Result may be not * physically contiguous. Use kvfree() to free. */ -static inline void *vmemdup_array_user(const void __user *src, size_t n, size_t size) +static inline __realloc_size(2, 3) +void *vmemdup_array_user(const void __user *src, size_t n, size_t size) { size_t nbytes; @@ -285,7 +287,8 @@ extern char *kstrndup(const char *s, size_t len, gfp_t gfp); extern void *kmemdup(const void *src, size_t len, gfp_t gfp) __realloc_size(2); extern void *kvmemdup(const void *src, size_t len, gfp_t gfp) __realloc_size(2); extern char *kmemdup_nul(const char *s, size_t len, gfp_t gfp); -extern void *kmemdup_array(const void *src, size_t element_size, size_t count, gfp_t gfp); +extern void *kmemdup_array(const void *src, size_t element_size, size_t count, gfp_t gfp) + __realloc_size(2, 3); /* lib/argv_split.c */ extern char **argv_split(gfp_t gfp, const char *str, int *argcp); diff --git a/lib/fortify_kunit.c b/lib/fortify_kunit.c index 306522fd0aa2..d2377e00caab 100644 --- a/lib/fortify_kunit.c +++ b/lib/fortify_kunit.c @@ -363,6 +363,31 @@ DEFINE_ALLOC_SIZE_TEST_PAIR(kvmalloc) } while (0) DEFINE_ALLOC_SIZE_TEST_PAIR(devm_kmalloc) +static const char * const test_strs[] = { + "", + "Hello there", + "A longer string, just for variety", +}; + +#define TEST_realloc(checker) do { \ + gfp_t gfp = GFP_KERNEL; \ + size_t len; \ + int i; \ + \ + for (i = 0; i < ARRAY_SIZE(test_strs); i++) { \ + len = strlen(test_strs[i]); \ + KUNIT_EXPECT_EQ(test, __builtin_constant_p(len), 0); \ + checker(len, kmemdup_array(test_strs[i], len, 1, gfp), \ + kfree(p)); \ + checker(len, kmemdup(test_strs[i], len, gfp), \ + kfree(p)); \ + } \ +} while (0) +static void fortify_test_realloc_size(struct kunit *test) +{ + TEST_realloc(check_dynamic); +} + /* * We can't have an array at the end of a structure or else * builds without -fstrict-flex-arrays=3 will report them as @@ -1046,6 +1071,7 @@ static struct kunit_case fortify_test_cases[] = { KUNIT_CASE(fortify_test_alloc_size_kvmalloc_dynamic), KUNIT_CASE(fortify_test_alloc_size_devm_kmalloc_const), KUNIT_CASE(fortify_test_alloc_size_devm_kmalloc_dynamic), + KUNIT_CASE(fortify_test_realloc_size), KUNIT_CASE(fortify_test_strlen), KUNIT_CASE(fortify_test_strnlen), KUNIT_CASE(fortify_test_strcpy), -- cgit v1.2.3-59-g8ed1b From ae1a863bcdbd6ea2abc93519a82ab5d715d5dcbc Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Sat, 18 May 2024 11:40:23 -0700 Subject: kunit/fortify: Fix memcmp() test to be amplitude agnostic When memcmp() returns a non-zero value, only the signed bit has any meaning. The actual value may differ between implementations. Reported-by: Nathan Chancellor Closes: https://github.com/ClangBuiltLinux/linux/issues/2025 Tested-by: Nathan Chancellor Link: https://lore.kernel.org/r/20240518184020.work.604-kees@kernel.org Signed-off-by: Kees Cook --- lib/fortify_kunit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/fortify_kunit.c') diff --git a/lib/fortify_kunit.c b/lib/fortify_kunit.c index d2377e00caab..39da5b3bc649 100644 --- a/lib/fortify_kunit.c +++ b/lib/fortify_kunit.c @@ -990,7 +990,7 @@ static void fortify_test_memcmp(struct kunit *test) KUNIT_ASSERT_EQ(test, memcmp(one, two, one_len), 0); KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0); /* Still in bounds, but no longer matching. */ - KUNIT_ASSERT_EQ(test, memcmp(one, two, one_len + 1), -32); + KUNIT_ASSERT_LT(test, memcmp(one, two, one_len + 1), 0); KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0); /* Catch too-large ranges. */ -- cgit v1.2.3-59-g8ed1b