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

[Andrey Konovalov]

On Sun, Oct 13, 2024 at 7:32 PM Nihar Chaithanya
<niharchaithanya@xxxxxxxxx> wrote:
>
> The Kunit tests for kmalloc_track_caller and kmalloc_node_track_caller
> are 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 unaligned, aligned and
>    beyond-aligned out-of-bounds access test for kmalloc_track_caller and
>    out-of-bounds access test for kmalloc_node_track_caller.
>
> Signed-off-by: Nihar Chaithanya <niharchaithanya@xxxxxxxxx>

You can add a Fixes tag here to link the patch the Bugzilla entry:

Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=216509

> ---
>  mm/kasan/kasan_test_c.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>
> diff --git a/mm/kasan/kasan_test_c.c b/mm/kasan/kasan_test_c.c
> index a181e4780d9d..b418bdff5bdb 100644
> --- a/mm/kasan/kasan_test_c.c
> +++ b/mm/kasan/kasan_test_c.c
> @@ -213,6 +213,39 @@ static void kmalloc_node_oob_right(struct kunit *test)
>         kfree(ptr);
>  }
>
> +static void kmalloc_track_caller_oob_right(struct kunit *test)

Let's simplify this and do a single bad access check here for each
kmalloc_track_caller and kmalloc_node_track_caller. Precise redzone
poisoning checks are already done in normal kmalloc tests. This test
is just intended to be sure that we didn't forget to include KASAN
instrumentation calls into the track_caller variants.

> +{
> +       char *ptr;
> +       size_t size = 128 - KASAN_GRANULE_SIZE - 5;

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);
> +       if (IS_ENABLED(CONFIG_KASAN_GENERIC))
> +               KUNIT_EXPECT_KASAN_FAIL(test, ptr[size] = 'x');
> +       KUNIT_EXPECT_KASAN_FAIL(test, ptr[size + 5] = 'y');
> +       KUNIT_EXPECT_KASAN_FAIL(test, ptr[0] =
> +                                       ptr[size + KASAN_GRANULE_SIZE + 5]);

Just one check for all KASAN modes here:

KUNIT_EXPECT_KASAN_FAIL(test, ptr[size] = 'y');

Nit: add empty line before kfree().

> +       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[0] = ptr[size]);

Nit: add empty line before kfree().

> +       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 +1991,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
>