Introduce lkdtm tests for NULL pointer dereference: check
access or exec at NULL address.
Signed-off-by: Christophe Leroy <[email protected]>
---
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
On Fri, Dec 14, 2018 at 7:26 AM Christophe Leroy
<[email protected]> 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 <[email protected]>
> ---
> 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
Le 09/01/2019 à 02:14, Kees Cook a écrit :
> On Fri, Dec 14, 2018 at 7:26 AM Christophe Leroy
> <[email protected]> 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 <[email protected]>
>> ---
>> 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
>>
>
>
On Tue, Jan 8, 2019 at 10:31 PM Christophe Leroy
<[email protected]> wrote:
>
>
>
> Le 09/01/2019 à 02:14, Kees Cook a écrit :
> > On Fri, Dec 14, 2018 at 7:26 AM Christophe Leroy
> > <[email protected]> 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 <[email protected]>
-Kees
>
> Christophe
>
> >
> > -Kees
> >
> >>
> >> Signed-off-by: Christophe Leroy <[email protected]>
> >> ---
> >> 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
On Wed, Jan 9, 2019 at 7:16 AM Kees Cook <[email protected]> wrote:
>
> On Tue, Jan 8, 2019 at 10:31 PM Christophe Leroy
> <[email protected]> wrote:
> >
> >
> >
> > Le 09/01/2019 à 02:14, Kees Cook a écrit :
> > > On Fri, Dec 14, 2018 at 7:26 AM Christophe Leroy
> > > <[email protected]> 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 <[email protected]>
Applied to my lkdtm -next tree.
--
Kees Cook