Re: [PATCH 5/5] mm/slub, kunit: Add testcase for krealloc redzone and zeroing

From: Danilo Krummrich
Date: Tue Sep 10 2024 - 06:14:02 EST


On Mon, Sep 09, 2024 at 09:29:58AM +0800, Feng Tang wrote:
> Danilo Krummrich raised issue about krealloc+GFP_ZERO [1], and Vlastimil
> suggested to add some test case which can sanity test the kmalloc-redzone
> and zeroing by utilizing the kmalloc's 'orig_size' debug feature.
>
> It covers the grow and shrink case of krealloc() re-using current kmalloc
> object, and the case of re-allocating a new bigger object.
>
> User can add "slub_debug" kernel cmdline parameter to test it.
>
> [1]. https://lore.kernel.org/lkml/20240812223707.32049-1-dakr@xxxxxxxxxx/
>
> Suggested-by: Vlastimil Babka <vbabka@xxxxxxx>
> Signed-off-by: Feng Tang <feng.tang@xxxxxxxxx>

Reviewed-by: Danilo Krummrich <dakr@xxxxxxxxxx>

> ---
> lib/slub_kunit.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 46 insertions(+)
>
> diff --git a/lib/slub_kunit.c b/lib/slub_kunit.c
> index 6e3a1e5a7142..03e0089149ad 100644
> --- a/lib/slub_kunit.c
> +++ b/lib/slub_kunit.c
> @@ -186,6 +186,51 @@ static void test_leak_destroy(struct kunit *test)
> KUNIT_EXPECT_EQ(test, 1, slab_errors);
> }
>
> +static void test_krealloc_redzone_zeroing(struct kunit *test)
> +{
> + char *p;
> + int i;
> +
> + KUNIT_TEST_REQUIRES(test, __slub_debug_enabled());
> +
> + /* Allocate a 64B kmalloc object */
> + p = kzalloc(48, GFP_KERNEL);
> + if (unlikely(is_kfence_address(p))) {
> + kfree(p);
> + return;
> + }
> + memset(p, 0xff, 48);
> +
> + kasan_disable_current();
> + OPTIMIZER_HIDE_VAR(p);
> +
> + /* Test shrink */
> + p = krealloc(p, 40, GFP_KERNEL | __GFP_ZERO);
> + for (i = 40; i < 64; i++)
> + KUNIT_EXPECT_EQ(test, p[i], SLUB_RED_ACTIVE);
> +
> + /* Test grow within the same 64B kmalloc object */
> + p = krealloc(p, 56, GFP_KERNEL | __GFP_ZERO);
> + for (i = 40; i < 56; i++)
> + KUNIT_EXPECT_EQ(test, p[i], 0);
> + for (i = 56; i < 64; i++)
> + KUNIT_EXPECT_EQ(test, p[i], SLUB_RED_ACTIVE);
> +
> + /* Test grow with allocating a bigger 128B object */
> + p = krealloc(p, 112, GFP_KERNEL | __GFP_ZERO);
> + if (unlikely(is_kfence_address(p)))
> + goto exit;
> +
> + for (i = 56; i < 112; i++)
> + KUNIT_EXPECT_EQ(test, p[i], 0);
> + for (i = 112; i < 128; i++)
> + KUNIT_EXPECT_EQ(test, p[i], SLUB_RED_ACTIVE);
> +
> +exit:
> + kfree(p);
> + kasan_enable_current();
> +}
> +
> static int test_init(struct kunit *test)
> {
> slab_errors = 0;
> @@ -196,6 +241,7 @@ static int test_init(struct kunit *test)
> }
>
> static struct kunit_case test_cases[] = {
> + KUNIT_CASE(test_krealloc_redzone_zeroing),
> KUNIT_CASE(test_clobber_zone),
>
> #ifndef CONFIG_KASAN
> --
> 2.34.1
>