drivers/misc/lkdtm: add new file in LKDTM to test fortified strscpy

This new test ensures that fortified strscpy has the same behavior than
vanilla strscpy (e.g.  returning -E2BIG when src content is truncated).
Finally, it generates a crash at runtime because there is a write overflow
in destination string.

Link: https://lkml.kernel.org/r/20201122162451.27551-5-laniel_francis@privacyrequired.com
Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Cc: Daniel Axtens <dja@axtens.net>
Cc: Daniel Micay <danielmicay@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile
index 1ef7888a12b52d4ebe0d38c5861758d802afbc59..7727bfd32be9e5479672f8808aac33110880fbab 100644 (file)
--- a/drivers/misc/lkdtm/Makefile
+++ b/drivers/misc/lkdtm/Makefile
@@ -10,6 +10,7 @@ lkdtm-$(CONFIG_LKDTM)         += rodata_objcopy.o
 lkdtm-$(CONFIG_LKDTM)          += usercopy.o
 lkdtm-$(CONFIG_LKDTM)          += stackleak.o
 lkdtm-$(CONFIG_LKDTM)          += cfi.o
+lkdtm-$(CONFIG_LKDTM)          += fortify.o
 
 KASAN_SANITIZE_rodata.o                := n
 KASAN_SANITIZE_stackleak.o     := n

diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
index b8c51a633fcc8e8a229ea1037df2ef54fc0f471f..3c0a67f072c056843fa217c9a0554247281b738a 100644 (file)
--- a/drivers/misc/lkdtm/core.c
+++ b/drivers/misc/lkdtm/core.c
@@ -175,6 +175,7 @@ static const struct crashtype crashtypes[] = {
        CRASHTYPE(USERCOPY_KERNEL),
        CRASHTYPE(STACKLEAK_ERASING),
        CRASHTYPE(CFI_FORWARD_PROTO),
+       CRASHTYPE(FORTIFIED_STRSCPY),
 #ifdef CONFIG_X86_32
        CRASHTYPE(DOUBLE_FAULT),
 #endif

diff --git a/drivers/misc/lkdtm/fortify.c b/drivers/misc/lkdtm/fortify.c
new file mode 100644 (file)
index 0000000..faf29cf
--- /dev/null
+++ b/drivers/misc/lkdtm/fortify.c
@@ -0,0 +1,82 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020 Francis Laniel <laniel_francis@privacyrequired.com>
+ *
+ * Add tests related to fortified functions in this file.
+ */
+#include "lkdtm.h"
+#include <linux/string.h>
+#include <linux/slab.h>
+
+
+/*
+ * Calls fortified strscpy to test that it returns the same result as vanilla
+ * strscpy and generate a panic because there is a write overflow (i.e. src
+ * length is greater than dst length).
+ */
+void lkdtm_FORTIFIED_STRSCPY(void)
+{
+       char *src;
+       char dst[5];
+
+       struct {
+               union {
+                       char big[10];
+                       char src[5];
+               };
+       } weird = { .big = "hello!" };
+       char weird_dst[sizeof(weird.src) + 1];
+
+       src = kstrdup("foobar", GFP_KERNEL);
+
+       if (src == NULL)
+               return;
+
+       /* Vanilla strscpy returns -E2BIG if size is 0. */
+       if (strscpy(dst, src, 0) != -E2BIG)
+               pr_warn("FAIL: strscpy() of 0 length did not return -E2BIG\n");
+
+       /* Vanilla strscpy returns -E2BIG if src is truncated. */
+       if (strscpy(dst, src, sizeof(dst)) != -E2BIG)
+               pr_warn("FAIL: strscpy() did not return -E2BIG while src is truncated\n");
+
+       /* After above call, dst must contain "foob" because src was truncated. */
+       if (strncmp(dst, "foob", sizeof(dst)) != 0)
+               pr_warn("FAIL: after strscpy() dst does not contain \"foob\" but \"%s\"\n",
+                       dst);
+
+       /* Shrink src so the strscpy() below succeeds. */
+       src[3] = '\0';
+
+       /*
+        * Vanilla strscpy returns number of character copied if everything goes
+        * well.
+        */
+       if (strscpy(dst, src, sizeof(dst)) != 3)
+               pr_warn("FAIL: strscpy() did not return 3 while src was copied entirely truncated\n");
+
+       /* After above call, dst must contain "foo" because src was copied. */
+       if (strncmp(dst, "foo", sizeof(dst)) != 0)
+               pr_warn("FAIL: after strscpy() dst does not contain \"foo\" but \"%s\"\n",
+                       dst);
+
+       /* Test when src is embedded inside a union. */
+       strscpy(weird_dst, weird.src, sizeof(weird_dst));
+
+       if (strcmp(weird_dst, "hello") != 0)
+               pr_warn("FAIL: after strscpy() weird_dst does not contain \"hello\" but \"%s\"\n",
+                       weird_dst);
+
+       /* Restore src to its initial value. */
+       src[3] = 'b';
+
+       /*
+        * Use strlen here so size cannot be known at compile time and there is
+        * a runtime write overflow.
+        */
+       strscpy(dst, src, strlen(src));
+
+       pr_warn("FAIL: No overflow in above strscpy()\n");
+
+       kfree(src);
+}

diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
index 49e6b945feb7ec668d32ce971ed6a63b8134a28a..138f06254b61842bdbbf1458acb1ad6a8518c7a1 100644 (file)
--- a/drivers/misc/lkdtm/lkdtm.h
+++ b/drivers/misc/lkdtm/lkdtm.h
@@ -104,4 +104,7 @@ void lkdtm_STACKLEAK_ERASING(void);
 /* cfi.c */
 void lkdtm_CFI_FORWARD_PROTO(void);
 
+/* fortify.c */
+void lkdtm_FORTIFIED_STRSCPY(void);
+
 #endif

diff --git a/tools/testing/selftests/lkdtm/tests.txt b/tools/testing/selftests/lkdtm/tests.txt
index 74a8d329a72c80cd4ab1590d3a95377ed30438f1..92ba4cc413140a076d1094fc2bde53aabfead41b 100644 (file)
--- a/tools/testing/selftests/lkdtm/tests.txt
+++ b/tools/testing/selftests/lkdtm/tests.txt
@@ -68,3 +68,4 @@ USERCOPY_STACK_BEYOND
 USERCOPY_KERNEL
 STACKLEAK_ERASING OK: the rest of the thread stack is properly erased
 CFI_FORWARD_PROTO
+FORTIFIED_STRSCPY