[PATCH 3/3] kunit/fortify: Add memcpy() tests

From: Kees Cook
Date: Mon Apr 29 2024 - 15:44:04 EST

Next message: Jonathan Cameron: "Re: [PATCH] iio: temperature: mcp9600: Fix temperature reading for negative values"
Previous message: Kees Cook: "[PATCH 0/3] kunit/fortify: Add memcpy() tests"
In reply to: Kees Cook: "[PATCH 1/3] kunit/fortify: Rename tests to use recommended conventions"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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.

Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
---
Cc: linux-hardening@xxxxxxxxxxxxxxx
---
include/linux/fortify-string.h | 6 ++-
lib/fortify_kunit.c | 85 ++++++++++++++++++++++++++++++++--
2 files changed, 87 insertions(+), 4 deletions(-)

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 bbbfc2238aa9..ad7fc85f34ac 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 <kunit/device.h>
#include <kunit/test.h>
#include <kunit/test-bug.h>
@@ -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),
--
2.34.1

Next message: Jonathan Cameron: "Re: [PATCH] iio: temperature: mcp9600: Fix temperature reading for negative values"
Previous message: Kees Cook: "[PATCH 0/3] kunit/fortify: Add memcpy() tests"
In reply to: Kees Cook: "[PATCH 1/3] kunit/fortify: Rename tests to use recommended conventions"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]