linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] kasan: test: add globals left-out-of-bounds test
@ 2021-11-17 11:09 Marco Elver
  2021-11-17 12:59 ` Andrey Konovalov
  0 siblings, 1 reply; 3+ messages in thread
From: Marco Elver @ 2021-11-17 11:09 UTC (permalink / raw)
  To: elver, Andrew Morton
  Cc: Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov,
	Dmitry Vyukov, kasan-dev, linux-kernel, linux-mm,
	Kaiwan N Billimoria

Add a test checking that KASAN generic can also detect out-of-bounds
accesses to the left of globals.

Unfortunately it seems that GCC doesn't catch this (tested GCC 10, 11).
The main difference between GCC's globals redzoning and Clang's is that
GCC relies on using increased alignment to producing padding, where
Clang's redzoning implementation actually adds real data after the
global and doesn't rely on alignment to produce padding. I believe this
is the main reason why GCC can't reliably catch globals out-of-bounds in
this case.

Given this is now a known issue, to avoid failing the whole test suite,
skip this test case with GCC.

Reported-by: Kaiwan N Billimoria <kaiwan.billimoria@gmail.com>
Signed-off-by: Marco Elver <elver@google.com>
---
 lib/test_kasan.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index 67ed689a0b1b..69c32c91420b 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -700,7 +700,7 @@ static void kmem_cache_bulk(struct kunit *test)
 
 static char global_array[10];
 
-static void kasan_global_oob(struct kunit *test)
+static void kasan_global_oob_right(struct kunit *test)
 {
 	/*
 	 * Deliberate out-of-bounds access. To prevent CONFIG_UBSAN_LOCAL_BOUNDS
@@ -723,6 +723,19 @@ static void kasan_global_oob(struct kunit *test)
 	KUNIT_EXPECT_KASAN_FAIL(test, *(volatile char *)p);
 }
 
+static void kasan_global_oob_left(struct kunit *test)
+{
+	char *volatile array = global_array;
+	char *p = array - 3;
+
+	/*
+	 * GCC is known to fail this test, skip it.
+	 */
+	KASAN_TEST_NEEDS_CONFIG_ON(test, CONFIG_CC_IS_CLANG);
+	KASAN_TEST_NEEDS_CONFIG_ON(test, CONFIG_KASAN_GENERIC);
+	KUNIT_EXPECT_KASAN_FAIL(test, *(volatile char *)p);
+}
+
 /* Check that ksize() makes the whole object accessible. */
 static void ksize_unpoisons_memory(struct kunit *test)
 {
@@ -1160,7 +1173,8 @@ static struct kunit_case kasan_kunit_test_cases[] = {
 	KUNIT_CASE(kmem_cache_oob),
 	KUNIT_CASE(kmem_cache_accounted),
 	KUNIT_CASE(kmem_cache_bulk),
-	KUNIT_CASE(kasan_global_oob),
+	KUNIT_CASE(kasan_global_oob_right),
+	KUNIT_CASE(kasan_global_oob_left),
 	KUNIT_CASE(kasan_stack_oob),
 	KUNIT_CASE(kasan_alloca_oob_left),
 	KUNIT_CASE(kasan_alloca_oob_right),
-- 
2.34.0.rc2.393.gf8c9666880-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] kasan: test: add globals left-out-of-bounds test
  2021-11-17 11:09 [PATCH] kasan: test: add globals left-out-of-bounds test Marco Elver
@ 2021-11-17 12:59 ` Andrey Konovalov
  2021-11-17 13:04   ` Marco Elver
  0 siblings, 1 reply; 3+ messages in thread
From: Andrey Konovalov @ 2021-11-17 12:59 UTC (permalink / raw)
  To: Marco Elver
  Cc: Andrew Morton, Andrey Ryabinin, Alexander Potapenko,
	Dmitry Vyukov, kasan-dev, LKML, Linux Memory Management List,
	Kaiwan N Billimoria

On Wed, Nov 17, 2021 at 12:09 PM Marco Elver <elver@google.com> wrote:
>
> Add a test checking that KASAN generic can also detect out-of-bounds
> accesses to the left of globals.
>
> Unfortunately it seems that GCC doesn't catch this (tested GCC 10, 11).
> The main difference between GCC's globals redzoning and Clang's is that
> GCC relies on using increased alignment to producing padding, where
> Clang's redzoning implementation actually adds real data after the
> global and doesn't rely on alignment to produce padding. I believe this
> is the main reason why GCC can't reliably catch globals out-of-bounds in
> this case.
>
> Given this is now a known issue, to avoid failing the whole test suite,
> skip this test case with GCC.
>
> Reported-by: Kaiwan N Billimoria <kaiwan.billimoria@gmail.com>
> Signed-off-by: Marco Elver <elver@google.com>

Hi Marco,

> ---
>  lib/test_kasan.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/lib/test_kasan.c b/lib/test_kasan.c
> index 67ed689a0b1b..69c32c91420b 100644
> --- a/lib/test_kasan.c
> +++ b/lib/test_kasan.c
> @@ -700,7 +700,7 @@ static void kmem_cache_bulk(struct kunit *test)
>
>  static char global_array[10];
>
> -static void kasan_global_oob(struct kunit *test)
> +static void kasan_global_oob_right(struct kunit *test)
>  {
>         /*
>          * Deliberate out-of-bounds access. To prevent CONFIG_UBSAN_LOCAL_BOUNDS
> @@ -723,6 +723,19 @@ static void kasan_global_oob(struct kunit *test)
>         KUNIT_EXPECT_KASAN_FAIL(test, *(volatile char *)p);
>  }
>
> +static void kasan_global_oob_left(struct kunit *test)
> +{
> +       char *volatile array = global_array;
> +       char *p = array - 3;
> +
> +       /*
> +        * GCC is known to fail this test, skip it.
> +        */

Please link the KASAN bugzilla issue here.

> +       KASAN_TEST_NEEDS_CONFIG_ON(test, CONFIG_CC_IS_CLANG);
> +       KASAN_TEST_NEEDS_CONFIG_ON(test, CONFIG_KASAN_GENERIC);
> +       KUNIT_EXPECT_KASAN_FAIL(test, *(volatile char *)p);
> +}
> +
>  /* Check that ksize() makes the whole object accessible. */
>  static void ksize_unpoisons_memory(struct kunit *test)
>  {
> @@ -1160,7 +1173,8 @@ static struct kunit_case kasan_kunit_test_cases[] = {
>         KUNIT_CASE(kmem_cache_oob),
>         KUNIT_CASE(kmem_cache_accounted),
>         KUNIT_CASE(kmem_cache_bulk),
> -       KUNIT_CASE(kasan_global_oob),
> +       KUNIT_CASE(kasan_global_oob_right),
> +       KUNIT_CASE(kasan_global_oob_left),
>         KUNIT_CASE(kasan_stack_oob),
>         KUNIT_CASE(kasan_alloca_oob_left),
>         KUNIT_CASE(kasan_alloca_oob_right),
> --
> 2.34.0.rc2.393.gf8c9666880-goog
>

Otherwise:

Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>

Thanks!

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] kasan: test: add globals left-out-of-bounds test
  2021-11-17 12:59 ` Andrey Konovalov
@ 2021-11-17 13:04   ` Marco Elver
  0 siblings, 0 replies; 3+ messages in thread
From: Marco Elver @ 2021-11-17 13:04 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Andrew Morton, Andrey Ryabinin, Alexander Potapenko,
	Dmitry Vyukov, kasan-dev, LKML, Linux Memory Management List,
	Kaiwan N Billimoria

On Wed, 17 Nov 2021 at 13:59, Andrey Konovalov <andreyknvl@gmail.com> wrote:
>
> On Wed, Nov 17, 2021 at 12:09 PM Marco Elver <elver@google.com> wrote:
> >
> > Add a test checking that KASAN generic can also detect out-of-bounds
> > accesses to the left of globals.
> >
> > Unfortunately it seems that GCC doesn't catch this (tested GCC 10, 11).
> > The main difference between GCC's globals redzoning and Clang's is that
> > GCC relies on using increased alignment to producing padding, where
> > Clang's redzoning implementation actually adds real data after the
> > global and doesn't rely on alignment to produce padding. I believe this
> > is the main reason why GCC can't reliably catch globals out-of-bounds in
> > this case.
> >
> > Given this is now a known issue, to avoid failing the whole test suite,
> > skip this test case with GCC.
> >
> > Reported-by: Kaiwan N Billimoria <kaiwan.billimoria@gmail.com>
> > Signed-off-by: Marco Elver <elver@google.com>
>
> Hi Marco,
>
> > ---
> >  lib/test_kasan.c | 18 ++++++++++++++++--
> >  1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/test_kasan.c b/lib/test_kasan.c
> > index 67ed689a0b1b..69c32c91420b 100644
> > --- a/lib/test_kasan.c
> > +++ b/lib/test_kasan.c
> > @@ -700,7 +700,7 @@ static void kmem_cache_bulk(struct kunit *test)
> >
> >  static char global_array[10];
> >
> > -static void kasan_global_oob(struct kunit *test)
> > +static void kasan_global_oob_right(struct kunit *test)
> >  {
> >         /*
> >          * Deliberate out-of-bounds access. To prevent CONFIG_UBSAN_LOCAL_BOUNDS
> > @@ -723,6 +723,19 @@ static void kasan_global_oob(struct kunit *test)
> >         KUNIT_EXPECT_KASAN_FAIL(test, *(volatile char *)p);
> >  }
> >
> > +static void kasan_global_oob_left(struct kunit *test)
> > +{
> > +       char *volatile array = global_array;
> > +       char *p = array - 3;
> > +
> > +       /*
> > +        * GCC is known to fail this test, skip it.
> > +        */
>
> Please link the KASAN bugzilla issue here.

I was wondering how to solve the cyclic dependency, because I wanted
to link this patch from the bugzilla.

Now that the bugzilla entry exists, I guess I can add it and then
update bugzilla with link to this patch closing the cycle. :-)

> > +       KASAN_TEST_NEEDS_CONFIG_ON(test, CONFIG_CC_IS_CLANG);
> > +       KASAN_TEST_NEEDS_CONFIG_ON(test, CONFIG_KASAN_GENERIC);
> > +       KUNIT_EXPECT_KASAN_FAIL(test, *(volatile char *)p);
> > +}
> > +
> >  /* Check that ksize() makes the whole object accessible. */
> >  static void ksize_unpoisons_memory(struct kunit *test)
> >  {
> > @@ -1160,7 +1173,8 @@ static struct kunit_case kasan_kunit_test_cases[] = {
> >         KUNIT_CASE(kmem_cache_oob),
> >         KUNIT_CASE(kmem_cache_accounted),
> >         KUNIT_CASE(kmem_cache_bulk),
> > -       KUNIT_CASE(kasan_global_oob),
> > +       KUNIT_CASE(kasan_global_oob_right),
> > +       KUNIT_CASE(kasan_global_oob_left),
> >         KUNIT_CASE(kasan_stack_oob),
> >         KUNIT_CASE(kasan_alloca_oob_left),
> >         KUNIT_CASE(kasan_alloca_oob_right),
> > --
> > 2.34.0.rc2.393.gf8c9666880-goog
> >
>
> Otherwise:
>
> Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>
>
> Thanks!
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/CA%2BfCnZcp3dFd3rwpLx6VUi2Yv9uqsWQyQNB6d3X-A7VgTjXUpw%40mail.gmail.com.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-11-17 13:04 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-17 11:09 [PATCH] kasan: test: add globals left-out-of-bounds test Marco Elver
2021-11-17 12:59 ` Andrey Konovalov
2021-11-17 13:04   ` Marco Elver

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