diff mbox series

[3/8] kasan: test: avoid corrupting memory via memset

Message ID e9e2f7180f96e2496f0249ac81887376c6171e8f.1628709663.git.andreyknvl@gmail.com (mailing list archive)
State New
Headers show
Series kasan: test: avoid crashing the kernel with HW_TAGS | expand

Commit Message

andrey.konovalov@linux.dev Aug. 11, 2021, 7:21 p.m. UTC
From: Andrey Konovalov <andreyknvl@gmail.com>

kmalloc_oob_memset_*() tests do writes past the allocated objects.
As the result, they corrupt memory, which might lead to crashes with the
HW_TAGS mode, as it neither uses quarantine nor redzones.

Adjust the tests to only write memory within the aligned kmalloc objects.

Signed-off-by: Andrey Konovalov <andreyknvl@gmail.com>
---
 lib/test_kasan.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

Comments

Marco Elver Aug. 12, 2021, 8:56 a.m. UTC | #1
On Wed, 11 Aug 2021 at 21:21, <andrey.konovalov@linux.dev> wrote:
> From: Andrey Konovalov <andreyknvl@gmail.com>
>
> kmalloc_oob_memset_*() tests do writes past the allocated objects.
> As the result, they corrupt memory, which might lead to crashes with the
> HW_TAGS mode, as it neither uses quarantine nor redzones.
>
> Adjust the tests to only write memory within the aligned kmalloc objects.
>
> Signed-off-by: Andrey Konovalov <andreyknvl@gmail.com>
> ---
>  lib/test_kasan.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/lib/test_kasan.c b/lib/test_kasan.c
> index c82a82eb5393..fd00cd35e82c 100644
> --- a/lib/test_kasan.c
> +++ b/lib/test_kasan.c
> @@ -431,61 +431,61 @@ static void kmalloc_uaf_16(struct kunit *test)
>  static void kmalloc_oob_memset_2(struct kunit *test)
>  {
>         char *ptr;
> -       size_t size = 8;
> +       size_t size = 128 - KASAN_GRANULE_SIZE;
>
>         ptr = kmalloc(size, GFP_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>
> -       KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + 7 + OOB_TAG_OFF, 0, 2));
> +       KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + size, 0, 2));

I think one important aspect of these tests in generic mode is that
the written range touches both valid and invalid memory. I think that
was meant to test any explicit instrumentation isn't just looking at
the starting address, but at the whole range.

It seems that with these changes that is no longer tested. Could we
somehow make it still test that?


>         kfree(ptr);
>  }
>
>  static void kmalloc_oob_memset_4(struct kunit *test)
>  {
>         char *ptr;
> -       size_t size = 8;
> +       size_t size = 128 - KASAN_GRANULE_SIZE;
>
>         ptr = kmalloc(size, GFP_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>
> -       KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + 5 + OOB_TAG_OFF, 0, 4));
> +       KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + size, 0, 4));
>         kfree(ptr);
>  }
>
> -
>  static void kmalloc_oob_memset_8(struct kunit *test)
>  {
>         char *ptr;
> -       size_t size = 8;
> +       size_t size = 128 - KASAN_GRANULE_SIZE;
>
>         ptr = kmalloc(size, GFP_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>
> -       KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + 1 + OOB_TAG_OFF, 0, 8));
> +       KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + size, 0, 8));
>         kfree(ptr);
>  }
>
>  static void kmalloc_oob_memset_16(struct kunit *test)
>  {
>         char *ptr;
> -       size_t size = 16;
> +       size_t size = 128 - KASAN_GRANULE_SIZE;
>
>         ptr = kmalloc(size, GFP_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>
> -       KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + 1 + OOB_TAG_OFF, 0, 16));
> +       KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + size, 0, 16));
>         kfree(ptr);
>  }
>
>  static void kmalloc_oob_in_memset(struct kunit *test)
>  {
>         char *ptr;
> -       size_t size = 666;
> +       size_t size = 128 - KASAN_GRANULE_SIZE;
>
>         ptr = kmalloc(size, GFP_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>
> -       KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr, 0, size + 5 + OOB_TAG_OFF));
> +       KUNIT_EXPECT_KASAN_FAIL(test,
> +                               memset(ptr, 0, size + KASAN_GRANULE_SIZE));
>         kfree(ptr);
>  }
>
> --
> 2.25.1
>
> --
> 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/e9e2f7180f96e2496f0249ac81887376c6171e8f.1628709663.git.andreyknvl%40gmail.com.
Andrey Konovalov Aug. 12, 2021, 12:55 p.m. UTC | #2
On Thu, Aug 12, 2021 at 10:57 AM Marco Elver <elver@google.com> wrote:
>
> On Wed, 11 Aug 2021 at 21:21, <andrey.konovalov@linux.dev> wrote:
> > From: Andrey Konovalov <andreyknvl@gmail.com>
> >
> > kmalloc_oob_memset_*() tests do writes past the allocated objects.
> > As the result, they corrupt memory, which might lead to crashes with the
> > HW_TAGS mode, as it neither uses quarantine nor redzones.
> >
> > Adjust the tests to only write memory within the aligned kmalloc objects.
> >
> > Signed-off-by: Andrey Konovalov <andreyknvl@gmail.com>
> > ---
> >  lib/test_kasan.c | 22 +++++++++++-----------
> >  1 file changed, 11 insertions(+), 11 deletions(-)
> >
> > diff --git a/lib/test_kasan.c b/lib/test_kasan.c
> > index c82a82eb5393..fd00cd35e82c 100644
> > --- a/lib/test_kasan.c
> > +++ b/lib/test_kasan.c
> > @@ -431,61 +431,61 @@ static void kmalloc_uaf_16(struct kunit *test)
> >  static void kmalloc_oob_memset_2(struct kunit *test)
> >  {
> >         char *ptr;
> > -       size_t size = 8;
> > +       size_t size = 128 - KASAN_GRANULE_SIZE;
> >
> >         ptr = kmalloc(size, GFP_KERNEL);
> >         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
> >
> > -       KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + 7 + OOB_TAG_OFF, 0, 2));
> > +       KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + size, 0, 2));
>
> I think one important aspect of these tests in generic mode is that
> the written range touches both valid and invalid memory. I think that
> was meant to test any explicit instrumentation isn't just looking at
> the starting address, but at the whole range.

Good point!

> It seems that with these changes that is no longer tested. Could we
> somehow make it still test that?

Yes, will do in v2.

Thanks, Marco!
diff mbox series

Patch

diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index c82a82eb5393..fd00cd35e82c 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -431,61 +431,61 @@  static void kmalloc_uaf_16(struct kunit *test)
 static void kmalloc_oob_memset_2(struct kunit *test)
 {
 	char *ptr;
-	size_t size = 8;
+	size_t size = 128 - KASAN_GRANULE_SIZE;
 
 	ptr = kmalloc(size, GFP_KERNEL);
 	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
 
-	KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + 7 + OOB_TAG_OFF, 0, 2));
+	KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + size, 0, 2));
 	kfree(ptr);
 }
 
 static void kmalloc_oob_memset_4(struct kunit *test)
 {
 	char *ptr;
-	size_t size = 8;
+	size_t size = 128 - KASAN_GRANULE_SIZE;
 
 	ptr = kmalloc(size, GFP_KERNEL);
 	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
 
-	KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + 5 + OOB_TAG_OFF, 0, 4));
+	KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + size, 0, 4));
 	kfree(ptr);
 }
 
-
 static void kmalloc_oob_memset_8(struct kunit *test)
 {
 	char *ptr;
-	size_t size = 8;
+	size_t size = 128 - KASAN_GRANULE_SIZE;
 
 	ptr = kmalloc(size, GFP_KERNEL);
 	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
 
-	KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + 1 + OOB_TAG_OFF, 0, 8));
+	KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + size, 0, 8));
 	kfree(ptr);
 }
 
 static void kmalloc_oob_memset_16(struct kunit *test)
 {
 	char *ptr;
-	size_t size = 16;
+	size_t size = 128 - KASAN_GRANULE_SIZE;
 
 	ptr = kmalloc(size, GFP_KERNEL);
 	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
 
-	KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + 1 + OOB_TAG_OFF, 0, 16));
+	KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + size, 0, 16));
 	kfree(ptr);
 }
 
 static void kmalloc_oob_in_memset(struct kunit *test)
 {
 	char *ptr;
-	size_t size = 666;
+	size_t size = 128 - KASAN_GRANULE_SIZE;
 
 	ptr = kmalloc(size, GFP_KERNEL);
 	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
 
-	KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr, 0, size + 5 + OOB_TAG_OFF));
+	KUNIT_EXPECT_KASAN_FAIL(test,
+				memset(ptr, 0, size + KASAN_GRANULE_SIZE));
 	kfree(ptr);
 }