From 861dc0d7fd972f2064ff48b211955717163a11e0 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Sun, 12 Sep 2021 15:27:52 -0700 Subject: lkdtm: Note that lkdtm_kernel_info should be removed in the future As per Linus's request, remove lkdtm_kernel_info once sufficient reporting exists in CI systems: https://lore.kernel.org/lkml/CAHk-=wiFvfkoFixTapvvyPMN9pq5G-+Dys2eSyBa1vzDGAO5+A@mail.gmail.com Cc: Arnd Bergmann Cc: Greg Kroah-Hartman Signed-off-by: Kees Cook --- drivers/misc/lkdtm/core.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c index 609d9ee2acc0..d4c6cdced37b 100644 --- a/drivers/misc/lkdtm/core.c +++ b/drivers/misc/lkdtm/core.c @@ -212,7 +212,11 @@ module_param(cpoint_count, int, 0644); MODULE_PARM_DESC(cpoint_count, " Crash Point Count, number of times the "\ "crash point is to be hit to trigger action"); -/* For test debug reporting. */ +/* + * For test debug reporting when CI systems provide terse summaries. + * TODO: Remove this once reasonable reporting exists in most CI systems: + * https://lore.kernel.org/lkml/CAHk-=wiFvfkoFixTapvvyPMN9pq5G-+Dys2eSyBa1vzDGAO5+A@mail.gmail.com + */ char *lkdtm_kernel_info; /* Return the crashtype number or NULL if the name is invalid */ -- cgit v1.3-14-g43fede From 026c6fa1a525ca3f8a615052e45d766208989597 Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Thu, 7 Oct 2021 10:12:35 +0200 Subject: lkdtm: avoid printk() in recursive_loop() The recursive_loop() function is intended as a diagnostic to ensure that exhausting the stack is caught and mitigated. Currently, it uses pr_info() to ensure that the function has side effects that the compiler cannot simply optimize away, so that the stack footprint does not get reduced inadvertently. The typical mitigation for stack overflow is to kill the task, and this overflow may occur inside the call to pr_info(), which means it could be holding the console lock when this happens. This means that the console lock is never going to be released again, preventing the diagnostic prints related to the stack overflow handling from being visible on the console. So let's replace the call to pr_info() with a call to memzero_explicit(), which is not a 'magic' function name like memset() or memcpy(), which the compiler may replace with plain loads and stores. To ensure that the stack frames are nested rather than tail-called, put the call to memzero_explicit() after the recursive call. Signed-off-by: Ard Biesheuvel Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/20211007081235.382697-1-ardb@kernel.org --- drivers/misc/lkdtm/bugs.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c index f4cb94a9aa9c..f21854ac5cc2 100644 --- a/drivers/misc/lkdtm/bugs.c +++ b/drivers/misc/lkdtm/bugs.c @@ -41,20 +41,22 @@ static DEFINE_SPINLOCK(lock_me_up); * Make sure compiler does not optimize this function or stack frame away: * - function marked noinline * - stack variables are marked volatile - * - stack variables are written (memset()) and read (pr_info()) - * - function has external effects (pr_info()) - * */ + * - stack variables are written (memset()) and read (buf[..] passed as arg) + * - function may have external effects (memzero_explicit()) + * - no tail recursion possible + */ static int noinline recursive_loop(int remaining) { volatile char buf[REC_STACK_SIZE]; + volatile int ret; memset((void *)buf, remaining & 0xFF, sizeof(buf)); - pr_info("loop %d/%d ...\n", (int)buf[remaining % sizeof(buf)], - recur_count); if (!remaining) - return 0; + ret = 0; else - return recursive_loop(remaining - 1); + ret = recursive_loop((int)buf[remaining % sizeof(buf)] - 1); + memzero_explicit((void *)buf, sizeof(buf)); + return ret; } /* If the depth is negative, use the default, otherwise keep parameter. */ -- cgit v1.3-14-g43fede From bc93a22a19eb2b68a16ecf04cdf4b2ed65aaf398 Mon Sep 17 00:00:00 2001 From: Christophe Leroy Date: Fri, 8 Oct 2021 18:58:40 +0200 Subject: lkdtm: Fix content of section containing lkdtm_rodata_do_nothing() On a kernel without CONFIG_STRICT_KERNEL_RWX, running EXEC_RODATA test leads to "Illegal instruction" failure. Looking at the content of rodata_objcopy.o, we see that the function content zeroes only: Disassembly of section .rodata: 0000000000000000 <.lkdtm_rodata_do_nothing>: 0: 00 00 00 00 .long 0x0 Add the contents flag in order to keep the content of the section while renaming it. Disassembly of section .rodata: 0000000000000000 <.lkdtm_rodata_do_nothing>: 0: 4e 80 00 20 blr Fixes: e9e08a07385e ("lkdtm: support llvm-objcopy") Cc: stable@vger.kernel.org Cc: Kees Cook Cc: Arnd Bergmann Cc: Greg Kroah-Hartman Cc: Nick Desaulniers Cc: Nathan Chancellor Signed-off-by: Christophe Leroy Reviewed-by: Nick Desaulniers Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/8900731fbc05fb8b0de18af7133a8fc07c3c53a1.1633712176.git.christophe.leroy@csgroup.eu --- drivers/misc/lkdtm/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile index aa12097668d3..e2984ce51fe4 100644 --- a/drivers/misc/lkdtm/Makefile +++ b/drivers/misc/lkdtm/Makefile @@ -20,7 +20,7 @@ CFLAGS_REMOVE_rodata.o += $(CC_FLAGS_LTO) OBJCOPYFLAGS := OBJCOPYFLAGS_rodata_objcopy.o := \ - --rename-section .noinstr.text=.rodata,alloc,readonly,load + --rename-section .noinstr.text=.rodata,alloc,readonly,load,contents targets += rodata.o rodata_objcopy.o $(obj)/rodata_objcopy.o: $(obj)/rodata.o FORCE $(call if_changed,objcopy) -- cgit v1.3-14-g43fede From 90091c367e74d5b58d9ebe979cc363f7468f58d3 Mon Sep 17 00:00:00 2001 From: Misono Tomohiro Date: Thu, 5 Aug 2021 19:12:36 +0900 Subject: selftest/lkdtm: Skip stack-entropy test if lkdtm is not available Exit with return code 4 if lkdtm is not available like other tests in order to properly skip the test. Signed-off-by: Misono Tomohiro Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/20210805101236.1140381-1-misono.tomohiro@jp.fujitsu.com --- tools/testing/selftests/lkdtm/stack-entropy.sh | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/lkdtm/stack-entropy.sh b/tools/testing/selftests/lkdtm/stack-entropy.sh index 1b4d95d575f8..14fedeef762e 100755 --- a/tools/testing/selftests/lkdtm/stack-entropy.sh +++ b/tools/testing/selftests/lkdtm/stack-entropy.sh @@ -4,13 +4,27 @@ # Measure kernel stack entropy by sampling via LKDTM's REPORT_STACK test. set -e samples="${1:-1000}" +TRIGGER=/sys/kernel/debug/provoke-crash/DIRECT +KSELFTEST_SKIP_TEST=4 + +# Verify we have LKDTM available in the kernel. +if [ ! -r $TRIGGER ] ; then + /sbin/modprobe -q lkdtm || true + if [ ! -r $TRIGGER ] ; then + echo "Cannot find $TRIGGER (missing CONFIG_LKDTM?)" + else + echo "Cannot write $TRIGGER (need to run as root?)" + fi + # Skip this test + exit $KSELFTEST_SKIP_TEST +fi # Capture dmesg continuously since it may fill up depending on sample size. log=$(mktemp -t stack-entropy-XXXXXX) dmesg --follow >"$log" & pid=$! report=-1 for i in $(seq 1 $samples); do - echo "REPORT_STACK" >/sys/kernel/debug/provoke-crash/DIRECT + echo "REPORT_STACK" > $TRIGGER if [ -t 1 ]; then percent=$(( 100 * $i / $samples )) if [ "$percent" -ne "$report" ]; then -- cgit v1.3-14-g43fede