Re: [PATCH v3 2/2] mm/slub, kunit: Add a test case for kmalloc redzone check

[Feng Tang]

On Thu, Dec 01, 2022 at 12:05:41AM +0100, Vlastimil Babka wrote:
[...]
> > diff --git a/lib/slub_kunit.c b/lib/slub_kunit.c
> > index 5b0c8e7eb6dc..ff24879e3afe 100644
> > --- a/lib/slub_kunit.c
> > +++ b/lib/slub_kunit.c
> > @@ -135,6 +135,27 @@ static void test_clobber_redzone_free(struct kunit *test)
> >  	kmem_cache_destroy(s);
> >  }
> >  
> > +static void test_kmalloc_redzone_access(struct kunit *test)
> > +{
> > +	struct kmem_cache *s = test_kmem_cache_create("TestSlub_RZ_kmalloc", 32,
> > +				SLAB_KMALLOC|SLAB_STORE_USER|SLAB_RED_ZONE);
> > +	u8 *p = kmalloc_trace(s, GFP_KERNEL, 18);
> > +
> > +	kasan_disable_current();
> > +
> > +	/* Suppress the -Warray-bounds warning */
> > +	OPTIMIZER_HIDE_VAR(p);
> > +	p[18] = 0xab;
> > +	p[19] = 0xab;
> > +
> > +	kmem_cache_free(s, p);
> > +	validate_slab_cache(s);
> > +	KUNIT_EXPECT_EQ(test, 2, slab_errors);
> 
> With this ordering the expectation was failing as slab_Errors was 0, had to
> fix it up to look more like TestSlub_RZ_alloc:

Thanks for the catch and fix!

I checked why it worked in my test, and it should be related with
kasan. My test environment has both kasan and kfence enabled, and
kasan could delay the object freeing, and with the original code,
when validate_slab_cache() is called, the object is not freed yet
and gets redzone-checked. 

> > +	kasan_enable_current();
> > +	kmem_cache_destroy(s);
> > +}
> > +
> 
> --- a/lib/slub_kunit.c
> +++ b/lib/slub_kunit.c
> @@ -148,11 +148,11 @@ static void test_kmalloc_redzone_access(struct kunit *test)
>         p[18] = 0xab;
>         p[19] = 0xab;
>  
> -       kmem_cache_free(s, p);
>         validate_slab_cache(s);
>         KUNIT_EXPECT_EQ(test, 2, slab_errors);
>  
>         kasan_enable_current();
> +       kmem_cache_free(s, p);
>         kmem_cache_destroy(s);
>  }
> 
> With that, added both to slab.git branch slab/for-6.2/kmalloc_redzone
> Thanks!

Thanks!

- Feng