diff mbox series

lkdtm: print real addresses

Message ID edfe11c6d1f9bb0a7c2cee655895dfeedc1dda5e.1541613057.git.christophe.leroy@c-s.fr (mailing list archive)
State Not Applicable
Headers show
Series lkdtm: print real addresses | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success next/apply_patch Successfully applied
snowpatch_ozlabs/checkpatch warning Test checkpatch on branch next
snowpatch_ozlabs/build-ppc64le success Test build-ppc64le on branch next
snowpatch_ozlabs/build-ppc64be success Test build-ppc64be on branch next
snowpatch_ozlabs/build-ppc64e success Test build-ppc64e on branch next
snowpatch_ozlabs/build-ppc32 success Test build-ppc32 on branch next

Commit Message

Christophe Leroy Nov. 7, 2018, 8:14 p.m. UTC
Today, when doing a lkdtm test before the readiness of the
random generator, (ptrval) is printed instead of the address
at which it perform the fault:

[ 1597.337030] lkdtm: Performing direct entry EXEC_USERSPACE
[ 1597.337142] lkdtm: attempting ok execution at (ptrval)
[ 1597.337398] lkdtm: attempting bad execution at (ptrval)
[ 1597.337460] kernel tried to execute user page (77858000) -exploit attempt? (uid: 0)
[ 1597.344769] Unable to handle kernel paging request for instruction fetch
[ 1597.351392] Faulting instruction address: 0x77858000
[ 1597.356312] Oops: Kernel access of bad area, sig: 11 [#1]

If the lkdtm test is done later on, it prints an hashed address.

In both cases this is pointless. The purpose of the test is to
ensure the kernel generates an Oops at the expected address,
so real addresses needs to be printed. This patch fixes that.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 drivers/misc/lkdtm/perms.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Comments

Greg Kroah-Hartman Nov. 27, 2018, 7:43 a.m. UTC | #1
On Wed, Nov 07, 2018 at 08:14:10PM +0000, Christophe Leroy wrote:
> Today, when doing a lkdtm test before the readiness of the
> random generator, (ptrval) is printed instead of the address
> at which it perform the fault:
> 
> [ 1597.337030] lkdtm: Performing direct entry EXEC_USERSPACE
> [ 1597.337142] lkdtm: attempting ok execution at (ptrval)
> [ 1597.337398] lkdtm: attempting bad execution at (ptrval)
> [ 1597.337460] kernel tried to execute user page (77858000) -exploit attempt? (uid: 0)
> [ 1597.344769] Unable to handle kernel paging request for instruction fetch
> [ 1597.351392] Faulting instruction address: 0x77858000
> [ 1597.356312] Oops: Kernel access of bad area, sig: 11 [#1]
> 
> If the lkdtm test is done later on, it prints an hashed address.
> 
> In both cases this is pointless. The purpose of the test is to
> ensure the kernel generates an Oops at the expected address,
> so real addresses needs to be printed. This patch fixes that.

I am pretty sure this is intentional.  Kees?
Kees Cook Nov. 27, 2018, 5:18 p.m. UTC | #2
On Mon, Nov 26, 2018 at 11:43 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Wed, Nov 07, 2018 at 08:14:10PM +0000, Christophe Leroy wrote:
>> Today, when doing a lkdtm test before the readiness of the
>> random generator, (ptrval) is printed instead of the address
>> at which it perform the fault:
>>
>> [ 1597.337030] lkdtm: Performing direct entry EXEC_USERSPACE
>> [ 1597.337142] lkdtm: attempting ok execution at (ptrval)
>> [ 1597.337398] lkdtm: attempting bad execution at (ptrval)
>> [ 1597.337460] kernel tried to execute user page (77858000) -exploit attempt? (uid: 0)
>> [ 1597.344769] Unable to handle kernel paging request for instruction fetch
>> [ 1597.351392] Faulting instruction address: 0x77858000
>> [ 1597.356312] Oops: Kernel access of bad area, sig: 11 [#1]
>>
>> If the lkdtm test is done later on, it prints an hashed address.
>>
>> In both cases this is pointless. The purpose of the test is to
>> ensure the kernel generates an Oops at the expected address,
>> so real addresses needs to be printed. This patch fixes that.
>
> I am pretty sure this is intentional.  Kees?

I've gone back and forth on this and in the end I decided I'd wait and
see if anyone was bothered by it besides just me. :) But, yes, for
lkdtm do really do want a "real" view of the pointer because we're
comparing it against page tables and/or kernel section layout, etc.

I've applied this to my lkdtm -next tree. Thanks!
diff mbox series

Patch

diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index 53b85c9d16b8..fa54add6375a 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c
@@ -47,7 +47,7 @@  static noinline void execute_location(void *dst, bool write)
 {
 	void (*func)(void) = dst;
 
-	pr_info("attempting ok execution at %p\n", do_nothing);
+	pr_info("attempting ok execution at %px\n", do_nothing);
 	do_nothing();
 
 	if (write == CODE_WRITE) {
@@ -55,7 +55,7 @@  static noinline void execute_location(void *dst, bool write)
 		flush_icache_range((unsigned long)dst,
 				   (unsigned long)dst + EXEC_SIZE);
 	}
-	pr_info("attempting bad execution at %p\n", func);
+	pr_info("attempting bad execution at %px\n", func);
 	func();
 }
 
@@ -66,14 +66,14 @@  static void execute_user_location(void *dst)
 	/* Intentionally crossing kernel/user memory boundary. */
 	void (*func)(void) = dst;
 
-	pr_info("attempting ok execution at %p\n", do_nothing);
+	pr_info("attempting ok execution at %px\n", do_nothing);
 	do_nothing();
 
 	copied = access_process_vm(current, (unsigned long)dst, do_nothing,
 				   EXEC_SIZE, FOLL_WRITE);
 	if (copied < EXEC_SIZE)
 		return;
-	pr_info("attempting bad execution at %p\n", func);
+	pr_info("attempting bad execution at %px\n", func);
 	func();
 }
 
@@ -82,7 +82,7 @@  void lkdtm_WRITE_RO(void)
 	/* Explicitly cast away "const" for the test. */
 	unsigned long *ptr = (unsigned long *)&rodata;
 
-	pr_info("attempting bad rodata write at %p\n", ptr);
+	pr_info("attempting bad rodata write at %px\n", ptr);
 	*ptr ^= 0xabcd1234;
 }
 
@@ -100,7 +100,7 @@  void lkdtm_WRITE_RO_AFTER_INIT(void)
 		return;
 	}
 
-	pr_info("attempting bad ro_after_init write at %p\n", ptr);
+	pr_info("attempting bad ro_after_init write at %px\n", ptr);
 	*ptr ^= 0xabcd1234;
 }
 
@@ -112,7 +112,7 @@  void lkdtm_WRITE_KERN(void)
 	size = (unsigned long)do_overwritten - (unsigned long)do_nothing;
 	ptr = (unsigned char *)do_overwritten;
 
-	pr_info("attempting bad %zu byte write at %p\n", size, ptr);
+	pr_info("attempting bad %zu byte write at %px\n", size, ptr);
 	memcpy(ptr, (unsigned char *)do_nothing, size);
 	flush_icache_range((unsigned long)ptr, (unsigned long)(ptr + size));
 
@@ -185,11 +185,11 @@  void lkdtm_ACCESS_USERSPACE(void)
 
 	ptr = (unsigned long *)user_addr;
 
-	pr_info("attempting bad read at %p\n", ptr);
+	pr_info("attempting bad read at %px\n", ptr);
 	tmp = *ptr;
 	tmp += 0xc0dec0de;
 
-	pr_info("attempting bad write at %p\n", ptr);
+	pr_info("attempting bad write at %px\n", ptr);
 	*ptr = tmp;
 
 	vm_munmap(user_addr, PAGE_SIZE);