[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20221112112055.1111078-4-liushixin2@huawei.com>
Date: Sat, 12 Nov 2022 19:20:55 +0800
From: Liu Shixin <liushixin2@...wei.com>
To: Christoph Lameter <cl@...ux.com>,
Pekka Enberg <penberg@...nel.org>,
"David Rientjes" <rientjes@...gle.com>,
Joonsoo Kim <iamjoonsoo.kim@....com>,
"Andrew Morton" <akpm@...ux-foundation.org>,
Vlastimil Babka <vbabka@...e.cz>,
"Roman Gushchin" <roman.gushchin@...ux.dev>,
Hyeonggon Yoo <42.hyeyoo@...il.com>
CC: <linux-mm@...ck.org>, <linux-kernel@...r.kernel.org>,
Liu Shixin <liushixin2@...wei.com>
Subject: [PATCH v3 3/3] mm/slub: Fix memory leak of kobj->name in sysfs_slab_add()
There is a memory leak of kobj->name in sysfs_slab_add():
unreferenced object 0xffff88817e446440 (size 32):
comm "insmod", pid 4085, jiffies 4296564501 (age 126.272s)
hex dump (first 32 bytes):
75 62 69 66 73 5f 69 6e 6f 64 65 5f 73 6c 61 62 ubifs_inode_slab
00 65 44 7e 81 88 ff ff 00 00 00 00 00 00 00 00 .eD~............
backtrace:
[<000000005b30fbbd>] __kmalloc_node_track_caller+0x4e/0x150
[<000000002f70da0c>] kstrdup_const+0x4b/0x80
[<00000000c6712c61>] kobject_set_name_vargs+0x2f/0xb0
[<00000000b151218e>] kobject_init_and_add+0xb0/0x120
[<00000000e56a4cf5>] sysfs_slab_add+0x17d/0x220
[<000000009326fd57>] __kmem_cache_create+0x406/0x590
[<00000000dde33cff>] kmem_cache_create_usercopy+0x1fc/0x300
[<00000000fe90cedb>] kmem_cache_create+0x12/0x20
[<000000007a6531c8>] 0xffffffffa02d802d
[<000000000e3b13c7>] do_one_initcall+0x87/0x2a0
[<00000000995ecdcf>] do_init_module+0xdf/0x320
[<000000008821941f>] load_module+0x2f98/0x3330
[<00000000ef51efa4>] __do_sys_finit_module+0x113/0x1b0
[<000000009339fbce>] do_syscall_64+0x35/0x80
[<000000006b7f2033>] entry_SYSCALL_64_after_hwframe+0x46/0xb0
Following the rules stated in the comment for kobject_init_and_add():
If this function returns an error, kobject_put() must be called to
properly clean up the memory associated with the object.
kobject_put() is more appropriate for error handling after kobject_init().
And we can use this function to solve this problem.
For the cache created early, the related sysfs_slab_add() is called in
slab_sysfs_init(). Skip free these kmem_cache since they are important
for system. Keep them working without sysfs.
Fixes: 80da026a8e5d ("mm/slub: fix slab double-free in case of duplicate sysfs filename")
Signed-off-by: Liu Shixin <liushixin2@...wei.com>
---
include/linux/slub_def.h | 4 ++--
mm/slab_common.c | 6 ++----
mm/slub.c | 21 +++++++++++++++++----
3 files changed, 21 insertions(+), 10 deletions(-)
diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h
index 26d56c4c74d1..90c3e06b77b1 100644
--- a/include/linux/slub_def.h
+++ b/include/linux/slub_def.h
@@ -144,11 +144,11 @@ struct kmem_cache {
#ifdef CONFIG_SYSFS
#define SLAB_SUPPORTS_SYSFS
-int sysfs_slab_add(struct kmem_cache *);
+int sysfs_slab_add(struct kmem_cache *, bool);
void sysfs_slab_unlink(struct kmem_cache *);
void sysfs_slab_release(struct kmem_cache *);
#else
-static inline int sysfs_slab_add(struct kmem_cache *s)
+static inline int sysfs_slab_add(struct kmem_cache *s, bool free_slab)
{
return 0;
}
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 55e2cf064dfe..30808a1d1b32 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -237,11 +237,9 @@ static struct kmem_cache *create_cache(const char *name,
#ifdef SLAB_SUPPORTS_SYSFS
/* Mutex is not taken during early boot */
if (slab_state >= FULL) {
- err = sysfs_slab_add(s);
- if (err) {
- slab_kmem_cache_release(s);
+ err = sysfs_slab_add(s, true);
+ if (err)
return ERR_PTR(err);
- }
debugfs_slab_add(s);
}
#endif
diff --git a/mm/slub.c b/mm/slub.c
index a1ad759753ce..06a3223fc833 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -5881,7 +5881,7 @@ static char *create_unique_id(struct kmem_cache *s)
return name;
}
-int sysfs_slab_add(struct kmem_cache *s)
+int sysfs_slab_add(struct kmem_cache *s, bool free_slab)
{
int err;
const char *name;
@@ -5911,14 +5911,17 @@ int sysfs_slab_add(struct kmem_cache *s)
* for the symlinks.
*/
name = create_unique_id(s);
- if (IS_ERR(name))
+ if (IS_ERR(name)) {
+ if (free_slab)
+ slab_kmem_cache_release(s);
return PTR_ERR(name);
+ }
}
s->kobj.kset = kset;
err = kobject_init_and_add(&s->kobj, &slab_ktype, NULL, "%s", name);
if (err)
- goto out;
+ goto out_put_kobj;
err = sysfs_create_group(&s->kobj, &slab_attr_group);
if (err)
@@ -5934,6 +5937,16 @@ int sysfs_slab_add(struct kmem_cache *s)
return err;
out_del_kobj:
kobject_del(&s->kobj);
+out_put_kobj:
+ /*
+ * Skip free kmem_cache that create early since they are important
+ * for system. Keep them working without sysfs. Only free name
+ * for this case.
+ */
+ if (free_slab)
+ kobject_put(&s->kobj);
+ else
+ kfree_const(&s->kobj.name);
goto out;
}
@@ -6002,7 +6015,7 @@ static int __init slab_sysfs_init(void)
slab_state = FULL;
list_for_each_entry(s, &slab_caches, list) {
- err = sysfs_slab_add(s);
+ err = sysfs_slab_add(s, false);
if (err)
pr_err("SLUB: Unable to add boot slab %s to sysfs\n",
s->name);
--
2.25.1
Powered by blists - more mailing lists