lkdtm/fortify: Consolidate FORTIFY_SOURCE tests
authorKees Cook <keescook@chromium.org>
Wed, 18 Aug 2021 17:48:53 +0000 (10:48 -0700)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Wed, 18 Aug 2021 20:28:51 +0000 (22:28 +0200)
The FORTIFY_SOURCE tests were split between bugs.c and fortify.c. Move
tests into fortify.c, standardize their naming, add CONFIG hints, and
add them to the lkdtm selftests.

Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Kees Cook <keescook@chromium.org>
Link: https://lore.kernel.org/r/20210818174855.2307828-3-keescook@chromium.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/misc/lkdtm/bugs.c
drivers/misc/lkdtm/core.c
drivers/misc/lkdtm/fortify.c
drivers/misc/lkdtm/lkdtm.h
tools/testing/selftests/lkdtm/tests.txt

index 03171e4..4282b62 100644 (file)
@@ -507,53 +507,3 @@ noinline void lkdtm_CORRUPT_PAC(void)
        pr_err("XFAIL: this test is arm64-only\n");
 #endif
 }
-
-void lkdtm_FORTIFY_OBJECT(void)
-{
-       struct target {
-               char a[10];
-       } target[2] = {};
-       int result;
-
-       /*
-        * Using volatile prevents the compiler from determining the value of
-        * 'size' at compile time. Without that, we would get a compile error
-        * rather than a runtime error.
-        */
-       volatile int size = 11;
-
-       pr_info("trying to read past the end of a struct\n");
-
-       result = memcmp(&target[0], &target[1], size);
-
-       /* Print result to prevent the code from being eliminated */
-       pr_err("FAIL: fortify did not catch an object overread!\n"
-              "\"%d\" was the memcmp result.\n", result);
-}
-
-void lkdtm_FORTIFY_SUBOBJECT(void)
-{
-       struct target {
-               char a[10];
-               char b[10];
-       } target;
-       char *src;
-
-       src = kmalloc(20, GFP_KERNEL);
-       strscpy(src, "over ten bytes", 20);
-
-       pr_info("trying to strcpy past the end of a member of a struct\n");
-
-       /*
-        * strncpy(target.a, src, 20); will hit a compile error because the
-        * compiler knows at build time that target.a < 20 bytes. Use strcpy()
-        * to force a runtime error.
-        */
-       strcpy(target.a, src);
-
-       /* Use target.a to prevent the code from being eliminated */
-       pr_err("FAIL: fortify did not catch an sub-object overrun!\n"
-              "\"%s\" was copied.\n", target.a);
-
-       kfree(src);
-}
index c9a0ad6..486ce0c 100644 (file)
@@ -118,8 +118,6 @@ static const struct crashtype crashtypes[] = {
        CRASHTYPE(UNSET_SMEP),
        CRASHTYPE(CORRUPT_PAC),
        CRASHTYPE(UNALIGNED_LOAD_STORE_WRITE),
-       CRASHTYPE(FORTIFY_OBJECT),
-       CRASHTYPE(FORTIFY_SUBOBJECT),
        CRASHTYPE(SLAB_LINEAR_OVERFLOW),
        CRASHTYPE(VMALLOC_LINEAR_OVERFLOW),
        CRASHTYPE(WRITE_AFTER_FREE),
@@ -179,6 +177,8 @@ static const struct crashtype crashtypes[] = {
        CRASHTYPE(USERCOPY_KERNEL),
        CRASHTYPE(STACKLEAK_ERASING),
        CRASHTYPE(CFI_FORWARD_PROTO),
+       CRASHTYPE(FORTIFIED_OBJECT),
+       CRASHTYPE(FORTIFIED_SUBOBJECT),
        CRASHTYPE(FORTIFIED_STRSCPY),
        CRASHTYPE(DOUBLE_FAULT),
 #ifdef CONFIG_PPC_BOOK3S_64
index 0f51d31..d06458a 100644 (file)
@@ -8,6 +8,59 @@
 #include <linux/string.h>
 #include <linux/slab.h>
 
+static volatile int fortify_scratch_space;
+
+void lkdtm_FORTIFIED_OBJECT(void)
+{
+       struct target {
+               char a[10];
+       } target[2] = {};
+       /*
+        * Using volatile prevents the compiler from determining the value of
+        * 'size' at compile time. Without that, we would get a compile error
+        * rather than a runtime error.
+        */
+       volatile int size = 11;
+
+       pr_info("trying to read past the end of a struct\n");
+
+       /* Store result to global to prevent the code from being eliminated */
+       fortify_scratch_space = memcmp(&target[0], &target[1], size);
+
+       pr_err("FAIL: fortify did not block an object overread!\n");
+       pr_expected_config(CONFIG_FORTIFY_SOURCE);
+}
+
+void lkdtm_FORTIFIED_SUBOBJECT(void)
+{
+       struct target {
+               char a[10];
+               char b[10];
+       } target;
+       volatile int size = 20;
+       char *src;
+
+       src = kmalloc(size, GFP_KERNEL);
+       strscpy(src, "over ten bytes", size);
+       size = strlen(src) + 1;
+
+       pr_info("trying to strcpy past the end of a member of a struct\n");
+
+       /*
+        * memcpy(target.a, src, 20); will hit a compile error because the
+        * compiler knows at build time that target.a < 20 bytes. Use a
+        * volatile to force a runtime error.
+        */
+       memcpy(target.a, src, size);
+
+       /* Store result to global to prevent the code from being eliminated */
+       fortify_scratch_space = target.a[3];
+
+       pr_err("FAIL: fortify did not block an sub-object overrun!\n");
+       pr_expected_config(CONFIG_FORTIFY_SOURCE);
+
+       kfree(src);
+}
 
 /*
  * Calls fortified strscpy to test that it returns the same result as vanilla
index 6a30b60..f2e6158 100644 (file)
@@ -74,8 +74,6 @@ void lkdtm_STACK_GUARD_PAGE_TRAILING(void);
 void lkdtm_UNSET_SMEP(void);
 void lkdtm_DOUBLE_FAULT(void);
 void lkdtm_CORRUPT_PAC(void);
-void lkdtm_FORTIFY_OBJECT(void);
-void lkdtm_FORTIFY_SUBOBJECT(void);
 
 /* heap.c */
 void __init lkdtm_heap_init(void);
@@ -150,6 +148,8 @@ void lkdtm_STACKLEAK_ERASING(void);
 void lkdtm_CFI_FORWARD_PROTO(void);
 
 /* fortify.c */
+void lkdtm_FORTIFIED_OBJECT(void);
+void lkdtm_FORTIFIED_SUBOBJECT(void);
 void lkdtm_FORTIFIED_STRSCPY(void);
 
 /* powerpc.c */
index 6a33dbe..09f7bfa 100644 (file)
@@ -73,4 +73,6 @@ USERCOPY_KERNEL
 STACKLEAK_ERASING OK: the rest of the thread stack is properly erased
 CFI_FORWARD_PROTO
 FORTIFIED_STRSCPY
+FORTIFIED_OBJECT
+FORTIFIED_SUBOBJECT
 PPC_SLB_MULTIHIT Recovered