Received-SPF: pass (google.com: domain of linux-kernel+bounces-84657-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <cbdab268-d1bd-437f-878f-8cad1173a55b@windriver.com>
Date: Wed, 28 Feb 2024 16:15:00 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] mm/slab: Fix a kmemleak in kmem_cache_destroy()
Content-Language: en-US
To: Vlastimil Babka <vbabka@suse.cz>, cl@linux.com, penberg@kernel.org,
        rientjes@google.com, iamjoonsoo.kim@lge.com, akpm@linux-foundation.org,
        roman.gushchin@linux.dev, 42.hyeyoo@gmail.com, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
References: <20240228030408.2740647-1-xiaolei.wang@windriver.com>
 <303e28cb-9c9e-470c-bee2-b7cf4b12e810@suse.cz>
From: xiaolei wang <xiaolei.wang@windriver.com>
In-Reply-To: <303e28cb-9c9e-470c-bee2-b7cf4b12e810@suse.cz>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?QlJsY2FkTktvampZTjBNVUxFaG9qbis4RGlUUE4xRVhacFhEZ1I4ZTFYZWF0?=
 =?utf-8?B?YkkySEl6ZlZpTW94UmM3Ym9pUEhnWW41QWx3VHJoeVRWbVpIeUZkYzBXaWZ1?=
 =?utf-8?B?QVZEQ3FKZHdCSG1rNGxaeGFqUXIvKzNBZ2FZV1V0U1h6UnRqT1hZeHRDbkFo?=
 =?utf-8?B?NjQyTEtyZUVvUDdOaW82T2VKVWx2QTlML0dpZ3JTRzZyRUJ1ZDdXc3FsaFFC?=
 =?utf-8?B?MWpSbXJpWDR1cHRKRTBKM0h2emI4d255Q0JPcEpLa3REQm1LTHk2NzB4czJm?=
 =?utf-8?B?d3RmR3lNQk1pN2YyYVFZOFRVdzB0TUYzSzlXSUlyTmlBRTA2ZHVnNjluRjlz?=
 =?utf-8?B?ZEU0Y0NHZmdkYWF1Z213ZFU3amtoT3dhZFUrb3BxVllId2szOC9Zc1VDbWZZ?=
 =?utf-8?B?YjhGalAwZnVVU2ZHa2pQM3d4RGxPK1Boc2owQTF1eXRZbFREMTB6SWFLWnJ6?=
 =?utf-8?B?TzFFcTk3MCtSZlg4Tjl0NUIxVmJvRFhmTTJqcURRZVd3LzhQNDFrMDlXOEQ2?=
 =?utf-8?B?bnp6RUdJNVl4U0trSlRqdWhYRkRHUkdVdnh1Mk5zS2FhWFVaMHhNSDZDNkpE?=
 =?utf-8?B?TmhpM05PNGRsUHJNRWx6TTJoWThXcUdUTFQ1YmZUbEZXQUF2WjhSNzFJV1Rp?=
 =?utf-8?B?K2pCWlVGbldBaUNTM3piSENwUVI0eW9JOEhZa2Q5aDJ3ZXNHR0J1dTBqRkVu?=
 =?utf-8?B?bUw2VzRNNklkclRrVnliSzZNS1RYcGtLWWtvenQ3eWtobFRPWlJCeDROejFy?=
 =?utf-8?B?aXRVQkhkZGpkZ25LZVVDRzRVb0FVL01sMzJxNGZVWlh4RkZXUERrQlZTWEYz?=
 =?utf-8?B?UHNaVTgxbWFyRjVHc0RUaFBYa0tsQXlkdVpQQmtKN0N4ZHdrN1hRM3BTRmpo?=
 =?utf-8?B?dEVxdFh4VmJQMDhVZU0zZ2dSSDBDQll3YWF6NUlpcE1tZzliMGtOWWZ6WHdr?=
 =?utf-8?B?YnZRa01XREFXTUFuWDI0TlpXd0JnMjg3alNqRVdyd0oxVXp0UXVoVE03UHN4?=
 =?utf-8?B?Mi9FQnFlZFRtcUxkNXJOcDA5RGpIK2JoeFQ4L0thMFRJTGNDV25UUzhyV09H?=
 =?utf-8?B?M3hUUXl6SjB3S0wrdDVWbXlETXd0ZStEZElNclcraGxPeGdSZEV4K2Nnakc2?=
 =?utf-8?B?ZGJhZjd0dXdPY3I2ZXZLeDVKOTRjZWVLU3N6VjFKQ2I4OFY5UGRjRXBmb0Ny?=
 =?utf-8?B?aFU2NHhqRFdrQXNYSGxyNWRjTm1udkZPSUV3L2crVkNjcDJxRHZRNlhwaGFy?=
 =?utf-8?B?Rzh0b0JmV0ZXR21lY3MvaFZtMXpuamdQcUtZSlZ4cktDeW5GYldQcnlzN3Nu?=
 =?utf-8?B?VkpnUGtnbnpvNzgrRWIwRTRMY0wwZGNTZ0Voa3NSZE9TdkdoYWxPYXdCdDhP?=
 =?utf-8?B?WUU0NnZjVEZVRkRQMnRXTnZBZUZWSFZCbmVlVFFidEZIUXVyQzd5Uitocng0?=
 =?utf-8?B?WkJhaW9BQS8zb2xMQmovYzRoVTM2M1VNNGVJa3BnL28vV0VwbExuV0drY0Fv?=
 =?utf-8?B?Z2gwQnNZdytpcWNBMFgrSWYrTFRvWUpXN3dIdUtPRFA3L0ZtenVYeGpVY1NP?=
 =?utf-8?B?d2JvRWpWeXhFZUV2aktqbHh5ejVvcDd5aVF1TjYzNWdWcHVUNy9rNFBUOVR5?=
 =?utf-8?B?RXZuVHBTcGJPQTRJVjhJOE51Mm9wWVhWdlRoZkZySHRvN1pyelJ1VWEzaktN?=
 =?utf-8?B?VmdVaTZCWjBpNW15M3Y1SGlMZEhBTWhBZ1JTb2Uzd1p1ekRKNHhCSDZwbXM4?=
 =?utf-8?B?WmFzL2JjNkErSjZ3TVpvSDRHY3NMSVZrUmwwMU9GeSt5M2xQR3JPMUptR3Vo?=
 =?utf-8?B?UVFsTEszMUN0Qll0ZzgveUZ3ekh0WFpaNWExcWc4eXpHS2k5NW1ZS0xOVnBr?=
 =?utf-8?B?UXVGbVhyNGEyYjNuTE5xQ29kbm1OUUM3R1IzTGZyZUoxWmxvWnVLSW9JY25s?=
 =?utf-8?B?ZzlrOTZzN3BjUmlPa2ZjMDNsN3pQdE9FbE9zQVJpQlNhRXF4S1BQZUdYZFQv?=
 =?utf-8?B?Y0hOci9TdVF2YTJzaCtUandVWVBHRWdLa0JJN0sxb1VwTkRGVlQ2aWRkUFBS?=
 =?utf-8?B?SkN5UzBZelhZdUZaZVFMT0ozd1Z5ZmprclJ4b0doTktvNUVYU01Ud0Flc2hW?=
 =?utf-8?B?dDZjZlNMYk9xREpIdURCZ1N1RDJvbzFjY0Z2YlZVeWR2NDVBSUN4K1FXbFl1?=
 =?utf-8?B?Y1puRTlvVVZLeFY2NXA0MjJpSjBLdGhxT3RvelRqTSs2UHNlWXBMdEJGTHBN?=
 =?utf-8?B?aUYrb25wTFZKajFrcHZibmNudFRRPT0=?=
X-OriginatorOrg: windriver.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 50ff0c5a-0fab-422e-7613-08dc383560cf
X-MS-Exchange-CrossTenant-AuthSource: MW5PR11MB5764.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Feb 2024 08:15:08.9188
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 8ddb2873-a1ad-4a18-ae4e-4644631433be
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 8gOEYnG7ktiMzGc8sSP7vvratmMUsFgJuQZtzgs5dnPN9f0q/juh0zyOe9IQvGiRYZwjDo6QjGU5Dvc/pkNXvLzyl4gsDU0isz6Z+8JLIBE=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB5278
X-Proofpoint-GUID: q4j1BkZulSe9bdQmtQ8lli7v1B1pTdQY
X-Proofpoint-ORIG-GUID: q4j1BkZulSe9bdQmtQ8lli7v1B1pTdQY
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-02-28_04,2024-02-27_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 lowpriorityscore=0
 adultscore=0 phishscore=0 mlxscore=0 impostorscore=0 spamscore=0
 bulkscore=0 malwarescore=0 mlxlogscore=999 clxscore=1015
 priorityscore=1501 suspectscore=0 classifier=spam adjust=0 reason=mlx
 scancount=1 engine=8.19.0-2402120000 definitions=main-2402280064


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@windriver.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);
>>   }
>>
>>   /*