| Message ID | 20210317030915.2865-1-qiang.zhang@windriver.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] bpf: Fix memory leak in copy_process() | expand |
On 3/17/21 4:09 AM, qiang.zhang@windriver.com wrote: > From: Zqiang <qiang.zhang@windriver.com> > > The syzbot report a memleak follow: > BUG: memory leak > unreferenced object 0xffff888101b41d00 (size 120): > comm "kworker/u4:0", pid 8, jiffies 4294944270 (age 12.780s) > backtrace: > [<ffffffff8125dc56>] alloc_pid+0x66/0x560 > [<ffffffff81226405>] copy_process+0x1465/0x25e0 > [<ffffffff81227943>] kernel_clone+0xf3/0x670 > [<ffffffff812281a1>] kernel_thread+0x61/0x80 > [<ffffffff81253464>] call_usermodehelper_exec_work > [<ffffffff81253464>] call_usermodehelper_exec_work+0xc4/0x120 > [<ffffffff812591c9>] process_one_work+0x2c9/0x600 > [<ffffffff81259ab9>] worker_thread+0x59/0x5d0 > [<ffffffff812611c8>] kthread+0x178/0x1b0 > [<ffffffff8100227f>] ret_from_fork+0x1f/0x30 > > unreferenced object 0xffff888110ef5c00 (size 232): > comm "kworker/u4:0", pid 8414, jiffies 4294944270 (age 12.780s) > backtrace: > [<ffffffff8154a0cf>] kmem_cache_zalloc > [<ffffffff8154a0cf>] __alloc_file+0x1f/0xf0 > [<ffffffff8154a809>] alloc_empty_file+0x69/0x120 > [<ffffffff8154a8f3>] alloc_file+0x33/0x1b0 > [<ffffffff8154ab22>] alloc_file_pseudo+0xb2/0x140 > [<ffffffff81559218>] create_pipe_files+0x138/0x2e0 > [<ffffffff8126c793>] umd_setup+0x33/0x220 > [<ffffffff81253574>] call_usermodehelper_exec_async+0xb4/0x1b0 > [<ffffffff8100227f>] ret_from_fork+0x1f/0x30 > > after the UMD process exits, the pipe_to_umh/pipe_from_umh and tgid > need to be release. > > Fixes: d71fa5c9763c ("bpf: Add kernel module with user mode driver that populates bpffs.") > Reported-by: syzbot+44908bb56d2bfe56b28e@syzkaller.appspotmail.com > Signed-off-by: Zqiang <qiang.zhang@windriver.com> Applied to bpf, thanks (also did minor style fixups to fix kernel style)!
diff --git a/include/linux/usermode_driver.h b/include/linux/usermode_driver.h index 073a9e0ec07d..ad970416260d 100644 --- a/include/linux/usermode_driver.h +++ b/include/linux/usermode_driver.h @@ -14,5 +14,6 @@ struct umd_info { int umd_load_blob(struct umd_info *info, const void *data, size_t len); int umd_unload_blob(struct umd_info *info); int fork_usermode_driver(struct umd_info *info); +void umd_cleanup_helper(struct umd_info *info); #endif /* __LINUX_USERMODE_DRIVER_H__ */ diff --git a/kernel/bpf/preload/bpf_preload_kern.c b/kernel/bpf/preload/bpf_preload_kern.c index 79c5772465f1..356c4ca4f530 100644 --- a/kernel/bpf/preload/bpf_preload_kern.c +++ b/kernel/bpf/preload/bpf_preload_kern.c @@ -61,8 +61,10 @@ static int finish(void) if (n != sizeof(magic)) return -EPIPE; tgid = umd_ops.info.tgid; - wait_event(tgid->wait_pidfd, thread_group_exited(tgid)); - umd_ops.info.tgid = NULL; + if (tgid) { + wait_event(tgid->wait_pidfd, thread_group_exited(tgid)); + umd_cleanup_helper(&umd_ops.info); + } return 0; } @@ -80,10 +82,15 @@ static int __init load_umd(void) static void __exit fini_umd(void) { + struct pid *tgid; bpf_preload_ops = NULL; /* kill UMD in case it's still there due to earlier error */ - kill_pid(umd_ops.info.tgid, SIGKILL, 1); - umd_ops.info.tgid = NULL; + tgid = umd_ops.info.tgid; + if (tgid) { + kill_pid(tgid, SIGKILL, 1); + wait_event(tgid->wait_pidfd, thread_group_exited(tgid)); + umd_cleanup_helper(&umd_ops.info); + } umd_unload_blob(&umd_ops.info); } late_initcall(load_umd); diff --git a/kernel/usermode_driver.c b/kernel/usermode_driver.c index 0b35212ffc3d..6372deae27a0 100644 --- a/kernel/usermode_driver.c +++ b/kernel/usermode_driver.c @@ -140,13 +140,23 @@ static void umd_cleanup(struct subprocess_info *info) /* cleanup if umh_setup() was successful but exec failed */ if (info->retval) { - fput(umd_info->pipe_to_umh); - fput(umd_info->pipe_from_umh); - put_pid(umd_info->tgid); - umd_info->tgid = NULL; + umd_cleanup_helper(umd_info); } } +/** + * umd_cleanup_helper - release the resources which allocated in umd_setup + * @info: information about usermode driver + */ +void umd_cleanup_helper(struct umd_info *info) +{ + fput(info->pipe_to_umh); + fput(info->pipe_from_umh); + put_pid(info->tgid); + info->tgid = NULL; +} +EXPORT_SYMBOL_GPL(umd_cleanup_helper); + /** * fork_usermode_driver - fork a usermode driver * @info: information about usermode driver (shouldn't be NULL)