* [PATCH drivers/misc 0/4] lkdtm: Various clean ups
@ 2020-05-29 20:03 Kees Cook
2020-05-29 20:03 ` [PATCH 1/4] lkdtm: Avoid more compiler optimizations for bad writes Kees Cook
` (4 more replies)
0 siblings, 5 replies; 21+ messages in thread
From: Kees Cook @ 2020-05-29 20:03 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Kees Cook, Prasad Sodagudi, Sami Tolvanen, Amit Daniel Kachhap,
linux-kselftest, clang-built-linux, linux-kernel
Hi Greg,
Can you please apply these patches to your drivers/misc tree for LKDTM?
It's mostly a collection of fixes and improvements and tweaks to the
selftest integration.
Thanks!
-Kees
Kees Cook (4):
lkdtm: Avoid more compiler optimizations for bad writes
lkdtm/heap: Avoid edge and middle of slabs
selftests/lkdtm: Reset WARN_ONCE to avoid false negatives
lkdtm: Make arch-specific tests always available
drivers/misc/lkdtm/bugs.c | 45 +++++++++++++------------
drivers/misc/lkdtm/heap.c | 9 ++---
drivers/misc/lkdtm/lkdtm.h | 2 --
drivers/misc/lkdtm/perms.c | 22 ++++++++----
drivers/misc/lkdtm/usercopy.c | 7 ++--
tools/testing/selftests/lkdtm/run.sh | 6 ++++
tools/testing/selftests/lkdtm/tests.txt | 1 +
7 files changed, 56 insertions(+), 36 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCH 1/4] lkdtm: Avoid more compiler optimizations for bad writes 2020-05-29 20:03 [PATCH drivers/misc 0/4] lkdtm: Various clean ups Kees Cook @ 2020-05-29 20:03 ` Kees Cook 2020-05-29 20:10 ` Nick Desaulniers 2020-05-29 20:03 ` [PATCH 2/4] lkdtm/heap: Avoid edge and middle of slabs Kees Cook ` (3 subsequent siblings) 4 siblings, 1 reply; 21+ messages in thread From: Kees Cook @ 2020-05-29 20:03 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Kees Cook, Prasad Sodagudi, Sami Tolvanen, Amit Daniel Kachhap, linux-kselftest, clang-built-linux, linux-kernel It seems at least Clang is able to throw away writes it knows are destined for read-only memory, which makes things like the WRITE_RO test fail, as the write gets elided. Instead, force the variable to be volatile, and make similar changes through-out other tests in an effort to avoid needing to repeat fixing these kinds of problems. Also includes pr_err() calls in failure paths so that kernel logs are more clear in the failure case. Reported-by: Prasad Sodagudi <psodagud@codeaurora.org> Suggested-by: Sami Tolvanen <samitolvanen@google.com> Signed-off-by: Kees Cook <keescook@chromium.org> --- drivers/misc/lkdtm/bugs.c | 11 +++++------ drivers/misc/lkdtm/perms.c | 22 +++++++++++++++------- drivers/misc/lkdtm/usercopy.c | 7 +++++-- 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c index 886459e0ddd9..e1b43f615549 100644 --- a/drivers/misc/lkdtm/bugs.c +++ b/drivers/misc/lkdtm/bugs.c @@ -118,9 +118,8 @@ noinline void lkdtm_CORRUPT_STACK(void) /* Use default char array length that triggers stack protection. */ char data[8] __aligned(sizeof(void *)); - __lkdtm_CORRUPT_STACK(&data); - - pr_info("Corrupted stack containing char array ...\n"); + pr_info("Corrupting stack containing char array ...\n"); + __lkdtm_CORRUPT_STACK((void *)&data); } /* Same as above but will only get a canary with -fstack-protector-strong */ @@ -131,9 +130,8 @@ noinline void lkdtm_CORRUPT_STACK_STRONG(void) unsigned long *ptr; } data __aligned(sizeof(void *)); - __lkdtm_CORRUPT_STACK(&data); - - pr_info("Corrupted stack containing union ...\n"); + pr_info("Corrupting stack containing union ...\n"); + __lkdtm_CORRUPT_STACK((void *)&data); } void lkdtm_UNALIGNED_LOAD_STORE_WRITE(void) @@ -248,6 +246,7 @@ void lkdtm_ARRAY_BOUNDS(void) kfree(not_checked); kfree(checked); + pr_err("FAIL: survived array bounds overflow!\n"); } void lkdtm_CORRUPT_LIST_ADD(void) diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c index 62f76d506f04..2dede2ef658f 100644 --- a/drivers/misc/lkdtm/perms.c +++ b/drivers/misc/lkdtm/perms.c @@ -57,6 +57,7 @@ static noinline void execute_location(void *dst, bool write) } pr_info("attempting bad execution at %px\n", func); func(); + pr_err("FAIL: func returned\n"); } static void execute_user_location(void *dst) @@ -75,20 +76,22 @@ static void execute_user_location(void *dst) return; pr_info("attempting bad execution at %px\n", func); func(); + pr_err("FAIL: func returned\n"); } void lkdtm_WRITE_RO(void) { - /* Explicitly cast away "const" for the test. */ - unsigned long *ptr = (unsigned long *)&rodata; + /* Explicitly cast away "const" for the test and make volatile. */ + volatile unsigned long *ptr = (unsigned long *)&rodata; pr_info("attempting bad rodata write at %px\n", ptr); *ptr ^= 0xabcd1234; + pr_err("FAIL: survived bad write\n"); } void lkdtm_WRITE_RO_AFTER_INIT(void) { - unsigned long *ptr = &ro_after_init; + volatile unsigned long *ptr = &ro_after_init; /* * Verify we were written to during init. Since an Oops @@ -102,19 +105,21 @@ void lkdtm_WRITE_RO_AFTER_INIT(void) pr_info("attempting bad ro_after_init write at %px\n", ptr); *ptr ^= 0xabcd1234; + pr_err("FAIL: survived bad write\n"); } void lkdtm_WRITE_KERN(void) { size_t size; - unsigned char *ptr; + volatile unsigned char *ptr; size = (unsigned long)do_overwritten - (unsigned long)do_nothing; ptr = (unsigned char *)do_overwritten; pr_info("attempting bad %zu byte write at %px\n", size, ptr); - memcpy(ptr, (unsigned char *)do_nothing, size); + memcpy((void *)ptr, (unsigned char *)do_nothing, size); flush_icache_range((unsigned long)ptr, (unsigned long)(ptr + size)); + pr_err("FAIL: survived bad write\n"); do_overwritten(); } @@ -193,9 +198,11 @@ void lkdtm_ACCESS_USERSPACE(void) pr_info("attempting bad read at %px\n", ptr); tmp = *ptr; tmp += 0xc0dec0de; + pr_err("FAIL: survived bad read\n"); pr_info("attempting bad write at %px\n", ptr); *ptr = tmp; + pr_err("FAIL: survived bad write\n"); vm_munmap(user_addr, PAGE_SIZE); } @@ -203,19 +210,20 @@ void lkdtm_ACCESS_USERSPACE(void) void lkdtm_ACCESS_NULL(void) { unsigned long tmp; - unsigned long *ptr = (unsigned long *)NULL; + volatile unsigned long *ptr = (unsigned long *)NULL; pr_info("attempting bad read at %px\n", ptr); tmp = *ptr; tmp += 0xc0dec0de; + pr_err("FAIL: survived bad read\n"); pr_info("attempting bad write at %px\n", ptr); *ptr = tmp; + pr_err("FAIL: survived bad write\n"); } void __init lkdtm_perms_init(void) { /* Make sure we can write to __ro_after_init values during __init */ ro_after_init |= 0xAA; - } diff --git a/drivers/misc/lkdtm/usercopy.c b/drivers/misc/lkdtm/usercopy.c index e172719dd86d..b833367a45d0 100644 --- a/drivers/misc/lkdtm/usercopy.c +++ b/drivers/misc/lkdtm/usercopy.c @@ -304,19 +304,22 @@ void lkdtm_USERCOPY_KERNEL(void) return; } - pr_info("attempting good copy_to_user from kernel rodata\n"); + pr_info("attempting good copy_to_user from kernel rodata: %px\n", + test_text); if (copy_to_user((void __user *)user_addr, test_text, unconst + sizeof(test_text))) { pr_warn("copy_to_user failed unexpectedly?!\n"); goto free_user; } - pr_info("attempting bad copy_to_user from kernel text\n"); + pr_info("attempting bad copy_to_user from kernel text: %px\n", + vm_mmap); if (copy_to_user((void __user *)user_addr, vm_mmap, unconst + PAGE_SIZE)) { pr_warn("copy_to_user failed, but lacked Oops\n"); goto free_user; } + pr_err("FAIL: survived bad copy_to_user()\n"); free_user: vm_munmap(user_addr, PAGE_SIZE); -- 2.25.1 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 1/4] lkdtm: Avoid more compiler optimizations for bad writes 2020-05-29 20:03 ` [PATCH 1/4] lkdtm: Avoid more compiler optimizations for bad writes Kees Cook @ 2020-05-29 20:10 ` Nick Desaulniers 0 siblings, 0 replies; 21+ messages in thread From: Nick Desaulniers @ 2020-05-29 20:10 UTC (permalink / raw) To: Kees Cook Cc: Greg Kroah-Hartman, Prasad Sodagudi, Sami Tolvanen, Amit Daniel Kachhap, open list:KERNEL SELFTEST FRAMEWORK, clang-built-linux, LKML On Fri, May 29, 2020 at 1:03 PM Kees Cook <keescook@chromium.org> wrote: > > It seems at least Clang is able to throw away writes it knows are > destined for read-only memory, which makes things like the WRITE_RO test > fail, as the write gets elided. Instead, force the variable to be Heh, yep. I recall the exact patch in LLVM causing build breakages for kernels and various parts of Android userspace within the past year, for code that tried to write to variables declared const through casts that removed the const. (Was the last patch for us to build MIPS IIRC). Doing so is explicitly UB. I did feel that that particular "optimization" was very specific to C/C++, and should not have been performed in LLVM (which should be more agnostic to the front end language's wacky rules, IMO) but rather Clang (which doesn't do much C/C++ language specific optimizations currently, though there are rough plans forming to change that). > volatile, and make similar changes through-out other tests in an effort > to avoid needing to repeat fixing these kinds of problems. Also includes > pr_err() calls in failure paths so that kernel logs are more clear in > the failure case. > > Reported-by: Prasad Sodagudi <psodagud@codeaurora.org> > Suggested-by: Sami Tolvanen <samitolvanen@google.com> > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > drivers/misc/lkdtm/bugs.c | 11 +++++------ > drivers/misc/lkdtm/perms.c | 22 +++++++++++++++------- > drivers/misc/lkdtm/usercopy.c | 7 +++++-- > 3 files changed, 25 insertions(+), 15 deletions(-) > > diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c > index 886459e0ddd9..e1b43f615549 100644 > --- a/drivers/misc/lkdtm/bugs.c > +++ b/drivers/misc/lkdtm/bugs.c > @@ -118,9 +118,8 @@ noinline void lkdtm_CORRUPT_STACK(void) > /* Use default char array length that triggers stack protection. */ > char data[8] __aligned(sizeof(void *)); > > - __lkdtm_CORRUPT_STACK(&data); > - > - pr_info("Corrupted stack containing char array ...\n"); > + pr_info("Corrupting stack containing char array ...\n"); > + __lkdtm_CORRUPT_STACK((void *)&data); > } > > /* Same as above but will only get a canary with -fstack-protector-strong */ > @@ -131,9 +130,8 @@ noinline void lkdtm_CORRUPT_STACK_STRONG(void) > unsigned long *ptr; > } data __aligned(sizeof(void *)); > > - __lkdtm_CORRUPT_STACK(&data); > - > - pr_info("Corrupted stack containing union ...\n"); > + pr_info("Corrupting stack containing union ...\n"); > + __lkdtm_CORRUPT_STACK((void *)&data); > } > > void lkdtm_UNALIGNED_LOAD_STORE_WRITE(void) > @@ -248,6 +246,7 @@ void lkdtm_ARRAY_BOUNDS(void) > > kfree(not_checked); > kfree(checked); > + pr_err("FAIL: survived array bounds overflow!\n"); > } > > void lkdtm_CORRUPT_LIST_ADD(void) > diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c > index 62f76d506f04..2dede2ef658f 100644 > --- a/drivers/misc/lkdtm/perms.c > +++ b/drivers/misc/lkdtm/perms.c > @@ -57,6 +57,7 @@ static noinline void execute_location(void *dst, bool write) > } > pr_info("attempting bad execution at %px\n", func); > func(); > + pr_err("FAIL: func returned\n"); > } > > static void execute_user_location(void *dst) > @@ -75,20 +76,22 @@ static void execute_user_location(void *dst) > return; > pr_info("attempting bad execution at %px\n", func); > func(); > + pr_err("FAIL: func returned\n"); > } > > void lkdtm_WRITE_RO(void) > { > - /* Explicitly cast away "const" for the test. */ > - unsigned long *ptr = (unsigned long *)&rodata; > + /* Explicitly cast away "const" for the test and make volatile. */ > + volatile unsigned long *ptr = (unsigned long *)&rodata; > > pr_info("attempting bad rodata write at %px\n", ptr); > *ptr ^= 0xabcd1234; > + pr_err("FAIL: survived bad write\n"); > } > > void lkdtm_WRITE_RO_AFTER_INIT(void) > { > - unsigned long *ptr = &ro_after_init; > + volatile unsigned long *ptr = &ro_after_init; > > /* > * Verify we were written to during init. Since an Oops > @@ -102,19 +105,21 @@ void lkdtm_WRITE_RO_AFTER_INIT(void) > > pr_info("attempting bad ro_after_init write at %px\n", ptr); > *ptr ^= 0xabcd1234; > + pr_err("FAIL: survived bad write\n"); > } > > void lkdtm_WRITE_KERN(void) > { > size_t size; > - unsigned char *ptr; > + volatile unsigned char *ptr; > > size = (unsigned long)do_overwritten - (unsigned long)do_nothing; > ptr = (unsigned char *)do_overwritten; > > pr_info("attempting bad %zu byte write at %px\n", size, ptr); > - memcpy(ptr, (unsigned char *)do_nothing, size); > + memcpy((void *)ptr, (unsigned char *)do_nothing, size); > flush_icache_range((unsigned long)ptr, (unsigned long)(ptr + size)); > + pr_err("FAIL: survived bad write\n"); > > do_overwritten(); > } > @@ -193,9 +198,11 @@ void lkdtm_ACCESS_USERSPACE(void) > pr_info("attempting bad read at %px\n", ptr); > tmp = *ptr; > tmp += 0xc0dec0de; > + pr_err("FAIL: survived bad read\n"); > > pr_info("attempting bad write at %px\n", ptr); > *ptr = tmp; > + pr_err("FAIL: survived bad write\n"); > > vm_munmap(user_addr, PAGE_SIZE); > } > @@ -203,19 +210,20 @@ void lkdtm_ACCESS_USERSPACE(void) > void lkdtm_ACCESS_NULL(void) > { > unsigned long tmp; > - unsigned long *ptr = (unsigned long *)NULL; > + volatile unsigned long *ptr = (unsigned long *)NULL; > > pr_info("attempting bad read at %px\n", ptr); > tmp = *ptr; > tmp += 0xc0dec0de; > + pr_err("FAIL: survived bad read\n"); > > pr_info("attempting bad write at %px\n", ptr); > *ptr = tmp; > + pr_err("FAIL: survived bad write\n"); > } > > void __init lkdtm_perms_init(void) > { > /* Make sure we can write to __ro_after_init values during __init */ > ro_after_init |= 0xAA; > - > } > diff --git a/drivers/misc/lkdtm/usercopy.c b/drivers/misc/lkdtm/usercopy.c > index e172719dd86d..b833367a45d0 100644 > --- a/drivers/misc/lkdtm/usercopy.c > +++ b/drivers/misc/lkdtm/usercopy.c > @@ -304,19 +304,22 @@ void lkdtm_USERCOPY_KERNEL(void) > return; > } > > - pr_info("attempting good copy_to_user from kernel rodata\n"); > + pr_info("attempting good copy_to_user from kernel rodata: %px\n", > + test_text); > if (copy_to_user((void __user *)user_addr, test_text, > unconst + sizeof(test_text))) { > pr_warn("copy_to_user failed unexpectedly?!\n"); > goto free_user; > } > > - pr_info("attempting bad copy_to_user from kernel text\n"); > + pr_info("attempting bad copy_to_user from kernel text: %px\n", > + vm_mmap); > if (copy_to_user((void __user *)user_addr, vm_mmap, > unconst + PAGE_SIZE)) { > pr_warn("copy_to_user failed, but lacked Oops\n"); > goto free_user; > } > + pr_err("FAIL: survived bad copy_to_user()\n"); > > free_user: > vm_munmap(user_addr, PAGE_SIZE); > -- > 2.25.1 > > -- > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group. > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20200529200347.2464284-2-keescook%40chromium.org. -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/4] lkdtm/heap: Avoid edge and middle of slabs 2020-05-29 20:03 [PATCH drivers/misc 0/4] lkdtm: Various clean ups Kees Cook 2020-05-29 20:03 ` [PATCH 1/4] lkdtm: Avoid more compiler optimizations for bad writes Kees Cook @ 2020-05-29 20:03 ` Kees Cook 2020-05-29 20:03 ` [PATCH 3/4] selftests/lkdtm: Reset WARN_ONCE to avoid false negatives Kees Cook ` (2 subsequent siblings) 4 siblings, 0 replies; 21+ messages in thread From: Kees Cook @ 2020-05-29 20:03 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Kees Cook, stable, Prasad Sodagudi, Sami Tolvanen, Amit Daniel Kachhap, linux-kselftest, clang-built-linux, linux-kernel Har har, after I moved the slab freelist pointer into the middle of the slab, now it looks like the contents are getting poisoned. Adjust the test to avoid the freelist pointer again. Fixes: 3202fa62fb43 ("slub: relocate freelist pointer to middle of object") Cc: stable@vger.kernel.org Signed-off-by: Kees Cook <keescook@chromium.org> --- drivers/misc/lkdtm/heap.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/misc/lkdtm/heap.c b/drivers/misc/lkdtm/heap.c index 3c5cec85edce..1323bc16f113 100644 --- a/drivers/misc/lkdtm/heap.c +++ b/drivers/misc/lkdtm/heap.c @@ -58,11 +58,12 @@ void lkdtm_READ_AFTER_FREE(void) int *base, *val, saw; size_t len = 1024; /* - * The slub allocator uses the first word to store the free - * pointer in some configurations. Use the middle of the - * allocation to avoid running into the freelist + * The slub allocator will use the either the first word or + * the middle of the allocation to store the free pointer, + * depending on configurations. Store in the second word to + * avoid running into the freelist. */ - size_t offset = (len / sizeof(*base)) / 2; + size_t offset = sizeof(*base); base = kmalloc(len, GFP_KERNEL); if (!base) { -- 2.25.1 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 3/4] selftests/lkdtm: Reset WARN_ONCE to avoid false negatives 2020-05-29 20:03 [PATCH drivers/misc 0/4] lkdtm: Various clean ups Kees Cook 2020-05-29 20:03 ` [PATCH 1/4] lkdtm: Avoid more compiler optimizations for bad writes Kees Cook 2020-05-29 20:03 ` [PATCH 2/4] lkdtm/heap: Avoid edge and middle of slabs Kees Cook @ 2020-05-29 20:03 ` Kees Cook 2020-05-29 20:22 ` Shuah Khan 2020-05-29 20:03 ` [PATCH 4/4] lkdtm: Make arch-specific tests always available Kees Cook 2020-06-23 23:10 ` [PATCH drivers/misc 0/4] lkdtm: Various clean ups Kees Cook 4 siblings, 1 reply; 21+ messages in thread From: Kees Cook @ 2020-05-29 20:03 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Kees Cook, Prasad Sodagudi, Sami Tolvanen, Amit Daniel Kachhap, linux-kselftest, clang-built-linux, linux-kernel Since we expect to see warnings every time for many tests, just reset the WARN_ONCE flags each time the script runs. Signed-off-by: Kees Cook <keescook@chromium.org> --- tools/testing/selftests/lkdtm/run.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tools/testing/selftests/lkdtm/run.sh b/tools/testing/selftests/lkdtm/run.sh index ee64ff8df8f4..8383eb89d88a 100755 --- a/tools/testing/selftests/lkdtm/run.sh +++ b/tools/testing/selftests/lkdtm/run.sh @@ -8,6 +8,7 @@ # set -e TRIGGER=/sys/kernel/debug/provoke-crash/DIRECT +CLEAR_ONCE=/sys/kernel/debug/clear_warn_once KSELFTEST_SKIP_TEST=4 # Verify we have LKDTM available in the kernel. @@ -67,6 +68,11 @@ cleanup() { } trap cleanup EXIT +# Reset WARN_ONCE counters so we trip it each time this runs. +if [ -w $CLEAR_ONCE ] ; then + echo 1 > $CLEAR_ONCE +fi + # Save existing dmesg so we can detect new content below dmesg > "$DMESG" -- 2.25.1 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 3/4] selftests/lkdtm: Reset WARN_ONCE to avoid false negatives 2020-05-29 20:03 ` [PATCH 3/4] selftests/lkdtm: Reset WARN_ONCE to avoid false negatives Kees Cook @ 2020-05-29 20:22 ` Shuah Khan 0 siblings, 0 replies; 21+ messages in thread From: Shuah Khan @ 2020-05-29 20:22 UTC (permalink / raw) To: Kees Cook, Greg Kroah-Hartman Cc: Prasad Sodagudi, Sami Tolvanen, Amit Daniel Kachhap, linux-kselftest, clang-built-linux, linux-kernel, Shuah Khan On 5/29/20 2:03 PM, Kees Cook wrote: > Since we expect to see warnings every time for many tests, just reset > the WARN_ONCE flags each time the script runs. > > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > tools/testing/selftests/lkdtm/run.sh | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/tools/testing/selftests/lkdtm/run.sh b/tools/testing/selftests/lkdtm/run.sh > index ee64ff8df8f4..8383eb89d88a 100755 > --- a/tools/testing/selftests/lkdtm/run.sh > +++ b/tools/testing/selftests/lkdtm/run.sh > @@ -8,6 +8,7 @@ > # > set -e > TRIGGER=/sys/kernel/debug/provoke-crash/DIRECT > +CLEAR_ONCE=/sys/kernel/debug/clear_warn_once > KSELFTEST_SKIP_TEST=4 > > # Verify we have LKDTM available in the kernel. > @@ -67,6 +68,11 @@ cleanup() { > } > trap cleanup EXIT > > +# Reset WARN_ONCE counters so we trip it each time this runs. > +if [ -w $CLEAR_ONCE ] ; then > + echo 1 > $CLEAR_ONCE > +fi > + > # Save existing dmesg so we can detect new content below > dmesg > "$DMESG" > > Acked-by: Shuah Khan <skhan@linuxfoundation.org> thanks, -- Shuah ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 4/4] lkdtm: Make arch-specific tests always available 2020-05-29 20:03 [PATCH drivers/misc 0/4] lkdtm: Various clean ups Kees Cook ` (2 preceding siblings ...) 2020-05-29 20:03 ` [PATCH 3/4] selftests/lkdtm: Reset WARN_ONCE to avoid false negatives Kees Cook @ 2020-05-29 20:03 ` Kees Cook 2020-06-23 23:10 ` [PATCH drivers/misc 0/4] lkdtm: Various clean ups Kees Cook 4 siblings, 0 replies; 21+ messages in thread From: Kees Cook @ 2020-05-29 20:03 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Kees Cook, Prasad Sodagudi, Sami Tolvanen, Amit Daniel Kachhap, linux-kselftest, clang-built-linux, linux-kernel I'd like arch-specific tests to XFAIL when on a mismatched architecture so that we can more easily compare test coverage across all systems. Lacking kernel configs or CPU features count as a FAIL, not an XFAIL. Signed-off-by: Kees Cook <keescook@chromium.org> --- drivers/misc/lkdtm/bugs.c | 34 ++++++++++++++----------- drivers/misc/lkdtm/lkdtm.h | 2 -- tools/testing/selftests/lkdtm/tests.txt | 1 + 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c index e1b43f615549..f3fde0759f2c 100644 --- a/drivers/misc/lkdtm/bugs.c +++ b/drivers/misc/lkdtm/bugs.c @@ -453,38 +453,42 @@ void lkdtm_DOUBLE_FAULT(void) #endif } -#ifdef CONFIG_ARM64_PTR_AUTH +#ifdef CONFIG_ARM64 static noinline void change_pac_parameters(void) { - /* Reset the keys of current task */ - ptrauth_thread_init_kernel(current); - ptrauth_thread_switch_kernel(current); + if (IS_ENABLED(CONFIG_ARM64_PTR_AUTH)) { + /* Reset the keys of current task */ + ptrauth_thread_init_kernel(current); + ptrauth_thread_switch_kernel(current); + } } +#endif -#define CORRUPT_PAC_ITERATE 10 noinline void lkdtm_CORRUPT_PAC(void) { +#ifdef CONFIG_ARM64 +#define CORRUPT_PAC_ITERATE 10 int i; + if (!IS_ENABLED(CONFIG_ARM64_PTR_AUTH)) + pr_err("FAIL: kernel not built with CONFIG_ARM64_PTR_AUTH\n"); + if (!system_supports_address_auth()) { - pr_err("FAIL: arm64 pointer authentication feature not present\n"); + pr_err("FAIL: CPU lacks pointer authentication feature\n"); return; } - pr_info("Change the PAC parameters to force function return failure\n"); + pr_info("changing PAC parameters to force function return failure...\n"); /* - * Pac is a hash value computed from input keys, return address and + * PAC is a hash value computed from input keys, return address and * stack pointer. As pac has fewer bits so there is a chance of * collision, so iterate few times to reduce the collision probability. */ for (i = 0; i < CORRUPT_PAC_ITERATE; i++) change_pac_parameters(); - pr_err("FAIL: %s test failed. Kernel may be unstable from here\n", __func__); -} -#else /* !CONFIG_ARM64_PTR_AUTH */ -noinline void lkdtm_CORRUPT_PAC(void) -{ - pr_err("FAIL: arm64 pointer authentication config disabled\n"); -} + pr_err("FAIL: survived PAC changes! Kernel may be unstable from here\n"); +#else + pr_err("XFAIL: this test is arm64-only\n"); #endif +} diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h index 601a2156a0d4..8878538b2c13 100644 --- a/drivers/misc/lkdtm/lkdtm.h +++ b/drivers/misc/lkdtm/lkdtm.h @@ -31,9 +31,7 @@ void lkdtm_CORRUPT_USER_DS(void); void lkdtm_STACK_GUARD_PAGE_LEADING(void); void lkdtm_STACK_GUARD_PAGE_TRAILING(void); void lkdtm_UNSET_SMEP(void); -#ifdef CONFIG_X86_32 void lkdtm_DOUBLE_FAULT(void); -#endif void lkdtm_CORRUPT_PAC(void); /* lkdtm_heap.c */ diff --git a/tools/testing/selftests/lkdtm/tests.txt b/tools/testing/selftests/lkdtm/tests.txt index 92ca32143ae5..9d266e79c6a2 100644 --- a/tools/testing/selftests/lkdtm/tests.txt +++ b/tools/testing/selftests/lkdtm/tests.txt @@ -14,6 +14,7 @@ STACK_GUARD_PAGE_LEADING STACK_GUARD_PAGE_TRAILING UNSET_SMEP CR4 bits went missing DOUBLE_FAULT +CORRUPT_PAC UNALIGNED_LOAD_STORE_WRITE #OVERWRITE_ALLOCATION Corrupts memory on failure #WRITE_AFTER_FREE Corrupts memory on failure -- 2.25.1 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH drivers/misc 0/4] lkdtm: Various clean ups 2020-05-29 20:03 [PATCH drivers/misc 0/4] lkdtm: Various clean ups Kees Cook ` (3 preceding siblings ...) 2020-05-29 20:03 ` [PATCH 4/4] lkdtm: Make arch-specific tests always available Kees Cook @ 2020-06-23 23:10 ` Kees Cook 2020-06-23 23:32 ` Randy Dunlap 2020-06-24 6:13 ` Greg Kroah-Hartman 4 siblings, 2 replies; 21+ messages in thread From: Kees Cook @ 2020-06-23 23:10 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Prasad Sodagudi, Sami Tolvanen, Amit Daniel Kachhap, linux-kselftest, clang-built-linux, linux-kernel On Fri, May 29, 2020 at 01:03:43PM -0700, Kees Cook wrote: > Hi Greg, > > Can you please apply these patches to your drivers/misc tree for LKDTM? > It's mostly a collection of fixes and improvements and tweaks to the > selftest integration. Friendly ping -- we're past -rc2 now. :) Thanks! -Kees > > Thanks! > > -Kees > > Kees Cook (4): > lkdtm: Avoid more compiler optimizations for bad writes > lkdtm/heap: Avoid edge and middle of slabs > selftests/lkdtm: Reset WARN_ONCE to avoid false negatives > lkdtm: Make arch-specific tests always available > > drivers/misc/lkdtm/bugs.c | 45 +++++++++++++------------ > drivers/misc/lkdtm/heap.c | 9 ++--- > drivers/misc/lkdtm/lkdtm.h | 2 -- > drivers/misc/lkdtm/perms.c | 22 ++++++++---- > drivers/misc/lkdtm/usercopy.c | 7 ++-- > tools/testing/selftests/lkdtm/run.sh | 6 ++++ > tools/testing/selftests/lkdtm/tests.txt | 1 + > 7 files changed, 56 insertions(+), 36 deletions(-) > > -- > 2.25.1 > -- Kees Cook ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH drivers/misc 0/4] lkdtm: Various clean ups 2020-06-23 23:10 ` [PATCH drivers/misc 0/4] lkdtm: Various clean ups Kees Cook @ 2020-06-23 23:32 ` Randy Dunlap 2020-06-24 0:01 ` Kees Cook 2020-06-24 7:23 ` Richard Weinberger 2020-06-24 6:13 ` Greg Kroah-Hartman 1 sibling, 2 replies; 21+ messages in thread From: Randy Dunlap @ 2020-06-23 23:32 UTC (permalink / raw) To: Kees Cook, Greg Kroah-Hartman Cc: Prasad Sodagudi, Sami Tolvanen, Amit Daniel Kachhap, linux-kselftest, clang-built-linux, linux-kernel, richard -rw- weinberger On 6/23/20 4:10 PM, Kees Cook wrote: > On Fri, May 29, 2020 at 01:03:43PM -0700, Kees Cook wrote: >> Hi Greg, >> >> Can you please apply these patches to your drivers/misc tree for LKDTM? >> It's mostly a collection of fixes and improvements and tweaks to the >> selftest integration. > > Friendly ping -- we're past -rc2 now. :) > > Thanks! > > -Kees > >> >> Thanks! >> >> -Kees >> >> Kees Cook (4): >> lkdtm: Avoid more compiler optimizations for bad writes >> lkdtm/heap: Avoid edge and middle of slabs >> selftests/lkdtm: Reset WARN_ONCE to avoid false negatives >> lkdtm: Make arch-specific tests always available >> >> drivers/misc/lkdtm/bugs.c | 45 +++++++++++++------------ >> drivers/misc/lkdtm/heap.c | 9 ++--- >> drivers/misc/lkdtm/lkdtm.h | 2 -- >> drivers/misc/lkdtm/perms.c | 22 ++++++++---- >> drivers/misc/lkdtm/usercopy.c | 7 ++-- >> tools/testing/selftests/lkdtm/run.sh | 6 ++++ >> tools/testing/selftests/lkdtm/tests.txt | 1 + >> 7 files changed, 56 insertions(+), 36 deletions(-) >> >> -- >> 2.25.1 >> > >> Regardless, it seems arch/x86/um/asm/desc.h is not needed any more? > True that, we can rip the file. Has anyone fixed the uml build errors? thanks. -- ~Randy ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH drivers/misc 0/4] lkdtm: Various clean ups 2020-06-23 23:32 ` Randy Dunlap @ 2020-06-24 0:01 ` Kees Cook 2020-06-24 7:23 ` Richard Weinberger 1 sibling, 0 replies; 21+ messages in thread From: Kees Cook @ 2020-06-24 0:01 UTC (permalink / raw) To: Randy Dunlap Cc: Greg Kroah-Hartman, Prasad Sodagudi, Sami Tolvanen, Amit Daniel Kachhap, linux-kselftest, clang-built-linux, linux-kernel, richard -rw- weinberger On Tue, Jun 23, 2020 at 04:32:12PM -0700, Randy Dunlap wrote: > On 6/23/20 4:10 PM, Kees Cook wrote: > > On Fri, May 29, 2020 at 01:03:43PM -0700, Kees Cook wrote: > >> Hi Greg, > >> > >> Can you please apply these patches to your drivers/misc tree for LKDTM? > >> It's mostly a collection of fixes and improvements and tweaks to the > >> selftest integration. > > > > Friendly ping -- we're past -rc2 now. :) > > > > Thanks! > > > > -Kees > > > >> > >> Thanks! > >> > >> -Kees > >> > >> Kees Cook (4): > >> lkdtm: Avoid more compiler optimizations for bad writes > >> lkdtm/heap: Avoid edge and middle of slabs > >> selftests/lkdtm: Reset WARN_ONCE to avoid false negatives > >> lkdtm: Make arch-specific tests always available > >> > >> drivers/misc/lkdtm/bugs.c | 45 +++++++++++++------------ > >> drivers/misc/lkdtm/heap.c | 9 ++--- > >> drivers/misc/lkdtm/lkdtm.h | 2 -- > >> drivers/misc/lkdtm/perms.c | 22 ++++++++---- > >> drivers/misc/lkdtm/usercopy.c | 7 ++-- > >> tools/testing/selftests/lkdtm/run.sh | 6 ++++ > >> tools/testing/selftests/lkdtm/tests.txt | 1 + > >> 7 files changed, 56 insertions(+), 36 deletions(-) > >> > >> -- > >> 2.25.1 > >> > > > > >> Regardless, it seems arch/x86/um/asm/desc.h is not needed any more? > > > True that, we can rip the file. > > Has anyone fixed the uml build errors? Oh, I thought that had nothing to do with the lkdtm changes? Should I rip out that file and resend? -- Kees Cook ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH drivers/misc 0/4] lkdtm: Various clean ups 2020-06-23 23:32 ` Randy Dunlap 2020-06-24 0:01 ` Kees Cook @ 2020-06-24 7:23 ` Richard Weinberger 2020-06-24 20:36 ` Kees Cook 1 sibling, 1 reply; 21+ messages in thread From: Richard Weinberger @ 2020-06-24 7:23 UTC (permalink / raw) To: Randy Dunlap Cc: Kees Cook, Greg Kroah-Hartman, Prasad Sodagudi, Sami Tolvanen, Amit Daniel Kachhap, linux-kselftest, clang-built-linux, linux-kernel, Richard Weinberger ----- Ursprüngliche Mail ----- >>> Regardless, it seems arch/x86/um/asm/desc.h is not needed any more? > >> True that, we can rip the file. > > Has anyone fixed the uml build errors? I didn't realize that this is a super urgent issue. ;-) Kees, if you want you can carry a patch in your series, I'll ack it. Otherwise I can also do a patch and bring it via the uml tree upstream as soon more fixes queued up. Thanks, //richard ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH drivers/misc 0/4] lkdtm: Various clean ups 2020-06-24 7:23 ` Richard Weinberger @ 2020-06-24 20:36 ` Kees Cook 2020-06-24 21:29 ` Randy Dunlap 0 siblings, 1 reply; 21+ messages in thread From: Kees Cook @ 2020-06-24 20:36 UTC (permalink / raw) To: Richard Weinberger Cc: Randy Dunlap, Greg Kroah-Hartman, Prasad Sodagudi, Sami Tolvanen, Amit Daniel Kachhap, linux-kselftest, clang-built-linux, linux-kernel, Richard Weinberger On Wed, Jun 24, 2020 at 09:23:25AM +0200, Richard Weinberger wrote: > ----- Ursprüngliche Mail ----- > >>> Regardless, it seems arch/x86/um/asm/desc.h is not needed any more? > > > >> True that, we can rip the file. > > > > Has anyone fixed the uml build errors? > > I didn't realize that this is a super urgent issue. ;-) > > Kees, if you want you can carry a patch in your series, I'll ack it. > Otherwise I can also do a patch and bring it via the uml tree upstream > as soon more fixes queued up. I think the lkdtm change will tweak this bug, so I'm happy to carry the patch (I just haven't had time to create and test one). Is it really just as simple as removing arch/x86/um/asm/desc.h? -- Kees Cook ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH drivers/misc 0/4] lkdtm: Various clean ups 2020-06-24 20:36 ` Kees Cook @ 2020-06-24 21:29 ` Randy Dunlap 2020-06-24 22:01 ` Richard Weinberger 0 siblings, 1 reply; 21+ messages in thread From: Randy Dunlap @ 2020-06-24 21:29 UTC (permalink / raw) To: Kees Cook, Richard Weinberger Cc: Greg Kroah-Hartman, Prasad Sodagudi, Sami Tolvanen, Amit Daniel Kachhap, linux-kselftest, clang-built-linux, linux-kernel, Richard Weinberger On 6/24/20 1:36 PM, Kees Cook wrote: > On Wed, Jun 24, 2020 at 09:23:25AM +0200, Richard Weinberger wrote: >> ----- Ursprüngliche Mail ----- >>>>> Regardless, it seems arch/x86/um/asm/desc.h is not needed any more? >>> >>>> True that, we can rip the file. >>> >>> Has anyone fixed the uml build errors? >> >> I didn't realize that this is a super urgent issue. ;-) >> >> Kees, if you want you can carry a patch in your series, I'll ack it. >> Otherwise I can also do a patch and bring it via the uml tree upstream >> as soon more fixes queued up. > > I think the lkdtm change will tweak this bug, so I'm happy to carry the > patch (I just haven't had time to create and test one). Is it really > just as simple as removing arch/x86/um/asm/desc.h? > I just tried that and the build is still failing, so No, it's not that simple. But thanks for offering. I'll just ignore the UML build errors for now. -- ~Randy ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH drivers/misc 0/4] lkdtm: Various clean ups 2020-06-24 21:29 ` Randy Dunlap @ 2020-06-24 22:01 ` Richard Weinberger 2020-06-24 22:23 ` Randy Dunlap 0 siblings, 1 reply; 21+ messages in thread From: Richard Weinberger @ 2020-06-24 22:01 UTC (permalink / raw) To: Randy Dunlap Cc: Kees Cook, Richard Weinberger, Greg Kroah-Hartman, Prasad Sodagudi, Sami Tolvanen, Amit Daniel Kachhap, open list:KERNEL SELFTEST FRAMEWORK, clang-built-linux, linux-kernel On Wed, Jun 24, 2020 at 11:29 PM Randy Dunlap <rdunlap@infradead.org> wrote: > > On 6/24/20 1:36 PM, Kees Cook wrote: > > On Wed, Jun 24, 2020 at 09:23:25AM +0200, Richard Weinberger wrote: > >> ----- Ursprüngliche Mail ----- > >>>>> Regardless, it seems arch/x86/um/asm/desc.h is not needed any more? > >>> > >>>> True that, we can rip the file. > >>> > >>> Has anyone fixed the uml build errors? > >> > >> I didn't realize that this is a super urgent issue. ;-) > >> > >> Kees, if you want you can carry a patch in your series, I'll ack it. > >> Otherwise I can also do a patch and bring it via the uml tree upstream > >> as soon more fixes queued up. > > > > I think the lkdtm change will tweak this bug, so I'm happy to carry the > > patch (I just haven't had time to create and test one). Is it really > > just as simple as removing arch/x86/um/asm/desc.h? > > > > I just tried that and the build is still failing, so No, it's not that simple. > > But thanks for offering. > > I'll just ignore the UML build errors for now. This is a allyesconfig? I just gave CONFIG_LKDTM=y a try, builds fine here. But the desc.h in uml is still in vain and can be deleted AFAICT. -- Thanks, //richard ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH drivers/misc 0/4] lkdtm: Various clean ups 2020-06-24 22:01 ` Richard Weinberger @ 2020-06-24 22:23 ` Randy Dunlap 2020-06-24 22:35 ` Randy Dunlap 0 siblings, 1 reply; 21+ messages in thread From: Randy Dunlap @ 2020-06-24 22:23 UTC (permalink / raw) To: Richard Weinberger Cc: Kees Cook, Richard Weinberger, Greg Kroah-Hartman, Prasad Sodagudi, Sami Tolvanen, Amit Daniel Kachhap, open list:KERNEL SELFTEST FRAMEWORK, clang-built-linux, linux-kernel On 6/24/20 3:01 PM, Richard Weinberger wrote: > On Wed, Jun 24, 2020 at 11:29 PM Randy Dunlap <rdunlap@infradead.org> wrote: >> >> On 6/24/20 1:36 PM, Kees Cook wrote: >>> On Wed, Jun 24, 2020 at 09:23:25AM +0200, Richard Weinberger wrote: >>>> ----- Ursprüngliche Mail ----- >>>>>>> Regardless, it seems arch/x86/um/asm/desc.h is not needed any more? >>>>> >>>>>> True that, we can rip the file. >>>>> >>>>> Has anyone fixed the uml build errors? >>>> >>>> I didn't realize that this is a super urgent issue. ;-) >>>> >>>> Kees, if you want you can carry a patch in your series, I'll ack it. >>>> Otherwise I can also do a patch and bring it via the uml tree upstream >>>> as soon more fixes queued up. >>> >>> I think the lkdtm change will tweak this bug, so I'm happy to carry the >>> patch (I just haven't had time to create and test one). Is it really >>> just as simple as removing arch/x86/um/asm/desc.h? >>> >> >> I just tried that and the build is still failing, so No, it's not that simple. >> >> But thanks for offering. >> >> I'll just ignore the UML build errors for now. > > This is a allyesconfig? > I just gave CONFIG_LKDTM=y a try, builds fine here. > I'm building linux-next and it fails. > But the desc.h in uml is still in vain and can be deleted AFAICT. -- ~Randy ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH drivers/misc 0/4] lkdtm: Various clean ups 2020-06-24 22:23 ` Randy Dunlap @ 2020-06-24 22:35 ` Randy Dunlap 2020-06-25 1:45 ` Randy Dunlap 0 siblings, 1 reply; 21+ messages in thread From: Randy Dunlap @ 2020-06-24 22:35 UTC (permalink / raw) To: Richard Weinberger Cc: Kees Cook, Richard Weinberger, Greg Kroah-Hartman, Prasad Sodagudi, Sami Tolvanen, Amit Daniel Kachhap, open list:KERNEL SELFTEST FRAMEWORK, clang-built-linux, linux-kernel On 6/24/20 3:23 PM, Randy Dunlap wrote: > On 6/24/20 3:01 PM, Richard Weinberger wrote: >> On Wed, Jun 24, 2020 at 11:29 PM Randy Dunlap <rdunlap@infradead.org> wrote: >>> >>> On 6/24/20 1:36 PM, Kees Cook wrote: >>>> On Wed, Jun 24, 2020 at 09:23:25AM +0200, Richard Weinberger wrote: >>>>> ----- Ursprüngliche Mail ----- >>>>>>>> Regardless, it seems arch/x86/um/asm/desc.h is not needed any more? >>>>>> >>>>>>> True that, we can rip the file. >>>>>> >>>>>> Has anyone fixed the uml build errors? >>>>> >>>>> I didn't realize that this is a super urgent issue. ;-) >>>>> >>>>> Kees, if you want you can carry a patch in your series, I'll ack it. >>>>> Otherwise I can also do a patch and bring it via the uml tree upstream >>>>> as soon more fixes queued up. >>>> >>>> I think the lkdtm change will tweak this bug, so I'm happy to carry the >>>> patch (I just haven't had time to create and test one). Is it really >>>> just as simple as removing arch/x86/um/asm/desc.h? >>>> >>> >>> I just tried that and the build is still failing, so No, it's not that simple. >>> >>> But thanks for offering. >>> >>> I'll just ignore the UML build errors for now. >> >> This is a allyesconfig? >> I just gave CONFIG_LKDTM=y a try, builds fine here. >> > > I'm building linux-next and it fails. More specifically, uml for i386 fails. x86_64 is OK. The problem is with the <asm/desc.h> file. I'm tampering with it... >> But the desc.h in uml is still in vain and can be deleted AFAICT. -- ~Randy ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH drivers/misc 0/4] lkdtm: Various clean ups 2020-06-24 22:35 ` Randy Dunlap @ 2020-06-25 1:45 ` Randy Dunlap 2020-06-25 6:06 ` Kees Cook 0 siblings, 1 reply; 21+ messages in thread From: Randy Dunlap @ 2020-06-25 1:45 UTC (permalink / raw) To: Richard Weinberger Cc: Kees Cook, Richard Weinberger, Greg Kroah-Hartman, Prasad Sodagudi, Sami Tolvanen, Amit Daniel Kachhap, open list:KERNEL SELFTEST FRAMEWORK, clang-built-linux, linux-kernel On 6/24/20 3:35 PM, Randy Dunlap wrote: > On 6/24/20 3:23 PM, Randy Dunlap wrote: >> On 6/24/20 3:01 PM, Richard Weinberger wrote: >>> On Wed, Jun 24, 2020 at 11:29 PM Randy Dunlap <rdunlap@infradead.org> wrote: >>>> >>>> On 6/24/20 1:36 PM, Kees Cook wrote: >>>>> On Wed, Jun 24, 2020 at 09:23:25AM +0200, Richard Weinberger wrote: >>>>>> ----- Ursprüngliche Mail ----- >>>>>>>>> Regardless, it seems arch/x86/um/asm/desc.h is not needed any more? >>>>>>> >>>>>>>> True that, we can rip the file. >>>>>>> >>>>>>> Has anyone fixed the uml build errors? >>>>>> >>>>>> I didn't realize that this is a super urgent issue. ;-) >>>>>> >>>>>> Kees, if you want you can carry a patch in your series, I'll ack it. >>>>>> Otherwise I can also do a patch and bring it via the uml tree upstream >>>>>> as soon more fixes queued up. >>>>> >>>>> I think the lkdtm change will tweak this bug, so I'm happy to carry the >>>>> patch (I just haven't had time to create and test one). Is it really >>>>> just as simple as removing arch/x86/um/asm/desc.h? >>>>> >>>> >>>> I just tried that and the build is still failing, so No, it's not that simple. >>>> >>>> But thanks for offering. >>>> >>>> I'll just ignore the UML build errors for now. >>> >>> This is a allyesconfig? >>> I just gave CONFIG_LKDTM=y a try, builds fine here. >>> >> >> I'm building linux-next and it fails. > > More specifically, uml for i386 fails. x86_64 is OK. > The problem is with the <asm/desc.h> file. > I'm tampering with it... I'm not getting anywhere with this. Too many mazes of tiny twisty passages. >>> But the desc.h in uml is still in vain and can be deleted AFAICT. Looks like lkdtm/bugs.c needs to get/use arch/x86/include/asm/processor.h but it actually uses arch/x86/um/asm/processor*.h, which does not have the needed structs etc. Here are the build errors and warnings that I am seeing with allmodconfig: CC [M] drivers/misc/lkdtm/bugs.o In file included from ../arch/x86/include/asm/desc.h:11:0, from ../drivers/misc/lkdtm/bugs.c:17: ../arch/x86/include/asm/cpu_entry_area.h:65:42: error: invalid application of ‘sizeof’ to incomplete type ‘struct x86_hw_tss’ unsigned long stack[(PAGE_SIZE - sizeof(struct x86_hw_tss)) / sizeof(unsigned long)]; ^~~~~~ ../arch/x86/include/asm/cpu_entry_area.h:66:20: error: field ‘tss’ has incomplete type struct x86_hw_tss tss; ^~~ ../arch/x86/include/asm/cpu_entry_area.h:89:26: error: field ‘entry_stack_page’ has incomplete type struct entry_stack_page entry_stack_page; ^~~~~~~~~~~~~~~~ ../arch/x86/include/asm/cpu_entry_area.h:100:20: error: field ‘tss’ has incomplete type struct tss_struct tss; ^~~ In file included from ../drivers/misc/lkdtm/bugs.c:17:0: ../arch/x86/include/asm/desc.h:45:25: error: ‘GDT_ENTRIES’ undeclared here (not in a function); did you mean ‘LDT_ENTRIES’? struct desc_struct gdt[GDT_ENTRIES]; ^~~~~~~~~~~ LDT_ENTRIES ../arch/x86/include/asm/desc.h: In function ‘__set_tss_desc’: ../arch/x86/include/asm/desc.h:186:10: error: ‘__KERNEL_TSS_LIMIT’ undeclared (first use in this function); did you mean ‘__KERNEL__’? __KERNEL_TSS_LIMIT); ^~~~~~~~~~~~~~~~~~ __KERNEL__ ../arch/x86/include/asm/desc.h:186:10: note: each undeclared identifier is reported only once for each function it appears in ../arch/x86/include/asm/desc.h: In function ‘native_set_ldt’: ../arch/x86/include/asm/desc.h:202:40: error: ‘GDT_ENTRY_LDT’ undeclared (first use in this function); did you mean ‘GDT_ENTRY_INIT’? write_gdt_entry(get_cpu_gdt_rw(cpu), GDT_ENTRY_LDT, ^ ../arch/x86/include/asm/desc.h:123:75: note: in definition of macro ‘write_gdt_entry’ #define write_gdt_entry(dt, entry, desc, type) native_write_gdt_entry(dt, entry, desc, type) ^~~~~ ../arch/x86/include/asm/desc.h: In function ‘native_load_tr_desc’: ../arch/x86/include/asm/desc.h:259:31: error: ‘GDT_ENTRY_TSS’ undeclared (first use in this function); did you mean ‘GDT_ENTRIES’? asm volatile("ltr %w0"::"q" (GDT_ENTRY_TSS*8)); ^~~~~~~~~~~~~ GDT_ENTRIES ../arch/x86/include/asm/desc.h: In function ‘native_load_tls’: ../arch/x86/include/asm/desc.h:278:33: error: ‘struct thread_struct’ has no member named ‘tls_array’ gdt[GDT_ENTRY_TLS_MIN + i] = t->tls_array[i]; ^~ In file included from ../arch/x86/include/asm/string.h:3:0, from ../include/linux/string.h:20, from ../arch/x86/um/asm/processor_32.h:9, from ../arch/x86/um/asm/processor.h:10, from ../include/linux/rcupdate.h:30, from ../include/linux/rculist.h:11, from ../include/linux/pid.h:5, from ../include/linux/sched.h:14, from ../drivers/misc/lkdtm/bugs.c:10: ../arch/x86/include/asm/desc.h: In function ‘force_reload_TR’: ../arch/x86/include/asm/desc.h:288:18: error: ‘GDT_ENTRY_TSS’ undeclared (first use in this function); did you mean ‘GDT_ENTRIES’? memcpy(&tss, &d[GDT_ENTRY_TSS], sizeof(tss_desc)); ^ ../arch/x86/include/asm/string_32.h:182:45: note: in definition of macro ‘memcpy’ #define memcpy(t, f, n) __builtin_memcpy(t, f, n) ^ In file included from ../include/linux/kernel.h:11:0, from ../drivers/misc/lkdtm/lkdtm.h:7, from ../drivers/misc/lkdtm/bugs.c:8: ../arch/x86/include/asm/desc.h: In function ‘invalidate_tss_limit’: ../arch/x86/include/asm/desc.h:327:32: error: ‘TIF_IO_BITMAP’ undeclared (first use in this function); did you mean ‘CONFIG_SBITMAP’? if (unlikely(test_thread_flag(TIF_IO_BITMAP))) ^ ../include/linux/compiler.h:78:42: note: in definition of macro ‘unlikely’ # define unlikely(x) __builtin_expect(!!(x), 0) ^ ../arch/x86/include/asm/desc.h:327:15: note: in expansion of macro ‘test_thread_flag’ if (unlikely(test_thread_flag(TIF_IO_BITMAP))) ^~~~~~~~~~~~~~~~ In file included from ../drivers/misc/lkdtm/bugs.c:17:0: ../arch/x86/include/asm/desc.h: At top level: ../arch/x86/include/asm/desc.h:334:0: warning: "LDT_empty" redefined #define LDT_empty(info) \ In file included from ../arch/um/include/asm/mmu.h:10:0, from ../include/linux/mm_types.h:18, from ../include/linux/sched/signal.h:13, from ../drivers/misc/lkdtm/bugs.c:11: ../arch/x86/um/asm/mm_context.h:65:0: note: this is the location of the previous definition #define LDT_empty(info) (_LDT_empty(info)) In file included from ../drivers/misc/lkdtm/bugs.c:17:0: ../arch/x86/include/asm/desc.h: In function ‘get_cpu_gdt_rw’: ../arch/x86/include/asm/desc.h:54:1: warning: control reaches end of non-void function [-Wreturn-type] } ^ -- ~Randy Reported-by: Randy Dunlap <rdunlap@infradead.org> ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH drivers/misc 0/4] lkdtm: Various clean ups 2020-06-25 1:45 ` Randy Dunlap @ 2020-06-25 6:06 ` Kees Cook 2020-06-25 6:24 ` Richard Weinberger 0 siblings, 1 reply; 21+ messages in thread From: Kees Cook @ 2020-06-25 6:06 UTC (permalink / raw) To: Randy Dunlap Cc: Richard Weinberger, Richard Weinberger, Greg Kroah-Hartman, Prasad Sodagudi, Sami Tolvanen, Amit Daniel Kachhap, open list:KERNEL SELFTEST FRAMEWORK, clang-built-linux, linux-kernel On Wed, Jun 24, 2020 at 06:45:47PM -0700, Randy Dunlap wrote: > Looks like lkdtm/bugs.c needs to get/use arch/x86/include/asm/processor.h > but it actually uses arch/x86/um/asm/processor*.h, which does not have the > needed structs etc. Should I just test for !UML in bugs.c? (This is all for the lkdtm_DOUBLE_FAULT() test.) I already do those kinds of checks for the lkdtm_UNSET_SMEP() test. e.g.: diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c index 736675f0a246..f3e7040a7739 100644 --- a/drivers/misc/lkdtm/bugs.c +++ b/drivers/misc/lkdtm/bugs.c @@ -13,7 +13,7 @@ #include <linux/uaccess.h> #include <linux/slab.h> -#ifdef CONFIG_X86_32 +#if IS_ENABLED(CONFIG_X86_32) && !IS_ENABLED(CONFIG_UML) #include <asm/desc.h> #endif @@ -419,7 +419,7 @@ void lkdtm_UNSET_SMEP(void) void lkdtm_DOUBLE_FAULT(void) { -#ifdef CONFIG_X86_32 +#if IS_ENABLED(CONFIG_X86_32) && !IS_ENABLED(CONFIG_UML) /* * Trigger #DF by setting the stack limit to zero. This clobbers * a GDT TLS slot, which is okay because the current task will die -- Kees Cook ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH drivers/misc 0/4] lkdtm: Various clean ups 2020-06-25 6:06 ` Kees Cook @ 2020-06-25 6:24 ` Richard Weinberger 2020-06-25 15:24 ` Randy Dunlap 0 siblings, 1 reply; 21+ messages in thread From: Richard Weinberger @ 2020-06-25 6:24 UTC (permalink / raw) To: Kees Cook Cc: Randy Dunlap, Richard Weinberger, Greg Kroah-Hartman, Prasad Sodagudi, Sami Tolvanen, Amit Daniel Kachhap, linux-kselftest, clang-built-linux, linux-kernel ----- Ursprüngliche Mail ----- > Von: "Kees Cook" <keescook@chromium.org> > An: "Randy Dunlap" <rdunlap@infradead.org> > CC: "Richard Weinberger" <richard.weinberger@gmail.com>, "richard" <richard@nod.at>, "Greg Kroah-Hartman" > <gregkh@linuxfoundation.org>, "Prasad Sodagudi" <psodagud@codeaurora.org>, "Sami Tolvanen" <samitolvanen@google.com>, > "Amit Daniel Kachhap" <amit.kachhap@arm.com>, "linux-kselftest" <linux-kselftest@vger.kernel.org>, "clang-built-linux" > <clang-built-linux@googlegroups.com>, "linux-kernel" <linux-kernel@vger.kernel.org> > Gesendet: Donnerstag, 25. Juni 2020 08:06:18 > Betreff: Re: [PATCH drivers/misc 0/4] lkdtm: Various clean ups > On Wed, Jun 24, 2020 at 06:45:47PM -0700, Randy Dunlap wrote: >> Looks like lkdtm/bugs.c needs to get/use arch/x86/include/asm/processor.h >> but it actually uses arch/x86/um/asm/processor*.h, which does not have the >> needed structs etc. > > Should I just test for !UML in bugs.c? (This is all for the > lkdtm_DOUBLE_FAULT() test.) I already do those kinds of checks for the > lkdtm_UNSET_SMEP() test. e.g.: Just had a look. Yes, this sounds good to me. UML has CONFIG_X86_32=y but no GDT. :-) Thanks, //richard ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH drivers/misc 0/4] lkdtm: Various clean ups 2020-06-25 6:24 ` Richard Weinberger @ 2020-06-25 15:24 ` Randy Dunlap 0 siblings, 0 replies; 21+ messages in thread From: Randy Dunlap @ 2020-06-25 15:24 UTC (permalink / raw) To: Richard Weinberger, Kees Cook Cc: Richard Weinberger, Greg Kroah-Hartman, Prasad Sodagudi, Sami Tolvanen, Amit Daniel Kachhap, linux-kselftest, clang-built-linux, linux-kernel On 6/24/20 11:24 PM, Richard Weinberger wrote: > ----- Ursprüngliche Mail ----- >> Von: "Kees Cook" <keescook@chromium.org> >> An: "Randy Dunlap" <rdunlap@infradead.org> >> CC: "Richard Weinberger" <richard.weinberger@gmail.com>, "richard" <richard@nod.at>, "Greg Kroah-Hartman" >> <gregkh@linuxfoundation.org>, "Prasad Sodagudi" <psodagud@codeaurora.org>, "Sami Tolvanen" <samitolvanen@google.com>, >> "Amit Daniel Kachhap" <amit.kachhap@arm.com>, "linux-kselftest" <linux-kselftest@vger.kernel.org>, "clang-built-linux" >> <clang-built-linux@googlegroups.com>, "linux-kernel" <linux-kernel@vger.kernel.org> >> Gesendet: Donnerstag, 25. Juni 2020 08:06:18 >> Betreff: Re: [PATCH drivers/misc 0/4] lkdtm: Various clean ups > >> On Wed, Jun 24, 2020 at 06:45:47PM -0700, Randy Dunlap wrote: >>> Looks like lkdtm/bugs.c needs to get/use arch/x86/include/asm/processor.h >>> but it actually uses arch/x86/um/asm/processor*.h, which does not have the >>> needed structs etc. >> >> Should I just test for !UML in bugs.c? (This is all for the >> lkdtm_DOUBLE_FAULT() test.) I already do those kinds of checks for the >> lkdtm_UNSET_SMEP() test. e.g.: > > Just had a look. Yes, this sounds good to me. UML has CONFIG_X86_32=y but no GDT. :-) Sounds good to me also. Thanks. -- ~Randy ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH drivers/misc 0/4] lkdtm: Various clean ups 2020-06-23 23:10 ` [PATCH drivers/misc 0/4] lkdtm: Various clean ups Kees Cook 2020-06-23 23:32 ` Randy Dunlap @ 2020-06-24 6:13 ` Greg Kroah-Hartman 1 sibling, 0 replies; 21+ messages in thread From: Greg Kroah-Hartman @ 2020-06-24 6:13 UTC (permalink / raw) To: Kees Cook Cc: Prasad Sodagudi, Sami Tolvanen, Amit Daniel Kachhap, linux-kselftest, clang-built-linux, linux-kernel On Tue, Jun 23, 2020 at 04:10:23PM -0700, Kees Cook wrote: > On Fri, May 29, 2020 at 01:03:43PM -0700, Kees Cook wrote: > > Hi Greg, > > > > Can you please apply these patches to your drivers/misc tree for LKDTM? > > It's mostly a collection of fixes and improvements and tweaks to the > > selftest integration. > > Friendly ping -- we're past -rc2 now. :) $ mdfrm -c ~/mail/todo/ 1970 messages in /home/gregkh/mail/todo/ You are in good company, give me some time to catch up please... thanks, greg k-h ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2020-06-25 15:25 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-29 20:03 [PATCH drivers/misc 0/4] lkdtm: Various clean ups Kees Cook 2020-05-29 20:03 ` [PATCH 1/4] lkdtm: Avoid more compiler optimizations for bad writes Kees Cook 2020-05-29 20:10 ` Nick Desaulniers 2020-05-29 20:03 ` [PATCH 2/4] lkdtm/heap: Avoid edge and middle of slabs Kees Cook 2020-05-29 20:03 ` [PATCH 3/4] selftests/lkdtm: Reset WARN_ONCE to avoid false negatives Kees Cook 2020-05-29 20:22 ` Shuah Khan 2020-05-29 20:03 ` [PATCH 4/4] lkdtm: Make arch-specific tests always available Kees Cook 2020-06-23 23:10 ` [PATCH drivers/misc 0/4] lkdtm: Various clean ups Kees Cook 2020-06-23 23:32 ` Randy Dunlap 2020-06-24 0:01 ` Kees Cook 2020-06-24 7:23 ` Richard Weinberger 2020-06-24 20:36 ` Kees Cook 2020-06-24 21:29 ` Randy Dunlap 2020-06-24 22:01 ` Richard Weinberger 2020-06-24 22:23 ` Randy Dunlap 2020-06-24 22:35 ` Randy Dunlap 2020-06-25 1:45 ` Randy Dunlap 2020-06-25 6:06 ` Kees Cook 2020-06-25 6:24 ` Richard Weinberger 2020-06-25 15:24 ` Randy Dunlap 2020-06-24 6:13 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).