linux-kernel - Re: [PATCH] mm/slab: Fix a kmemleak in kmem_cache

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]

Date: Wed, 28 Feb 2024 16:15:00 +0800
From: xiaolei wang <xiaolei.wang@...driver.com>
To: Vlastimil Babka <vbabka@...e.cz>, cl@...ux.com, penberg@...nel.org,
        rientjes@...gle.com, iamjoonsoo.kim@....com, akpm@...ux-foundation.org,
        roman.gushchin@...ux.dev, 42.hyeyoo@...il.com, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm/slab: Fix a kmemleak in kmem_cache_destroy()


On 2/28/24 15:34, Vlastimil Babka wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> On 2/28/24 04:04, Xiaolei Wang wrote:
>> For earlier kmem cache creation, slab_sysfs_init() has not been called.
>> Consequently, kmem_cache_destroy() cannot utilize kobj_type::release to
>> release the kmem_cache structure. Therefore, tweak kmem_cache_release()
>> to use slab_kmem_cache_release() for releasing kmem_cache when slab_state
>> isn't FULL. This will fixes the memory leaks like following:
> So do I understand correctly the leak happens when a cache is destroyed
> while slab_state < FULL? Do we ever destroy a cache that early?

Yes, like some test programs, test_meminit_init() or 
arm_v7s_alloc_pgtable(),

destroy will cause leaks after use, and when slab_state < FULL,

there are many places to create cache, which itself also has the risk of 
leaks

thanks

xiaolei

>
>> unreferenced object 0xffff0000c2d87080 (size 128):
>>     comm "swapper/0", pid 1, jiffies 4294893428
>>     hex dump (first 32 bytes):
>>       00 00 00 00 ad 4e ad de ff ff ff ff 6b 6b 6b 6b .....N......kkkk
>>       ff ff ff ff ff ff ff ff b8 ab 48 89 00 80 ff ff.....H.....
>>     backtrace (crc 8819d0f6):
>>       [<ffff80008317a298>] kmemleak_alloc+0xb0/0xc4
>>       [<ffff8000807e553c>] kmem_cache_alloc_node+0x288/0x3a8
>>       [<ffff8000807e95f0>] __kmem_cache_create+0x1e4/0x64c
>>       [<ffff8000807216bc>] kmem_cache_create_usercopy+0x1c4/0x2cc
>>       [<ffff8000807217e0>] kmem_cache_create+0x1c/0x28
>>       [<ffff8000819f6278>] arm_v7s_alloc_pgtable+0x1c0/0x6d4
>>       [<ffff8000819f53a0>] alloc_io_pgtable_ops+0xe8/0x2d0
>>       [<ffff800084b2d2c4>] arm_v7s_do_selftests+0xe0/0x73c
>>       [<ffff800080016b68>] do_one_initcall+0x11c/0x7ac
>>       [<ffff800084a71ddc>] kernel_init_freeable+0x53c/0xbb8
>>       [<ffff8000831728d8>] kernel_init+0x24/0x144
>>       [<ffff800080018e98>] ret_from_fork+0x10/0x20
>>
>> Signed-off-by: Xiaolei Wang <xiaolei.wang@...driver.com>
>> ---
>>   mm/slab_common.c | 8 ++++++--
>>   mm/slub.c        | 6 ++----
>>   2 files changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/mm/slab_common.c b/mm/slab_common.c
>> index 238293b1dbe1..b6b35bcdd196 100644
>> --- a/mm/slab_common.c
>> +++ b/mm/slab_common.c
>> @@ -404,8 +404,12 @@ EXPORT_SYMBOL(kmem_cache_create);
>>    */
>>   static void kmem_cache_release(struct kmem_cache *s)
>>   {
>> -     sysfs_slab_unlink(s);
>> -     sysfs_slab_release(s);
>> +     if (slab_state >= FULL) {
>> +             sysfs_slab_unlink(s);
>> +             sysfs_slab_release(s);
>> +     } else {
>> +             slab_kmem_cache_release(s);
>> +     }
>>   }
>>   #else
>>   static void kmem_cache_release(struct kmem_cache *s)
>> diff --git a/mm/slub.c b/mm/slub.c
>> index 2ef88bbf56a3..9ba59d064b6d 100644
>> --- a/mm/slub.c
>> +++ b/mm/slub.c
>> @@ -6792,14 +6792,12 @@ static int sysfs_slab_add(struct kmem_cache *s)
>>
>>   void sysfs_slab_unlink(struct kmem_cache *s)
>>   {
>> -     if (slab_state >= FULL)
>> -             kobject_del(&s->kobj);
>> +     kobject_del(&s->kobj);
>>   }
>>
>>   void sysfs_slab_release(struct kmem_cache *s)
>>   {
>> -     if (slab_state >= FULL)
>> -             kobject_put(&s->kobj);
>> +     kobject_put(&s->kobj);
>>   }
>>
>>   /*