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

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

[Kees Cook]

Hi,

This adds memcpy() fortify KUnit tests, similar to how the others
were done. This is in preparation for removing the 0-sized destination
exclusion now that we seem to be in good shape now after removing all
our 0-sized arrays.

-Kees

Kees Cook (3):
  kunit/fortify: Rename tests to use recommended conventions
  kunit/fortify: Do not spam logs with fortify WARNs
  kunit/fortify: Add memcpy() tests

 include/linux/fortify-string.h |   6 +-
 lib/fortify_kunit.c            | 174 ++++++++++++++++++++++++---------
 2 files changed, 135 insertions(+), 45 deletions(-)

-- 
2.34.1

[PATCH 1/3] kunit/fortify: Rename tests to use recommended conventions

[Kees Cook]

The recommended conventions for KUnit tests is ${module}_test_${what}.
Adjust the fortify tests to match.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
Cc: linux-hardening@vger.kernel.org
---
 lib/fortify_kunit.c | 80 ++++++++++++++++++++++-----------------------
 1 file changed, 40 insertions(+), 40 deletions(-)

diff --git a/lib/fortify_kunit.c b/lib/fortify_kunit.c
index 493ec02dd5b3..6f9a86c94538 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),
 	{}
 };
 
-- 
2.34.1

[PATCH 2/3] kunit/fortify: Do not spam logs with fortify WARNs

[Kees Cook]

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.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
Cc: linux-hardening@vger.kernel.org
---
 lib/fortify_kunit.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/lib/fortify_kunit.c b/lib/fortify_kunit.c
index 6f9a86c94538..bbbfc2238aa9 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)
-- 
2.34.1

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

[Kees Cook]

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@chromium.org>
---
Cc: linux-hardening@vger.kernel.org
---
 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