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 | 4 ++- arch/arm64/kernel/xcall/core.c | 25 +++++++++++++--- arch/arm64/kernel/xcall/proc.c | 53 ++-------------------------------- 3 files changed, 27 insertions(+), 55 deletions(-) diff --git a/arch/arm64/include/asm/xcall.h b/arch/arm64/include/asm/xcall.h index c9143b7d2096..0d0c4c794010 100644 --- a/arch/arm64/include/asm/xcall.h +++ b/arch/arm64/include/asm/xcall.h @@ -26,7 +26,6 @@ struct xcall_comm { char *binary; struct path binary_path; char *module; - struct list_head list; }; struct xcall { @@ -37,6 +36,7 @@ struct xcall { struct inode *binary; struct xcall_prog *program; char *name; + struct xcall_comm *info; }; struct xcall_area { @@ -52,6 +52,8 @@ struct xcall_area { extern const syscall_fn_t *default_sys_call_table(void); #ifdef CONFIG_DYNAMIC_XCALL +extern void free_xcall_comm(struct xcall_comm *info); +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..18b73c044a8d 100644 --- a/arch/arm64/kernel/xcall/core.c +++ b/arch/arm64/kernel/xcall/core.c @@ -117,6 +117,8 @@ static void put_xcall(struct xcall *xcall) return; kfree(xcall->name); + free_xcall_comm(xcall->info); + if (xcall->program) module_put(xcall->program->owner); @@ -135,16 +137,18 @@ static struct xcall *find_xcall(const char *name, struct inode *binary) return NULL; } -static struct xcall *insert_xcall_locked(struct xcall *xcall) +static struct xcall *insert_xcall_locked(struct xcall *xcall, struct xcall_comm *info) { struct xcall *ret = NULL; spin_lock(&xcall_list_lock); ret = find_xcall(xcall->name, xcall->binary); - if (!ret) + if (!ret) { + xcall->info = info; list_add(&xcall->list, &xcalls_list); - else + } else put_xcall(ret); + spin_unlock(&xcall_list_lock); return ret; } @@ -284,6 +288,19 @@ 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; @@ -305,7 +322,7 @@ int xcall_attach(struct xcall_comm *comm) return -ENOMEM; } - if (insert_xcall_locked(xcall)) { + if (insert_xcall_locked(xcall, comm)) { delete_xcall(xcall); return -EINVAL; } diff --git a/arch/arm64/kernel/xcall/proc.c b/arch/arm64/kernel/xcall/proc.c index beb2c66dfc47..9dea182340ef 100644 --- a/arch/arm64/kernel/xcall/proc.c +++ b/arch/arm64/kernel/xcall/proc.c @@ -13,12 +13,9 @@ #include <asm/xcall.h> -static LIST_HEAD(comm_list); -static DECLARE_RWSEM(comm_rwsem); - static struct proc_dir_entry *root_xcall_dir; -static void free_xcall_comm(struct xcall_comm *info) +void free_xcall_comm(struct xcall_comm *info) { if (!info) return; @@ -29,38 +26,6 @@ static void free_xcall_comm(struct xcall_comm *info) 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] == '/'; @@ -141,21 +106,16 @@ int proc_xcall_command(int argc, char **argv) info = kzalloc(sizeof(*info), GFP_KERNEL); if (!info) return -ENOMEM; - INIT_LIST_HEAD(&info->list); op = parse_xcall_command(argc, argv, info); switch (op) { case '+': ret = xcall_attach(info); - if (!ret) - insert_xcall_comm_locked(info); - else + if (ret) free_xcall_comm(info); break; case '-': ret = xcall_detach(info); - if (!ret) - delete_xcall_comm_locked(info); free_xcall_comm(info); break; default: @@ -168,15 +128,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