linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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

* [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

* [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 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

* 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

* 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: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

* 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

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).