linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] lkdtm: Add a tests for NULL pointer dereference
@ 2018-12-14 15:26 Christophe Leroy
  2019-01-09  1:14 ` Kees Cook
  0 siblings, 1 reply; 5+ messages in thread
From: Christophe Leroy @ 2018-12-14 15:26 UTC (permalink / raw)
  To: Kees Cook, Arnd Bergmann, Greg Kroah-Hartman; +Cc: linux-kernel, linuxppc-dev

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

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] lkdtm: Add a tests for NULL pointer dereference
  2018-12-14 15:26 [PATCH] lkdtm: Add a tests for NULL pointer dereference Christophe Leroy
@ 2019-01-09  1:14 ` Kees Cook
  2019-01-09  6:31   ` Christophe Leroy
  0 siblings, 1 reply; 5+ messages in thread
From: Kees Cook @ 2019-01-09  1:14 UTC (permalink / raw)
  To: Christophe Leroy; +Cc: Arnd Bergmann, Greg Kroah-Hartman, LKML, PowerPC

On Fri, Dec 14, 2018 at 7:26 AM Christophe Leroy
<christophe.leroy@c-s.fr> 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

>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
>  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

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] lkdtm: Add a tests for NULL pointer dereference
  2019-01-09  1:14 ` Kees Cook
@ 2019-01-09  6:31   ` Christophe Leroy
  2019-01-09 15:16     ` Kees Cook
  0 siblings, 1 reply; 5+ messages in thread
From: Christophe Leroy @ 2019-01-09  6:31 UTC (permalink / raw)
  To: Kees Cook; +Cc: Arnd Bergmann, Greg Kroah-Hartman, LKML, PowerPC



Le 09/01/2019 à 02:14, Kees Cook a écrit :
> On Fri, Dec 14, 2018 at 7:26 AM Christophe Leroy
> <christophe.leroy@c-s.fr> 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.

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
> 
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>>   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
>>
> 
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] lkdtm: Add a tests for NULL pointer dereference
  2019-01-09  6:31   ` Christophe Leroy
@ 2019-01-09 15:16     ` Kees Cook
  2019-01-09 20:05       ` Kees Cook
  0 siblings, 1 reply; 5+ messages in thread
From: Kees Cook @ 2019-01-09 15:16 UTC (permalink / raw)
  To: Christophe Leroy; +Cc: Arnd Bergmann, Greg Kroah-Hartman, LKML, PowerPC

On Tue, Jan 8, 2019 at 10:31 PM Christophe Leroy
<christophe.leroy@c-s.fr> wrote:
>
>
>
> Le 09/01/2019 à 02:14, Kees Cook a écrit :
> > On Fri, Dec 14, 2018 at 7:26 AM Christophe Leroy
> > <christophe.leroy@c-s.fr> 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.
>
> 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 <keescook@chromium.org>

-Kees

>
> Christophe
>
> >
> > -Kees
> >
> >>
> >> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> >> ---
> >>   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

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] lkdtm: Add a tests for NULL pointer dereference
  2019-01-09 15:16     ` Kees Cook
@ 2019-01-09 20:05       ` Kees Cook
  0 siblings, 0 replies; 5+ messages in thread
From: Kees Cook @ 2019-01-09 20:05 UTC (permalink / raw)
  To: Christophe Leroy; +Cc: Arnd Bergmann, Greg Kroah-Hartman, LKML, PowerPC

On Wed, Jan 9, 2019 at 7:16 AM Kees Cook <keescook@chromium.org> wrote:
>
> On Tue, Jan 8, 2019 at 10:31 PM Christophe Leroy
> <christophe.leroy@c-s.fr> wrote:
> >
> >
> >
> > Le 09/01/2019 à 02:14, Kees Cook a écrit :
> > > On Fri, Dec 14, 2018 at 7:26 AM Christophe Leroy
> > > <christophe.leroy@c-s.fr> 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.
> >
> > 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 <keescook@chromium.org>

Applied to my lkdtm -next tree.

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-01-09 20:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-14 15:26 [PATCH] lkdtm: Add a tests for NULL pointer dereference Christophe Leroy
2019-01-09  1:14 ` Kees Cook
2019-01-09  6:31   ` Christophe Leroy
2019-01-09 15:16     ` Kees Cook
2019-01-09 20:05       ` Kees Cook

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