Re: [Linux Kernel Bug] memory leak in ubi_attach
From: Zhihao Cheng
Date: Mon Jan 29 2024 - 06:51:42 EST
在 2024/1/24 22:41, Chenyuan Yang 写道:
Hi Zhihao,
Thanks very much for your reply and further exploration of this crash!
I have attached the config to reproduce this memory leak. For the
command line, you can use qemu to boot the kernel (You need to build
the bullseye-image first).
```
mkdir -p logs
qemu-system-x86_64 \
-m 2G \
-smp 2 \
-kernel linux/arch/x86/boot/bzImage \
-append "console=ttyS0 root=/dev/sda earlyprintk=serial net.ifnames=0" \
-drive file=image/bullseye-qemu.img,format=raw \
-net user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 \
-net nic,model=e1000 \
-enable-kvm \
-nographic \
-pidfile vm.pid \
```
In qemu, you can run
```
gcc -pthread repro.c -o exe
./exe
```
to reproduce the memory leak.
The bad news is that I can't reproduce the problem with your kernel
config. The good news is that I seem to find the cause of the problem by
code reviewing:
alloc_ai -> kmem_cache_create -> kmem_cache_create_usercopy:
s = __kmem_cache_alias
s = find_mergeable // cannot find mergeable kmem_cache, s = NULL
create_cache
s = kmem_cache_zalloc
__kmem_cache_create(s)
sysfs_slab_add
kobject_init_and_add(&s->kobj)
kobject_init(kobj) // kobj->kref = 1
kobject_add_varg
kobject_set_name_vargs
kvasprintf_const
kstrdup_const
kstrdup
kmalloc_track_caller // allocated memory
kobj->name = s // assign allocated memory to kobj->name
kobject_add_internal
create_dir // failed caused by ENOMEM
// kobj->name won't be released even 's' is released
You can reproduce it by executing ubiattach -m0 with reproduce.diff applied:
[root@localhost ~]# cat /sys/kernel/debug/kmemleak
unreferenced object 0xffff88813b488080 (size 16):
comm "ubiattach", pid 9535, jiffies 4294943095
hex dump (first 16 bytes):
3a 30 30 30 30 30 35 36 00 00 00 00 00 00 00 00 :0000056........
backtrace (crc 8a117ab9):
[<ffffffff815f91c2>] __kmalloc_node_track_caller+0x322/0x420
[<ffffffff81575efc>] kstrdup+0x3c/0x70
[<ffffffff81575f8f>] kstrdup_const+0x5f/0x70
[<ffffffff82535426>] kvasprintf_const+0xc6/0x110
[<ffffffff84b88cb0>] kobject_set_name_vargs+0x40/0xd0
[<ffffffff84b8911b>] kobject_init_and_add+0x8b/0xf0
[<ffffffff815faba9>] sysfs_slab_add+0x119/0x1e0
[<ffffffff815fc2dc>] __kmem_cache_create+0x41c/0x590
[<ffffffff81586019>] kmem_cache_create_usercopy+0x169/0x300
[<ffffffff815861c1>] kmem_cache_create+0x11/0x20
[<ffffffff82ec8896>] ubi_attach+0xc6/0x1de0
[<ffffffff82eb77a3>] ubi_attach_mtd_dev+0x8a3/0x1120
[<ffffffff82eb8bfc>] ctrl_cdev_ioctl+0x1fc/0x270
[<ffffffff816ca312>] __x64_sys_ioctl+0xf2/0x140
[<ffffffff84bc3970>] do_syscall_64+0x50/0x140
[<ffffffff84c0008b>] entry_SYSCALL_64_after_hwframe+0x63/0x6b
There are two ways to fix it:
1. Release kobj->name directly in the error baranch of
kobject_add_internal() after calling create_dir().
2. Put kobj's refcnt in the error baranch of kobject_add_internal()
after calling create_dir().
I'm not sure which one is better or whether there are other fix methods,
maybe you can send an email in kobj related mailiing lists to confirm
the problem.
diff --git a/drivers/mtd/ubi/attach.c b/drivers/mtd/ubi/attach.c
index ae5abe492b52..3694fd52c530 100644
--- a/drivers/mtd/ubi/attach.c
+++ b/drivers/mtd/ubi/attach.c
@@ -1447,6 +1447,7 @@ static int scan_all(struct ubi_device *ubi, struct ubi_attach_info *ai,
return err;
}
+extern int g_inject;
static struct ubi_attach_info *alloc_ai(void)
{
struct ubi_attach_info *ai;
@@ -1461,9 +1462,11 @@ static struct ubi_attach_info *alloc_ai(void)
INIT_LIST_HEAD(&ai->alien);
INIT_LIST_HEAD(&ai->fastmap);
ai->volumes = RB_ROOT;
+ g_inject = 1;
ai->aeb_slab_cache = kmem_cache_create("ubi_aeb_slab_cache",
sizeof(struct ubi_ainf_peb),
0, 0, NULL);
+ g_inject = 0;
if (!ai->aeb_slab_cache) {
kfree(ai);
ai = NULL;
diff --git a/lib/kobject.c b/lib/kobject.c
index 59dbcbdb1c91..37da8b767557 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -204,7 +204,8 @@ static void kobject_init_internal(struct kobject *kobj)
kobj->state_initialized = 1;
}
-
+struct kobject *g_k;
+int g_inject;
static int kobject_add_internal(struct kobject *kobj)
{
int error = 0;
@@ -235,6 +236,13 @@ static int kobject_add_internal(struct kobject *kobj)
parent ? kobject_name(parent) : "<NULL>",
kobj->kset ? kobject_name(&kobj->kset->kobj) : "<NULL>");
+ if (g_inject) {
+ error = -ENOMEM; // Simulate failure in create_dir->sysfs_create_dir_ns->kernfs_create_dir_ns->kernfs_new_node->__kernfs_new_node-->kmem_cache_zalloc->slab_alloc_node->slab_pre_alloc_hook->should_failslab
+ g_k = kobj;
+ pr_err("inject failure, kobj ref %d\n", refcount_read(&kobj->kref.refcount));
+ smp_wmb();
+ } else
+
error = create_dir(kobj);
if (error) {
kobj_kset_leave(kobj);
@@ -685,6 +693,12 @@ static void kobject_cleanup(struct kobject *kobj)
kobject_name(kobj), kobj);
}
+ smp_rmb();
+ if (g_k == kobj) {
+ pr_err("free name %s\n", name);
+ dump_stack();
+ }
+
/* free name if we allocated it */
if (name) {
pr_debug("'%s': free name\n", name);
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 238293b1dbe1..c76277004bd0 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -158,10 +158,12 @@ int slab_unmergeable(struct kmem_cache *s)
return 0;
}
+extern int g_inject;
struct kmem_cache *find_mergeable(unsigned int size, unsigned int align,
slab_flags_t flags, const char *name, void (*ctor)(void *))
{
struct kmem_cache *s;
+ static int enter = 0;
if (slab_nomerge)
return NULL;
@@ -184,6 +186,11 @@ struct kmem_cache *find_mergeable(unsigned int size, unsigned int align,
if (size > s->size)
continue;
+ if (g_inject && !enter) {
+ enter = 1;
+ break;
+ }
+
if ((flags & SLAB_MERGE_SAME) != (s->flags & SLAB_MERGE_SAME))
continue;
/*