linux-kernel - Re: [PATCH slab hotfixes 1/2] mm, slab: suppress warnings in test_leak

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAB=+i9QOrgKcp24i_O9nFkxnySWdO0Yv6NjjDwXopDWbT3RJQw@mail.gmail.com>
Date: Tue, 1 Oct 2024 15:05:37 +0900
From: Hyeonggon Yoo <42.hyeyoo@...il.com>
To: Vlastimil Babka <vbabka@...e.cz>
Cc: Andrew Morton <akpm@...ux-foundation.org>, Christoph Lameter <cl@...ux.com>, 
	David Rientjes <rientjes@...gle.com>, Roman Gushchin <roman.gushchin@...ux.dev>, 
	linux-kernel@...r.kernel.org, linux-mm@...ck.org, 
	kernel test robot <oliver.sang@...el.com>, Guenter Roeck <linux@...ck-us.net>
Subject: Re: [PATCH slab hotfixes 1/2] mm, slab: suppress warnings in
 test_leak_destroy kunit test

On Mon, Sep 30, 2024 at 5:37 PM Vlastimil Babka <vbabka@...e.cz> wrote:
>
> The test_leak_destroy kunit test intends to test the detection of stray
> objects in kmem_cache_destroy(), which normally produces a warning. The
> other slab kunit tests suppress the warnings in the kunit test context,
> so suppress warnings and related printk output in this test as well.
> Automated test running environments then don't need to learn to filter
> the warnings.
>
> Also rename the test's kmem_cache, the name was wrongly copy-pasted from
> test_kfree_rcu.
>
> Fixes: 4e1c44b3db79 ("kunit, slub: add test_kfree_rcu() and test_leak_destroy()")
> Reported-by: kernel test robot <oliver.sang@...el.com>
> Closes: https://lore.kernel.org/oe-lkp/202408251723.42f3d902-oliver.sang@intel.com
> Reported-by: Hyeonggon Yoo <42.hyeyoo@...il.com>
> Closes: https://lore.kernel.org/all/CAB=+i9RHHbfSkmUuLshXGY_ifEZg9vCZi3fqr99+kmmnpDus7Q@mail.gmail.com/
> Reported-by: Guenter Roeck <linux@...ck-us.net>
> Closes: https://lore.kernel.org/all/6fcb1252-7990-4f0d-8027-5e83f0fb9409@roeck-us.net/
> Signed-off-by: Vlastimil Babka <vbabka@...e.cz>
> ---
>  lib/slub_kunit.c | 4 ++--
>  mm/slab.h        | 6 ++++++
>  mm/slab_common.c | 5 +++--
>  mm/slub.c        | 5 +++--
>  4 files changed, 14 insertions(+), 6 deletions(-)

Looks good to me,
Reviewed-by: Hyeonggon Yoo <42.hyeyoo@...il.com>

> diff --git a/lib/slub_kunit.c b/lib/slub_kunit.c
> index 6e3a1e5a7142f797fe20a28967657f50a466d4ee..85d51ec09846d4fa219db6bda336c6f0b89e98e4 100644
> --- a/lib/slub_kunit.c
> +++ b/lib/slub_kunit.c
> @@ -177,13 +177,13 @@ static void test_kfree_rcu(struct kunit *test)
>
>  static void test_leak_destroy(struct kunit *test)
>  {
> -       struct kmem_cache *s = test_kmem_cache_create("TestSlub_kfree_rcu",
> +       struct kmem_cache *s = test_kmem_cache_create("TestSlub_leak_destroy",
>                                                         64, SLAB_NO_MERGE);
>         kmem_cache_alloc(s, GFP_KERNEL);
>
>         kmem_cache_destroy(s);
>
> -       KUNIT_EXPECT_EQ(test, 1, slab_errors);
> +       KUNIT_EXPECT_EQ(test, 2, slab_errors);
>  }
>
>  static int test_init(struct kunit *test)
> diff --git a/mm/slab.h b/mm/slab.h
> index f22fb760b2866124d9d873d28b5a7fa6867aeb90..5cf0fbab0b0554ee70f5371772bc37b290893b8a 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -546,6 +546,12 @@ static inline bool kmem_cache_debug_flags(struct kmem_cache *s, slab_flags_t fla
>         return false;
>  }
>
> +#if IS_ENABLED(CONFIG_SLUB_DEBUG) && IS_ENABLED(CONFIG_KUNIT)
> +bool slab_in_kunit_test(void);
> +#else
> +static inline bool slab_in_kunit_test(void) { return false; }
> +#endif
> +
>  #ifdef CONFIG_SLAB_OBJ_EXT
>
>  /*
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 7443244656150325fb2a7d0158a71821e1418062..3d26c257ed8b57c336ec5c38d4bbd5f5e51d50ff 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -508,8 +508,9 @@ void kmem_cache_destroy(struct kmem_cache *s)
>         kasan_cache_shutdown(s);
>
>         err = __kmem_cache_shutdown(s);
> -       WARN(err, "%s %s: Slab cache still has objects when called from %pS",
> -            __func__, s->name, (void *)_RET_IP_);
> +       if (!slab_in_kunit_test())
> +               WARN(err, "%s %s: Slab cache still has objects when called from %pS",
> +                    __func__, s->name, (void *)_RET_IP_);
>
>         list_del(&s->list);
>
> diff --git a/mm/slub.c b/mm/slub.c
> index 21f71cb6cc06d951a657290421f41170bb3c76cf..5b832512044e3ead8ccde2c02308bd8954246db5 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -827,7 +827,7 @@ static bool slab_add_kunit_errors(void)
>         return true;
>  }
>
> -static bool slab_in_kunit_test(void)
> +bool slab_in_kunit_test(void)
>  {
>         struct kunit_resource *resource;
>
> @@ -843,7 +843,6 @@ static bool slab_in_kunit_test(void)
>  }
>  #else
>  static inline bool slab_add_kunit_errors(void) { return false; }
> -static inline bool slab_in_kunit_test(void) { return false; }
>  #endif
>
>  static inline unsigned int size_from_object(struct kmem_cache *s)
> @@ -5436,6 +5435,8 @@ static void list_slab_objects(struct kmem_cache *s, struct slab *slab,
>         for_each_object(p, s, addr, slab->objects) {
>
>                 if (!test_bit(__obj_to_index(s, addr, p), object_map)) {
> +                       if (slab_add_kunit_errors())
> +                               continue;
>                         pr_err("Object 0x%p @offset=%tu\n", p, p - addr);
>                         print_tracking(s, p);
>                 }
>
> --
> 2.46.1
>