All of lore.kernel.org
 help / color / mirror / Atom feed
From: Liu Shixin <liushixin2@huawei.com>
To: Christoph Lameter <cl@linux.com>,
	Pekka Enberg <penberg@kernel.org>,
	"David Rientjes" <rientjes@google.com>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	Vlastimil Babka <vbabka@suse.cz>,
	"Roman Gushchin" <roman.gushchin@linux.dev>,
	Hyeonggon Yoo <42.hyeyoo@gmail.com>
Cc: <linux-mm@kvack.org>, <linux-kernel@vger.kernel.org>,
	Liu Shixin <liushixin2@huawei.com>
Subject: [PATCH v4 3/3] mm/slub: Fix memory leak of kobj->name in sysfs_slab_add()
Date: Sat, 12 Nov 2022 19:46:02 +0800	[thread overview]
Message-ID: <20221112114602.1268989-4-liushixin2@huawei.com> (raw)
In-Reply-To: <20221112114602.1268989-1-liushixin2@huawei.com>

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@huawei.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..25575bce0c3c 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 the name
+	 * for early allocated kmem_cache.
+	 */
+	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


  parent reply	other threads:[~2022-11-12 10:58 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-12 11:45 [PATCH v4 0/3] Refactor __kmem_cache_create() and fix memory leak Liu Shixin
2022-11-12 11:46 ` [PATCH v4 1/3] mm/slab_common: Move cache_name to create_cache() Liu Shixin
2022-11-12 11:46 ` [PATCH v4 2/3] mm/slub: Refactor __kmem_cache_create() Liu Shixin
2022-11-12 11:46 ` Liu Shixin [this message]
2022-11-16 12:59   ` [PATCH v4 3/3] mm/slub: Fix memory leak of kobj->name in sysfs_slab_add() Hyeonggon Yoo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221112114602.1268989-4-liushixin2@huawei.com \
    --to=liushixin2@huawei.com \
    --cc=42.hyeyoo@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=penberg@kernel.org \
    --cc=rientjes@google.com \
    --cc=roman.gushchin@linux.dev \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.