[PATCH] lkdtm: Add a tests for NULL pointer dereference

Subsystems: char and misc drivers, linux kernel dump test module (lkdtm), the rest

STALE2761d

5 messages, 2 authors, 2019-01-09 · open the first message on its own page

[PATCH] lkdtm: Add a tests for NULL pointer dereference

From: Christophe Leroy <hidden>
Date: 2018-12-14 15:26:24

Introduce lkdtm tests for NULL pointer dereference: check
access or exec at NULL address.

Signed-off-by: Christophe Leroy <redacted>
---
 drivers/misc/lkdtm/core.c  |  2 ++
 drivers/misc/lkdtm/lkdtm.h |  2 ++
 drivers/misc/lkdtm/perms.c | 18 ++++++++++++++++++
 3 files changed, 22 insertions(+)

diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
index bc76756b7eda..36910e1d5c09 100644
--- a/drivers/misc/lkdtm/core.c
+++ b/drivers/misc/lkdtm/core.c

@@ -157,7 +157,9 @@ static const struct crashtype crashtypes[] = {
 	CRASHTYPE(EXEC_VMALLOC),
 	CRASHTYPE(EXEC_RODATA),
 	CRASHTYPE(EXEC_USERSPACE),
+	CRASHTYPE(EXEC_NULL),
 	CRASHTYPE(ACCESS_USERSPACE),
+	CRASHTYPE(ACCESS_NULL),
 	CRASHTYPE(WRITE_RO),
 	CRASHTYPE(WRITE_RO_AFTER_INIT),
 	CRASHTYPE(WRITE_KERN),

diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
index 3c6fd327e166..b69ee004a3f7 100644
--- a/drivers/misc/lkdtm/lkdtm.h
+++ b/drivers/misc/lkdtm/lkdtm.h

@@ -45,7 +45,9 @@ void lkdtm_EXEC_KMALLOC(void);
 void lkdtm_EXEC_VMALLOC(void);
 void lkdtm_EXEC_RODATA(void);
 void lkdtm_EXEC_USERSPACE(void);
+void lkdtm_EXEC_NULL(void);
 void lkdtm_ACCESS_USERSPACE(void);
+void lkdtm_ACCESS_NULL(void);
 
 /* lkdtm_refcount.c */
 void lkdtm_REFCOUNT_INC_OVERFLOW(void);

diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index fa54add6375a..62f76d506f04 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c

@@ -164,6 +164,11 @@ void lkdtm_EXEC_USERSPACE(void)
 	vm_munmap(user_addr, PAGE_SIZE);
 }
 
+void lkdtm_EXEC_NULL(void)
+{
+	execute_location(NULL, CODE_AS_IS);
+}
+
 void lkdtm_ACCESS_USERSPACE(void)
 {
 	unsigned long user_addr, tmp = 0;

@@ -195,6 +200,19 @@ void lkdtm_ACCESS_USERSPACE(void)
 	vm_munmap(user_addr, PAGE_SIZE);
 }
 
+void lkdtm_ACCESS_NULL(void)
+{
+	unsigned long tmp;
+	unsigned long *ptr = (unsigned long *)NULL;
+
+	pr_info("attempting bad read at %px\n", ptr);
+	tmp = *ptr;
+	tmp += 0xc0dec0de;
+
+	pr_info("attempting bad write at %px\n", ptr);
+	*ptr = tmp;
+}
+
 void __init lkdtm_perms_init(void)
 {
 	/* Make sure we can write to __ro_after_init values during __init */

-- 
2.13.3

Re: [PATCH] lkdtm: Add a tests for NULL pointer dereference

From: Kees Cook <hidden>
Date: 2019-01-09 01:14:53

On Fri, Dec 14, 2018 at 7:26 AM Christophe Leroy
[off-list ref] wrote:

Introduce lkdtm tests for NULL pointer dereference: check
access or exec at NULL address.

Why is this not already covered by the existing tests? (Is there
something special about NULL that is being missed?) I'd expect SMAP
and SMEP to cover NULL as well.

-Kees

quoted hunk

Signed-off-by: Christophe Leroy <redacted>
---
 drivers/misc/lkdtm/core.c  |  2 ++
 drivers/misc/lkdtm/lkdtm.h |  2 ++
 drivers/misc/lkdtm/perms.c | 18 ++++++++++++++++++
 3 files changed, 22 insertions(+)

diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
index bc76756b7eda..36910e1d5c09 100644
--- a/drivers/misc/lkdtm/core.c
+++ b/drivers/misc/lkdtm/core.c

@@ -157,7 +157,9 @@ static const struct crashtype crashtypes[] = {
        CRASHTYPE(EXEC_VMALLOC),
        CRASHTYPE(EXEC_RODATA),
        CRASHTYPE(EXEC_USERSPACE),
+       CRASHTYPE(EXEC_NULL),
        CRASHTYPE(ACCESS_USERSPACE),
+       CRASHTYPE(ACCESS_NULL),
        CRASHTYPE(WRITE_RO),
        CRASHTYPE(WRITE_RO_AFTER_INIT),
        CRASHTYPE(WRITE_KERN),

diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
index 3c6fd327e166..b69ee004a3f7 100644
--- a/drivers/misc/lkdtm/lkdtm.h
+++ b/drivers/misc/lkdtm/lkdtm.h

@@ -45,7 +45,9 @@ void lkdtm_EXEC_KMALLOC(void);
 void lkdtm_EXEC_VMALLOC(void);
 void lkdtm_EXEC_RODATA(void);
 void lkdtm_EXEC_USERSPACE(void);
+void lkdtm_EXEC_NULL(void);
 void lkdtm_ACCESS_USERSPACE(void);
+void lkdtm_ACCESS_NULL(void);

 /* lkdtm_refcount.c */
 void lkdtm_REFCOUNT_INC_OVERFLOW(void);

diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index fa54add6375a..62f76d506f04 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c

@@ -164,6 +164,11 @@ void lkdtm_EXEC_USERSPACE(void)
        vm_munmap(user_addr, PAGE_SIZE);
 }

+void lkdtm_EXEC_NULL(void)
+{
+       execute_location(NULL, CODE_AS_IS);
+}
+
 void lkdtm_ACCESS_USERSPACE(void)
 {
        unsigned long user_addr, tmp = 0;

@@ -195,6 +200,19 @@ void lkdtm_ACCESS_USERSPACE(void)
        vm_munmap(user_addr, PAGE_SIZE);
 }

+void lkdtm_ACCESS_NULL(void)
+{
+       unsigned long tmp;
+       unsigned long *ptr = (unsigned long *)NULL;
+
+       pr_info("attempting bad read at %px\n", ptr);
+       tmp = *ptr;
+       tmp += 0xc0dec0de;
+
+       pr_info("attempting bad write at %px\n", ptr);
+       *ptr = tmp;
+}
+
 void __init lkdtm_perms_init(void)
 {
        /* Make sure we can write to __ro_after_init values during __init */
--

2.13.3


-- 
Kees Cook

Re: [PATCH] lkdtm: Add a tests for NULL pointer dereference

From: Christophe Leroy <hidden>
Date: 2019-01-09 06:31:23


Le 09/01/2019 à 02:14, Kees Cook a écrit :

On Fri, Dec 14, 2018 at 7:26 AM Christophe Leroy
[off-list ref] wrote:

quoted

Introduce lkdtm tests for NULL pointer dereference: check
access or exec at NULL address.

Why is this not already covered by the existing tests? (Is there
something special about NULL that is being missed?) I'd expect SMAP
and SMEP to cover NULL as well.

Most arches print a different message whether the faulty address is 
above or under PAGE_SIZE. Below is exemple from x86:

	pr_alert("BUG: unable to handle kernel %s at %px\n",
		 address < PAGE_SIZE ? "NULL pointer dereference" : "paging request",
		 (void *)address);


Until recently, the powerpc arch didn't do it. When I implemented it 
(https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=49a502ea23bf9dec47f8f3c3960909ff409cd1bb), 
I needed a way to test it and couldn't find an existing one, hence this 
new LKDTM test.

But maybe I missed something ?

Christophe

-Kees

quoted

Signed-off-by: Christophe Leroy <redacted>
---
  drivers/misc/lkdtm/core.c  |  2 ++
  drivers/misc/lkdtm/lkdtm.h |  2 ++
  drivers/misc/lkdtm/perms.c | 18 ++++++++++++++++++
  3 files changed, 22 insertions(+)

diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
index bc76756b7eda..36910e1d5c09 100644
--- a/drivers/misc/lkdtm/core.c
+++ b/drivers/misc/lkdtm/core.c

@@ -157,7 +157,9 @@ static const struct crashtype crashtypes[] = {
         CRASHTYPE(EXEC_VMALLOC),
         CRASHTYPE(EXEC_RODATA),
         CRASHTYPE(EXEC_USERSPACE),
+       CRASHTYPE(EXEC_NULL),
         CRASHTYPE(ACCESS_USERSPACE),
+       CRASHTYPE(ACCESS_NULL),
         CRASHTYPE(WRITE_RO),
         CRASHTYPE(WRITE_RO_AFTER_INIT),
         CRASHTYPE(WRITE_KERN),

diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
index 3c6fd327e166..b69ee004a3f7 100644
--- a/drivers/misc/lkdtm/lkdtm.h
+++ b/drivers/misc/lkdtm/lkdtm.h

@@ -45,7 +45,9 @@ void lkdtm_EXEC_KMALLOC(void);
  void lkdtm_EXEC_VMALLOC(void);
  void lkdtm_EXEC_RODATA(void);
  void lkdtm_EXEC_USERSPACE(void);
+void lkdtm_EXEC_NULL(void);
  void lkdtm_ACCESS_USERSPACE(void);
+void lkdtm_ACCESS_NULL(void);

  /* lkdtm_refcount.c */
  void lkdtm_REFCOUNT_INC_OVERFLOW(void);

diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index fa54add6375a..62f76d506f04 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c

@@ -164,6 +164,11 @@ void lkdtm_EXEC_USERSPACE(void)
         vm_munmap(user_addr, PAGE_SIZE);
  }

+void lkdtm_EXEC_NULL(void)
+{
+       execute_location(NULL, CODE_AS_IS);
+}
+
  void lkdtm_ACCESS_USERSPACE(void)
  {
         unsigned long user_addr, tmp = 0;

@@ -195,6 +200,19 @@ void lkdtm_ACCESS_USERSPACE(void)
         vm_munmap(user_addr, PAGE_SIZE);
  }

+void lkdtm_ACCESS_NULL(void)
+{
+       unsigned long tmp;
+       unsigned long *ptr = (unsigned long *)NULL;
+
+       pr_info("attempting bad read at %px\n", ptr);
+       tmp = *ptr;
+       tmp += 0xc0dec0de;
+
+       pr_info("attempting bad write at %px\n", ptr);
+       *ptr = tmp;
+}
+
  void __init lkdtm_perms_init(void)
  {
         /* Make sure we can write to __ro_after_init values during __init */
--

2.13.3

Re: [PATCH] lkdtm: Add a tests for NULL pointer dereference

From: Kees Cook <hidden>
Date: 2019-01-09 15:17:12

On Tue, Jan 8, 2019 at 10:31 PM Christophe Leroy
[off-list ref] wrote:



Le 09/01/2019 à 02:14, Kees Cook a écrit :

quoted

On Fri, Dec 14, 2018 at 7:26 AM Christophe Leroy
[off-list ref] wrote:

quoted

Introduce lkdtm tests for NULL pointer dereference: check
access or exec at NULL address.

Why is this not already covered by the existing tests? (Is there
something special about NULL that is being missed?) I'd expect SMAP
and SMEP to cover NULL as well.

Most arches print a different message whether the faulty address is
above or under PAGE_SIZE. Below is exemple from x86:

        pr_alert("BUG: unable to handle kernel %s at %px\n",
                 address < PAGE_SIZE ? "NULL pointer dereference" : "paging request",
                 (void *)address);


Until recently, the powerpc arch didn't do it. When I implemented it
(https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=49a502ea23bf9dec47f8f3c3960909ff409cd1bb),
I needed a way to test it and couldn't find an existing one, hence this
new LKDTM test.

But maybe I missed something ?

Okay, gotcha. You're getting more complete reporting coverage. Sounds
good to me. Thanks!

Acked-by: Kees Cook <redacted>

-Kees

Christophe

quoted

-Kees

quoted

Signed-off-by: Christophe Leroy <redacted>
---
  drivers/misc/lkdtm/core.c  |  2 ++
  drivers/misc/lkdtm/lkdtm.h |  2 ++
  drivers/misc/lkdtm/perms.c | 18 ++++++++++++++++++
  3 files changed, 22 insertions(+)

diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
index bc76756b7eda..36910e1d5c09 100644
--- a/drivers/misc/lkdtm/core.c
+++ b/drivers/misc/lkdtm/core.c

@@ -157,7 +157,9 @@ static const struct crashtype crashtypes[] = {
         CRASHTYPE(EXEC_VMALLOC),
         CRASHTYPE(EXEC_RODATA),
         CRASHTYPE(EXEC_USERSPACE),
+       CRASHTYPE(EXEC_NULL),
         CRASHTYPE(ACCESS_USERSPACE),
+       CRASHTYPE(ACCESS_NULL),
         CRASHTYPE(WRITE_RO),
         CRASHTYPE(WRITE_RO_AFTER_INIT),
         CRASHTYPE(WRITE_KERN),

diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
index 3c6fd327e166..b69ee004a3f7 100644
--- a/drivers/misc/lkdtm/lkdtm.h
+++ b/drivers/misc/lkdtm/lkdtm.h

@@ -45,7 +45,9 @@ void lkdtm_EXEC_KMALLOC(void);
  void lkdtm_EXEC_VMALLOC(void);
  void lkdtm_EXEC_RODATA(void);
  void lkdtm_EXEC_USERSPACE(void);
+void lkdtm_EXEC_NULL(void);
  void lkdtm_ACCESS_USERSPACE(void);
+void lkdtm_ACCESS_NULL(void);

  /* lkdtm_refcount.c */
  void lkdtm_REFCOUNT_INC_OVERFLOW(void);

diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index fa54add6375a..62f76d506f04 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c

@@ -164,6 +164,11 @@ void lkdtm_EXEC_USERSPACE(void)
         vm_munmap(user_addr, PAGE_SIZE);
  }

+void lkdtm_EXEC_NULL(void)
+{
+       execute_location(NULL, CODE_AS_IS);
+}
+
  void lkdtm_ACCESS_USERSPACE(void)
  {
         unsigned long user_addr, tmp = 0;

@@ -195,6 +200,19 @@ void lkdtm_ACCESS_USERSPACE(void)
         vm_munmap(user_addr, PAGE_SIZE);
  }

+void lkdtm_ACCESS_NULL(void)
+{
+       unsigned long tmp;
+       unsigned long *ptr = (unsigned long *)NULL;
+
+       pr_info("attempting bad read at %px\n", ptr);
+       tmp = *ptr;
+       tmp += 0xc0dec0de;
+
+       pr_info("attempting bad write at %px\n", ptr);
+       *ptr = tmp;
+}
+
  void __init lkdtm_perms_init(void)
  {
         /* Make sure we can write to __ro_after_init values during __init */
--

2.13.3



-- 
Kees Cook

Re: [PATCH] lkdtm: Add a tests for NULL pointer dereference

From: Kees Cook <hidden>
Date: 2019-01-09 20:05:20

On Wed, Jan 9, 2019 at 7:16 AM Kees Cook [off-list ref] wrote:

On Tue, Jan 8, 2019 at 10:31 PM Christophe Leroy
[off-list ref] wrote:

quoted



Le 09/01/2019 à 02:14, Kees Cook a écrit :

quoted

On Fri, Dec 14, 2018 at 7:26 AM Christophe Leroy
[off-list ref] wrote:

quoted

Introduce lkdtm tests for NULL pointer dereference: check
access or exec at NULL address.

Why is this not already covered by the existing tests? (Is there
something special about NULL that is being missed?) I'd expect SMAP
and SMEP to cover NULL as well.

Most arches print a different message whether the faulty address is
above or under PAGE_SIZE. Below is exemple from x86:

        pr_alert("BUG: unable to handle kernel %s at %px\n",
                 address < PAGE_SIZE ? "NULL pointer dereference" : "paging request",
                 (void *)address);


Until recently, the powerpc arch didn't do it. When I implemented it
(https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=49a502ea23bf9dec47f8f3c3960909ff409cd1bb),
I needed a way to test it and couldn't find an existing one, hence this
new LKDTM test.

But maybe I missed something ?

Okay, gotcha. You're getting more complete reporting coverage. Sounds
good to me. Thanks!

Acked-by: Kees Cook <redacted>

Applied to my lkdtm -next tree.

-- 
Kees Cook

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help