* [PATCH 1/4] lkdtm/bugs: Add ARRAY_BOUNDS to selftests
2021-08-18 17:48 [PATCH 0/4] LKDTM: Various CI improvements Kees Cook
@ 2021-08-18 17:48 ` Kees Cook
2021-08-18 17:48 ` [PATCH 2/4] lkdtm/fortify: Consolidate FORTIFY_SOURCE tests Kees Cook
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Kees Cook @ 2021-08-18 17:48 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Kees Cook, kernelci, Guillaume Tucker, Arnd Bergmann,
linux-kernel, linux-kselftest, linux-hardening
Add CONFIG hints about why the ARRAY_BOUNDS test might fail, and
similarly include the CONFIGs needed to pass the ARRAY_BOUNDS test via
the selftests, and add to selftests.
Cc: kernelci@groups.io
Suggested-by: Guillaume Tucker <guillaume.tucker@collabora.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/misc/lkdtm/bugs.c | 1 +
tools/testing/selftests/lkdtm/config | 2 ++
tools/testing/selftests/lkdtm/tests.txt | 1 +
3 files changed, 4 insertions(+)
diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c
index 88c218a9f8b3..03171e412356 100644
--- a/drivers/misc/lkdtm/bugs.c
+++ b/drivers/misc/lkdtm/bugs.c
@@ -267,6 +267,7 @@ void lkdtm_ARRAY_BOUNDS(void)
kfree(not_checked);
kfree(checked);
pr_err("FAIL: survived array bounds overflow!\n");
+ pr_expected_config(CONFIG_UBSAN_BOUNDS);
}
void lkdtm_CORRUPT_LIST_ADD(void)
diff --git a/tools/testing/selftests/lkdtm/config b/tools/testing/selftests/lkdtm/config
index 013446e87f1f..38edea25631b 100644
--- a/tools/testing/selftests/lkdtm/config
+++ b/tools/testing/selftests/lkdtm/config
@@ -6,3 +6,5 @@ CONFIG_HARDENED_USERCOPY=y
# CONFIG_HARDENED_USERCOPY_FALLBACK is not set
CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT=y
CONFIG_INIT_ON_ALLOC_DEFAULT_ON=y
+CONFIG_UBSAN_BOUNDS=y
+CONFIG_UBSAN_TRAP=y
diff --git a/tools/testing/selftests/lkdtm/tests.txt b/tools/testing/selftests/lkdtm/tests.txt
index 846cfd508d3c..6a33dbea8491 100644
--- a/tools/testing/selftests/lkdtm/tests.txt
+++ b/tools/testing/selftests/lkdtm/tests.txt
@@ -7,6 +7,7 @@ EXCEPTION
#EXHAUST_STACK Corrupts memory on failure
#CORRUPT_STACK Crashes entire system on success
#CORRUPT_STACK_STRONG Crashes entire system on success
+ARRAY_BOUNDS
CORRUPT_LIST_ADD list_add corruption
CORRUPT_LIST_DEL list_del corruption
STACK_GUARD_PAGE_LEADING
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/4] lkdtm/fortify: Consolidate FORTIFY_SOURCE tests
2021-08-18 17:48 [PATCH 0/4] LKDTM: Various CI improvements Kees Cook
2021-08-18 17:48 ` [PATCH 1/4] lkdtm/bugs: Add ARRAY_BOUNDS to selftests Kees Cook
@ 2021-08-18 17:48 ` Kees Cook
2021-08-18 17:48 ` [PATCH 3/4] lkdtm: Add kernel version to failure hints Kees Cook
2021-08-18 17:48 ` [PATCH 4/4] lkdtm/heap: Avoid __alloc_size hint warning for VMALLOC_LINEAR_OVERFLOW Kees Cook
3 siblings, 0 replies; 5+ messages in thread
From: Kees Cook @ 2021-08-18 17:48 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Kees Cook, Arnd Bergmann, Guillaume Tucker, linux-kernel,
kernelci, linux-kselftest, linux-hardening
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>
---
drivers/misc/lkdtm/bugs.c | 50 -----------------------
drivers/misc/lkdtm/core.c | 4 +-
drivers/misc/lkdtm/fortify.c | 53 +++++++++++++++++++++++++
drivers/misc/lkdtm/lkdtm.h | 4 +-
tools/testing/selftests/lkdtm/tests.txt | 2 +
5 files changed, 59 insertions(+), 54 deletions(-)
diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c
index 03171e412356..4282b625200f 100644
--- a/drivers/misc/lkdtm/bugs.c
+++ b/drivers/misc/lkdtm/bugs.c
@@ -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);
-}
diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
index 9dda87c6b54a..dbdb0af7f68a 100644
--- a/drivers/misc/lkdtm/core.c
+++ b/drivers/misc/lkdtm/core.c
@@ -119,8 +119,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),
@@ -180,6 +178,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
diff --git a/drivers/misc/lkdtm/fortify.c b/drivers/misc/lkdtm/fortify.c
index 0f51d31b57ca..d06458a4858e 100644
--- a/drivers/misc/lkdtm/fortify.c
+++ b/drivers/misc/lkdtm/fortify.c
@@ -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
diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
index 6a30b60519f3..f2e61581c1ae 100644
--- a/drivers/misc/lkdtm/lkdtm.h
+++ b/drivers/misc/lkdtm/lkdtm.h
@@ -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 */
diff --git a/tools/testing/selftests/lkdtm/tests.txt b/tools/testing/selftests/lkdtm/tests.txt
index 6a33dbea8491..09f7bfa383cc 100644
--- a/tools/testing/selftests/lkdtm/tests.txt
+++ b/tools/testing/selftests/lkdtm/tests.txt
@@ -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
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/4] lkdtm: Add kernel version to failure hints
2021-08-18 17:48 [PATCH 0/4] LKDTM: Various CI improvements Kees Cook
2021-08-18 17:48 ` [PATCH 1/4] lkdtm/bugs: Add ARRAY_BOUNDS to selftests Kees Cook
2021-08-18 17:48 ` [PATCH 2/4] lkdtm/fortify: Consolidate FORTIFY_SOURCE tests Kees Cook
@ 2021-08-18 17:48 ` Kees Cook
2021-08-18 17:48 ` [PATCH 4/4] lkdtm/heap: Avoid __alloc_size hint warning for VMALLOC_LINEAR_OVERFLOW Kees Cook
3 siblings, 0 replies; 5+ messages in thread
From: Kees Cook @ 2021-08-18 17:48 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Kees Cook, Arnd Bergmann, kernelci, Guillaume Tucker,
linux-kernel, linux-kselftest, linux-hardening
In an effort to keep as much information in once place as possible in
CI logs, report the kernel version and architecture in the failure hints.
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: kernelci@groups.io
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/misc/lkdtm/lkdtm.h | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
index f2e61581c1ae..d7d64d9765eb 100644
--- a/drivers/misc/lkdtm/lkdtm.h
+++ b/drivers/misc/lkdtm/lkdtm.h
@@ -5,13 +5,17 @@
#define pr_fmt(fmt) "lkdtm: " fmt
#include <linux/kernel.h>
+#include <generated/compile.h>
+#include <generated/utsrelease.h>
+
+#define LKDTM_KERNEL "kernel (" UTS_RELEASE " " UTS_MACHINE ")"
#define pr_expected_config(kconfig) \
{ \
if (IS_ENABLED(kconfig)) \
- pr_err("Unexpected! This kernel was built with " #kconfig "=y\n"); \
+ pr_err("Unexpected! This " LKDTM_KERNEL " was built with " #kconfig "=y\n"); \
else \
- pr_warn("This is probably expected, since this kernel was built *without* " #kconfig "=y\n"); \
+ pr_warn("This is probably expected, since this " LKDTM_KERNEL " was built *without* " #kconfig "=y\n"); \
}
#ifndef MODULE
@@ -21,24 +25,24 @@ int lkdtm_check_bool_cmdline(const char *param);
if (IS_ENABLED(kconfig)) { \
switch (lkdtm_check_bool_cmdline(param)) { \
case 0: \
- pr_warn("This is probably expected, since this kernel was built with " #kconfig "=y but booted with '" param "=N'\n"); \
+ pr_warn("This is probably expected, since this " LKDTM_KERNEL " was built with " #kconfig "=y but booted with '" param "=N'\n"); \
break; \
case 1: \
- pr_err("Unexpected! This kernel was built with " #kconfig "=y and booted with '" param "=Y'\n"); \
+ pr_err("Unexpected! This " LKDTM_KERNEL " was built with " #kconfig "=y and booted with '" param "=Y'\n"); \
break; \
default: \
- pr_err("Unexpected! This kernel was built with " #kconfig "=y (and booted without '" param "' specified)\n"); \
+ pr_err("Unexpected! This " LKDTM_KERNEL " was built with " #kconfig "=y (and booted without '" param "' specified)\n"); \
} \
} else { \
switch (lkdtm_check_bool_cmdline(param)) { \
case 0: \
- pr_warn("This is probably expected, as kernel was built *without* " #kconfig "=y and booted with '" param "=N'\n"); \
+ pr_warn("This is probably expected, as this " LKDTM_KERNEL " was built *without* " #kconfig "=y and booted with '" param "=N'\n"); \
break; \
case 1: \
- pr_err("Unexpected! This kernel was built *without* " #kconfig "=y but booted with '" param "=Y'\n"); \
+ pr_err("Unexpected! This " LKDTM_KERNEL " was built *without* " #kconfig "=y but booted with '" param "=Y'\n"); \
break; \
default: \
- pr_err("This is probably expected, since this kernel was built *without* " #kconfig "=y (and booted without '" param "' specified)\n"); \
+ pr_err("This is probably expected, since this " LKDTM_KERNEL " was built *without* " #kconfig "=y (and booted without '" param "' specified)\n"); \
break; \
} \
} \
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 4/4] lkdtm/heap: Avoid __alloc_size hint warning for VMALLOC_LINEAR_OVERFLOW
2021-08-18 17:48 [PATCH 0/4] LKDTM: Various CI improvements Kees Cook
` (2 preceding siblings ...)
2021-08-18 17:48 ` [PATCH 3/4] lkdtm: Add kernel version to failure hints Kees Cook
@ 2021-08-18 17:48 ` Kees Cook
3 siblings, 0 replies; 5+ messages in thread
From: Kees Cook @ 2021-08-18 17:48 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Kees Cook, Arnd Bergmann, Guillaume Tucker, linux-kernel,
kernelci, linux-kselftest, linux-hardening
Once __alloc_size hints have been added, the compiler will (correctly!)
see this as an overflow. We are, however, trying to test for this
condition at run-time (not compile-time), so work around it with a
volatile int offset.
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/misc/lkdtm/heap.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/misc/lkdtm/heap.c b/drivers/misc/lkdtm/heap.c
index 3d9aae5821a0..8a92f5a800fa 100644
--- a/drivers/misc/lkdtm/heap.c
+++ b/drivers/misc/lkdtm/heap.c
@@ -12,6 +12,13 @@ static struct kmem_cache *double_free_cache;
static struct kmem_cache *a_cache;
static struct kmem_cache *b_cache;
+/*
+ * Using volatile here means the compiler cannot ever make assumptions
+ * about this value. This means compile-time length checks involving
+ * this variable cannot be performed; only run-time checks.
+ */
+static volatile int __offset = 1;
+
/*
* If there aren't guard pages, it's likely that a consecutive allocation will
* let us overflow into the second allocation without overwriting something real.
@@ -24,7 +31,7 @@ void lkdtm_VMALLOC_LINEAR_OVERFLOW(void)
two = vzalloc(PAGE_SIZE);
pr_info("Attempting vmalloc linear overflow ...\n");
- memset(one, 0xAA, PAGE_SIZE + 1);
+ memset(one, 0xAA, PAGE_SIZE + __offset);
vfree(two);
vfree(one);
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread