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