Re: [PATCH 4/8] slub_kunit: Add a test for slab_freelist_random=

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] slub_kunit: missing kasan_reset_tag() in test_freelist_random()
--

commit a6a4a9e9d2827c11abe21a80276d4c98393a0acc
Author: Chris Down <chris@chrisdown.name>

slub_kunit: Add a test for slab_freelist_random=

This commit exposes slab_freelist_random to KUnit and adds a test case in
slub_kunit to verify whether slab objects are allocated in linear address
order or randomized order based on slab_freelist_random.

> diff --git a/lib/tests/slub_kunit.c b/lib/tests/slub_kunit.c
> index 5ace733685231..49b9942bc564e 100644
> --- a/lib/tests/slub_kunit.c
> +++ b/lib/tests/slub_kunit.c
> @@ -190,6 +190,56 @@ static void test_clobber_redzone_free(struct kunit *test)
>  	kmem_cache_destroy(s);
>  }
>  
> +static void test_freelist_random(struct kunit *test)
> +{
> +	struct kmem_cache *s = test_kmem_cache_create("TestSlub_freelist_random",
> +					64, SLAB_CONSISTENCY_CHECKS);
> +	bool random = false;
> +	unsigned int i, nr, nr_linear = 0;
> +	void **objs, *first;
> +
> +#ifdef CONFIG_SLAB_FREELIST_RANDOM
> +	random = slab_freelist_random;
> +	KUNIT_EXPECT_EQ(test, !!s->random_seq, random);
> +#endif
> +	kunit_info(test, "freelist randomisation is %s\n", str_on_off(random));
> +
> +	/*
> +	 * Caches with debug flags have no sheaves, so objects come straight
> +	 * off the slab freelist.
> +	 */
> +	first = kmem_cache_alloc(s, GFP_KERNEL);
> +	KUNIT_ASSERT_NOT_NULL(test, first);
> +	nr = virt_to_slab(first)->objects;
> +	objs = kunit_kcalloc(test, nr, sizeof(*objs), GFP_KERNEL);
> +	KUNIT_ASSERT_NOT_NULL(test, objs);
> +	objs[0] = first;
> +
> +	for (i = 1; i < nr; i++) {
> +		objs[i] = kmem_cache_alloc(s, GFP_KERNEL);
> +		KUNIT_ASSERT_NOT_NULL(test, objs[i]);
> +	}
> +
> +	for (i = 1; i < nr; i++) {
> +		if (virt_to_slab(objs[i]) != virt_to_slab(objs[0]))
> +			break;
> +		if (objs[i] == objs[i - 1] + s->size)
> +			nr_linear++;
> +	}
> +
> +	if (i == nr) {
> +		/* Without randomisation, objects are handed out in address order. */
> +		KUNIT_EXPECT_EQ(test, nr_linear == nr - 1, !random);
> +	} else {

[Severity: Medium]
Can this pointer comparison fail under tag-based KASAN modes
(CONFIG_KASAN_SW_TAGS or CONFIG_KASAN_HW_TAGS) when freelist randomization is
disabled?

Because test_freelist_random is registered in test_cases[] outside the
#ifndef CONFIG_KASAN block, it runs under tag-based KASAN configurations. The
cache s is created without a constructor or SLAB_TYPESAFE_BY_RCU, so each
kmem_cache_alloc() call passes through:

  slab_post_alloc_hook()
    -> kasan_slab_alloc()
      -> __kasan_slab_alloc()
        -> assign_tag()
        -> set_tag()

which embeds a random tag in the top byte of each returned pointer in objs[].

While virt_to_slab() strips the tag via virt_to_page() so the loop reaches
i == nr, the direct comparison:

  if (objs[i] == objs[i - 1] + s->size)

compares the tagged pointers without calling kasan_reset_tag(). When
consecutive allocations receive different tags, nr_linear will not reach
nr - 1 even though the untagged object addresses are contiguous, causing
KUNIT_EXPECT_EQ(test, nr_linear == nr - 1, !random) to fail when !random.

Should objs[i] and objs[i - 1] be untagged with kasan_reset_tag() before
comparing their addresses?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793086056.git.chris@chrisdown.name?part=4