hulk inclusion category: bugfix bugzilla: https://gitee.com/openeuler/release-management/issues/ID5CMS -------------------------------- There is a test which is registering and unregistering xcall by /proc/xcall/comm in multithread. Then kmemleak reported a leak unreferenced object 0xffff00104bd86d00 (size 64): comm "bash", pid 4246, jiffies 4295025941 (age 1701.080s) hex dump (first 32 bytes): 60 fd ff 4b 10 00 ff ff 80 6f d8 4b 10 00 ff ff `..K.....o.K.... a0 df 28 04 90 00 ff ff 18 41 aa 04 88 00 ff ff ..(......A...... backtrace: kmemleak_alloc+0xb4/0xd0 __kmem_cache_alloc_node+0x330/0x420 kmalloc_trace+0x48/0xf8 proc_xcall_command+0x44/0x378 xcall_comm_write+0xc8/0x160 proc_reg_write+0x110/0x180 vfs_write+0x150/0x3a8 ksys_write+0xd0/0x180 __arm64_sys_write+0x4c/0x68 invoke_syscall+0x68/0x1a0 el0_svc_common.constprop.0+0x11c/0x150 do_el0_svc+0x68/0xb0 el0_slow_syscall+0x44/0x1e8 .slow_syscall+0x18c/0x190 In proccesing below, we cannot promise xcall_comm_list and xcall_comm is consistent. This is why the memleak happened. And it has another bug which is xcall has been unregistered, but we can still find it in /proc/ xcall/comm. ------------------------------------------------------- A thread B thread ------------------------------------------------------- xcall_attach insert_xcall_locked xcall_detach delete_xcall_comm_locked insert_xcall_comm_locked ------------------------------------------------------- To solve this problem, merge struct xcall_comm into struct xcall. Make sure this two struct is protected by the same lock. So they will be consistent. Fixes: 7d904de7feb2 ("xcall2.0: Add userspace proc interface") Signed-off-by: Xinyu Zheng <zhengxinyu6@huawei.com> --- arch/arm64/include/asm/xcall.h | 3 +- arch/arm64/kernel/xcall/core.c | 44 +++++++++++++++++------ arch/arm64/kernel/xcall/proc.c | 64 ++-------------------------------- 3 files changed, 39 insertions(+), 72 deletions(-) diff --git a/arch/arm64/include/asm/xcall.h b/arch/arm64/include/asm/xcall.h index c9143b7d2096..2e043d422608 100644 --- a/arch/arm64/include/asm/xcall.h +++ b/arch/arm64/include/asm/xcall.h @@ -36,7 +36,7 @@ struct xcall { /* file attached xcall */ struct inode *binary; struct xcall_prog *program; - char *name; + struct xcall_comm *info; }; struct xcall_area { @@ -52,6 +52,7 @@ struct xcall_area { extern const syscall_fn_t *default_sys_call_table(void); #ifdef CONFIG_DYNAMIC_XCALL +extern void xcall_info_show(struct seq_file *m); extern int xcall_attach(struct xcall_comm *info); extern int xcall_detach(struct xcall_comm *info); extern int xcall_pre_sstep_check(struct pt_regs *regs); diff --git a/arch/arm64/kernel/xcall/core.c b/arch/arm64/kernel/xcall/core.c index a88c4ed6e575..cb97ce0257c6 100644 --- a/arch/arm64/kernel/xcall/core.c +++ b/arch/arm64/kernel/xcall/core.c @@ -111,12 +111,25 @@ static struct xcall *get_xcall(struct xcall *xcall) return xcall; } +static void free_xcall_comm(struct xcall_comm *info) +{ + if (!info) + return; + + kfree(info->name); + kfree(info->binary); + kfree(info->module); + path_put(&info->binary_path); + kfree(info); +} + static void put_xcall(struct xcall *xcall) { if (!refcount_dec_and_test(&xcall->ref)) return; - kfree(xcall->name); + free_xcall_comm(xcall->info); + if (xcall->program) module_put(xcall->program->owner); @@ -128,7 +141,7 @@ static struct xcall *find_xcall(const char *name, struct inode *binary) struct xcall *xcall; list_for_each_entry(xcall, &xcalls_list, list) { - if ((name && !strcmp(name, xcall->name)) || + if ((name && !strcmp(name, xcall->info->name)) || (binary && xcall->binary == binary)) return get_xcall(xcall); } @@ -140,7 +153,7 @@ static struct xcall *insert_xcall_locked(struct xcall *xcall) struct xcall *ret = NULL; spin_lock(&xcall_list_lock); - ret = find_xcall(xcall->name, xcall->binary); + ret = find_xcall(xcall->info->name, xcall->binary); if (!ret) list_add(&xcall->list, &xcalls_list); else @@ -166,6 +179,7 @@ static int init_xcall(struct xcall *xcall, struct xcall_comm *comm) if (!program || !try_module_get(program->owner)) return -EINVAL; + xcall->info = comm; xcall->binary = d_real_inode(comm->binary_path.dentry); xcall->program = program; refcount_set(&xcall->ref, 1); @@ -284,27 +298,37 @@ void clear_xcall_area(struct mm_struct *mm) mm->xcall = NULL; } +void xcall_info_show(struct seq_file *m) +{ + struct xcall *xcall; + + spin_lock(&xcall_list_lock); + list_for_each_entry(xcall, &xcalls_list, list) { + seq_printf(m, "+:%s %s %s\n", + xcall->info->name, xcall->info->binary, + xcall->info->module); + } + spin_unlock(&xcall_list_lock); +} + int xcall_attach(struct xcall_comm *comm) { struct xcall *xcall; int ret; xcall = kzalloc(sizeof(struct xcall), GFP_KERNEL); - if (!xcall) + if (!xcall) { + free_xcall_comm(comm); return -ENOMEM; + } ret = init_xcall(xcall, comm); if (ret) { + free_xcall_comm(comm); kfree(xcall); return ret; } - xcall->name = kstrdup(comm->name, GFP_KERNEL); - if (!xcall->name) { - delete_xcall(xcall); - return -ENOMEM; - } - if (insert_xcall_locked(xcall)) { delete_xcall(xcall); return -EINVAL; diff --git a/arch/arm64/kernel/xcall/proc.c b/arch/arm64/kernel/xcall/proc.c index 54ca0d53908f..074652e27ce9 100644 --- a/arch/arm64/kernel/xcall/proc.c +++ b/arch/arm64/kernel/xcall/proc.c @@ -13,54 +13,8 @@ #include <asm/xcall.h> -static LIST_HEAD(comm_list); -static DECLARE_RWSEM(comm_rwsem); - struct proc_dir_entry *root_xcall_dir; -static void free_xcall_comm(struct xcall_comm *info) -{ - if (!info) - return; - kfree(info->name); - kfree(info->binary); - kfree(info->module); - path_put(&info->binary_path); - kfree(info); -} - -static struct xcall_comm *find_xcall_comm(struct xcall_comm *comm) -{ - struct xcall_comm *temp; - - list_for_each_entry(temp, &comm_list, list) { - if (!strcmp(comm->name, temp->name)) - return temp; - } - - return NULL; -} - -static void delete_xcall_comm_locked(struct xcall_comm *info) -{ - struct xcall_comm *ret; - - down_write(&comm_rwsem); - ret = find_xcall_comm(info); - if (ret) - list_del(&ret->list); - up_write(&comm_rwsem); - free_xcall_comm(ret); -} - -static void insert_xcall_comm_locked(struct xcall_comm *info) -{ - down_write(&comm_rwsem); - if (!find_xcall_comm(info)) - list_add(&info->list, &comm_list); - up_write(&comm_rwsem); -} - static int is_absolute_path(const char *path) { return path[0] == '/'; @@ -147,16 +101,11 @@ int proc_xcall_command(int argc, char **argv) switch (op) { case '+': ret = xcall_attach(info); - if (!ret) - insert_xcall_comm_locked(info); - else - free_xcall_comm(info); break; case '-': ret = xcall_detach(info); - if (!ret) - delete_xcall_comm_locked(info); - free_xcall_comm(info); + kfree(info->name); + kfree(info); break; default: kfree(info); @@ -168,15 +117,8 @@ int proc_xcall_command(int argc, char **argv) static int xcall_comm_show(struct seq_file *m, void *v) { - struct xcall_comm *info; + xcall_info_show(m); - down_read(&comm_rwsem); - list_for_each_entry(info, &comm_list, list) { - seq_printf(m, "+:%s %s %s\n", - info->name, info->binary, - info->module); - } - up_read(&comm_rwsem); return 0; } -- 2.34.1