On Tue, Oct 1, 2024 at 10:03 PM Nilay Shroff <nilay@xxxxxxxxxxxxx> wrote:
>
> The fix implemented in commit 4ec10268ed98 ("mm, slab: unlink slabinfo,
> sysfs and debugfs immediately") caused a subtle side effect due to which
> while destroying the kmem cache, the code path would never get into
> sysfs_slab_release() function even though SLAB_SUPPORTS_SYSFS is defined
> and slab state is FULL. Due to this side effect, we would never release
> kobject defined for kmem cache and leak the associated memory.
>
> The issue here's with the use of __is_defined() macro in kmem_cache_
> release(). The __is_defined() macro expands to __take_second_arg(
> arg1_or_junk 1, 0). If "arg1_or_junk" is defined to 1 then it expands to
> __take_second_arg(0, 1, 0) and returns 1. If "arg1_or_junk" is NOT defined
> to any value then it expands to __take_second_arg(... 1, 0) and returns 0.
>
> In this particular issue, SLAB_SUPPORTS_SYSFS is defined without any
> associated value and that causes __is_defined(SLAB_SUPPORTS_SYSFS) to
> always evaluate to 0 and hence it would never invoke sysfs_slab_release().
>
> This patch helps fix this issue by defining SLAB_SUPPORTS_SYSFS to 1.
>
> Fixes: 4ec10268ed98 ("mm, slab: unlink slabinfo, sysfs and debugfs immediately")
> Reported-by: Yi Zhang <yi.zhang@xxxxxxxxxx>
> Closes: https://lore.kernel.org/all/CAHj4cs9YCCcfmdxN43-9H3HnTYQsRtTYw1Kzq-L468GfLKAENA@xxxxxxxxxxxxxx/
> Signed-off-by: Nilay Shroff <nilay@xxxxxxxxxxxxx>
Tested-by: Yi Zhang <yi.zhang@xxxxxxxxxx>
> ---
> mm/slab.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/slab.h b/mm/slab.h
> index f22fb760b286..3e0a08ea4c42 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -310,7 +310,7 @@ struct kmem_cache {
> };
>
> #if defined(CONFIG_SYSFS) && !defined(CONFIG_SLUB_TINY)
> -#define SLAB_SUPPORTS_SYSFS
> +#define SLAB_SUPPORTS_SYSFS 1
> void sysfs_slab_unlink(struct kmem_cache *s);
> void sysfs_slab_release(struct kmem_cache *s);
> #else
> --
> 2.45.2
>
--
Best Regards,
Yi Zhang
