[PATCH v2] kasan: add kunit tests for kmalloc_track_caller, kmalloc_node_track_caller

[PATCH v2] kasan: add kunit tests for kmalloc_track_caller, kmalloc_node_track_caller

[Nihar Chaithanya]

The Kunit tests for kmalloc_track_caller and kmalloc_node_track_caller
were missing in kasan_test_c.c, which check that these functions poison
the memory properly.

Add a Kunit test:
-> kmalloc_tracker_caller_oob_right(): This includes out-of-bounds
   access test for kmalloc_track_caller and kmalloc_node_track_caller.

Signed-off-by: Nihar Chaithanya <niharchaithanya@gmail.com>
Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=216509
---
v1->v2: Simplified the three separate out-of-bounds tests to a single test for
kmalloc_track_caller.

Link to v1: https://lore.kernel.org/all/20241013172912.1047136-1-niharchaithanya@gmail.com/

 mm/kasan/kasan_test_c.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/mm/kasan/kasan_test_c.c b/mm/kasan/kasan_test_c.c
index a181e4780d9d..62efc1ee9612 100644
--- a/mm/kasan/kasan_test_c.c
+++ b/mm/kasan/kasan_test_c.c
@@ -213,6 +213,37 @@ static void kmalloc_node_oob_right(struct kunit *test)
 	kfree(ptr);
 }
 
+static void kmalloc_track_caller_oob_right(struct kunit *test)
+{
+	char *ptr;
+	size_t size = 128 - KASAN_GRANULE_SIZE;
+
+	/*
+	 * Check that KASAN detects out-of-bounds access for object allocated via
+	 * kmalloc_track_caller().
+	 */
+	ptr = kmalloc_track_caller(size, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
+
+	OPTIMIZER_HIDE_VAR(ptr);
+	KUNIT_EXPECT_KASAN_FAIL(test, ptr[size] = 'y');
+
+	kfree(ptr);
+
+	/*
+	 * Check that KASAN detects out-of-bounds access for object allocated via
+	 * kmalloc_node_track_caller().
+	 */
+	size = 4096;
+	ptr = kmalloc_node_track_caller(size, GFP_KERNEL, 0);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
+
+	OPTIMIZER_HIDE_VAR(ptr);
+	KUNIT_EXPECT_KASAN_FAIL(test, ptr[size] = 'y');
+
+	kfree(ptr);
+}
+
 /*
  * Check that KASAN detects an out-of-bounds access for a big object allocated
  * via kmalloc(). But not as big as to trigger the page_alloc fallback.
@@ -1958,6 +1989,7 @@ static struct kunit_case kasan_kunit_test_cases[] = {
 	KUNIT_CASE(kmalloc_oob_right),
 	KUNIT_CASE(kmalloc_oob_left),
 	KUNIT_CASE(kmalloc_node_oob_right),
+	KUNIT_CASE(kmalloc_track_caller_oob_right),
 	KUNIT_CASE(kmalloc_big_oob_right),
 	KUNIT_CASE(kmalloc_large_oob_right),
 	KUNIT_CASE(kmalloc_large_uaf),
-- 
2.34.1

Re: [PATCH v2] kasan: add kunit tests for kmalloc_track_caller, kmalloc_node_track_caller

[Andrey Konovalov]

On Mon, Oct 14, 2024 at 6:32 AM Nihar Chaithanya
<niharchaithanya@gmail.com> wrote:
>
> The Kunit tests for kmalloc_track_caller and kmalloc_node_track_caller
> were missing in kasan_test_c.c, which check that these functions poison
> the memory properly.
>
> Add a Kunit test:
> -> kmalloc_tracker_caller_oob_right(): This includes out-of-bounds
>    access test for kmalloc_track_caller and kmalloc_node_track_caller.
>
> Signed-off-by: Nihar Chaithanya <niharchaithanya@gmail.com>
> Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=216509
> ---
> v1->v2: Simplified the three separate out-of-bounds tests to a single test for
> kmalloc_track_caller.
>
> Link to v1: https://lore.kernel.org/all/20241013172912.1047136-1-niharchaithanya@gmail.com/
>
>  mm/kasan/kasan_test_c.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/mm/kasan/kasan_test_c.c b/mm/kasan/kasan_test_c.c
> index a181e4780d9d..62efc1ee9612 100644
> --- a/mm/kasan/kasan_test_c.c
> +++ b/mm/kasan/kasan_test_c.c
> @@ -213,6 +213,37 @@ static void kmalloc_node_oob_right(struct kunit *test)
>         kfree(ptr);
>  }
>
> +static void kmalloc_track_caller_oob_right(struct kunit *test)
> +{
> +       char *ptr;
> +       size_t size = 128 - KASAN_GRANULE_SIZE;
> +
> +       /*
> +        * Check that KASAN detects out-of-bounds access for object allocated via
> +        * kmalloc_track_caller().
> +        */
> +       ptr = kmalloc_track_caller(size, GFP_KERNEL);
> +       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
> +
> +       OPTIMIZER_HIDE_VAR(ptr);
> +       KUNIT_EXPECT_KASAN_FAIL(test, ptr[size] = 'y');
> +
> +       kfree(ptr);
> +
> +       /*
> +        * Check that KASAN detects out-of-bounds access for object allocated via
> +        * kmalloc_node_track_caller().
> +        */
> +       size = 4096;
> +       ptr = kmalloc_node_track_caller(size, GFP_KERNEL, 0);
> +       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
> +
> +       OPTIMIZER_HIDE_VAR(ptr);
> +       KUNIT_EXPECT_KASAN_FAIL(test, ptr[size] = 'y');

What you had here before (ptr[0] = ptr[size]) was better. ptr[size] =
'y' with size == 4096 does an out-of-bounds write access, which
corrupts uncontrolled memory for the tag-based KASAN modes, which do
not use redzones. We try to avoid corrupting memory in KASAN tests, as
the kernel might crash otherwise before all tests complete.

So let's either change this back to ptr[0] = ptr[size] or just reuse
the same size for both test cases (or does kmalloc_node_track_caller
require size >= 4K?).

> +
> +       kfree(ptr);
> +}
> +
>  /*
>   * Check that KASAN detects an out-of-bounds access for a big object allocated
>   * via kmalloc(). But not as big as to trigger the page_alloc fallback.
> @@ -1958,6 +1989,7 @@ static struct kunit_case kasan_kunit_test_cases[] = {
>         KUNIT_CASE(kmalloc_oob_right),
>         KUNIT_CASE(kmalloc_oob_left),
>         KUNIT_CASE(kmalloc_node_oob_right),
> +       KUNIT_CASE(kmalloc_track_caller_oob_right),
>         KUNIT_CASE(kmalloc_big_oob_right),
>         KUNIT_CASE(kmalloc_large_oob_right),
>         KUNIT_CASE(kmalloc_large_uaf),
> --
> 2.34.1
>

Re: [PATCH v2] kasan: add kunit tests for kmalloc_track_caller, kmalloc_node_track_caller

[Nihar Chaithanya]

On 14/10/24 18:19, Andrey Konovalov wrote:
> On Mon, Oct 14, 2024 at 6:32 AM Nihar Chaithanya
> <niharchaithanya@gmail.com> wrote:
>> The Kunit tests for kmalloc_track_caller and kmalloc_node_track_caller
>> were missing in kasan_test_c.c, which check that these functions poison
>> the memory properly.
>>
>> Add a Kunit test:
>> -> kmalloc_tracker_caller_oob_right(): This includes out-of-bounds
>>     access test for kmalloc_track_caller and kmalloc_node_track_caller.
>>
>> Signed-off-by: Nihar Chaithanya <niharchaithanya@gmail.com>
>> Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=216509
>> ---
>> v1->v2: Simplified the three separate out-of-bounds tests to a single test for
>> kmalloc_track_caller.
>>
>> Link to v1: https://lore.kernel.org/all/20241013172912.1047136-1-niharchaithanya@gmail.com/
>>
>>   mm/kasan/kasan_test_c.c | 32 ++++++++++++++++++++++++++++++++
>>   1 file changed, 32 insertions(+)
>>
>> diff --git a/mm/kasan/kasan_test_c.c b/mm/kasan/kasan_test_c.c
>> index a181e4780d9d..62efc1ee9612 100644
>> --- a/mm/kasan/kasan_test_c.c
>> +++ b/mm/kasan/kasan_test_c.c
>> @@ -213,6 +213,37 @@ static void kmalloc_node_oob_right(struct kunit *test)
>>          kfree(ptr);
>>   }
>>
>> +static void kmalloc_track_caller_oob_right(struct kunit *test)
>> +{
>> +       char *ptr;
>> +       size_t size = 128 - KASAN_GRANULE_SIZE;
>> +
>> +       /*
>> +        * Check that KASAN detects out-of-bounds access for object allocated via
>> +        * kmalloc_track_caller().
>> +        */
>> +       ptr = kmalloc_track_caller(size, GFP_KERNEL);
>> +       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>> +
>> +       OPTIMIZER_HIDE_VAR(ptr);
>> +       KUNIT_EXPECT_KASAN_FAIL(test, ptr[size] = 'y');
>> +
>> +       kfree(ptr);
>> +
>> +       /*
>> +        * Check that KASAN detects out-of-bounds access for object allocated via
>> +        * kmalloc_node_track_caller().
>> +        */
>> +       size = 4096;
>> +       ptr = kmalloc_node_track_caller(size, GFP_KERNEL, 0);
>> +       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>> +
>> +       OPTIMIZER_HIDE_VAR(ptr);
>> +       KUNIT_EXPECT_KASAN_FAIL(test, ptr[size] = 'y');
> What you had here before (ptr[0] = ptr[size]) was better. ptr[size] =
> 'y' with size == 4096 does an out-of-bounds write access, which
> corrupts uncontrolled memory for the tag-based KASAN modes, which do
> not use redzones. We try to avoid corrupting memory in KASAN tests, as
> the kernel might crash otherwise before all tests complete.
>
> So let's either change this back to ptr[0] = ptr[size] or just reuse
> the same size for both test cases (or does kmalloc_node_track_caller
> require size >= 4K?).

We can reuse the same test for both cases without changing the size, I ran
the test without changing the size (i.e., size == 128 - KASAN_GRANULE_SIZE),
the KASAN report was generated. I found instances (drm/tiny) where the size
passed to the kmalloc_node_track_caller is < 4k.

>> +
>> +       kfree(ptr);
>> +}
>> +
>>   /*
>>    * Check that KASAN detects an out-of-bounds access for a big object allocated
>>    * via kmalloc(). But not as big as to trigger the page_alloc fallback.
>> @@ -1958,6 +1989,7 @@ static struct kunit_case kasan_kunit_test_cases[] = {
>>          KUNIT_CASE(kmalloc_oob_right),
>>          KUNIT_CASE(kmalloc_oob_left),
>>          KUNIT_CASE(kmalloc_node_oob_right),
>> +       KUNIT_CASE(kmalloc_track_caller_oob_right),
>>          KUNIT_CASE(kmalloc_big_oob_right),
>>          KUNIT_CASE(kmalloc_large_oob_right),
>>          KUNIT_CASE(kmalloc_large_uaf),
>> --
>> 2.34.1
>>