| Message ID | 20181016153510.16962-1-ap420073@gmail.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] net: bpfilter: use get_pid_task instead of pid_task | expand |
On Wed, Oct 17, 2018 at 12:35:10AM +0900, Taehee Yoo wrote: > pid_task() dereferences rcu protected tasks array. > But there is no rcu_read_lock() in shutdown_umh() routine so that > rcu_read_lock() is needed. > get_pid_task() is wrapper function of pid_task. it holds rcu_read_lock() > then calls pid_task(). if task isn't NULL, it increases reference count > of task. > > test commands: > %modprobe bpfilter > %modprobe -rv bpfilter > > splat looks like: > [15102.030932] ============================= > [15102.030957] WARNING: suspicious RCU usage > [15102.030985] 4.19.0-rc7+ #21 Not tainted > [15102.031010] ----------------------------- > [15102.031038] kernel/pid.c:330 suspicious rcu_dereference_check() usage! > [15102.031063] > other info that might help us debug this: > > [15102.031332] > rcu_scheduler_active = 2, debug_locks = 1 > [15102.031363] 1 lock held by modprobe/1570: > [15102.031389] #0: 00000000580ef2b0 (bpfilter_lock){+.+.}, at: stop_umh+0x13/0x52 [bpfilter] > [15102.031552] > stack backtrace: > [15102.031583] CPU: 1 PID: 1570 Comm: modprobe Not tainted 4.19.0-rc7+ #21 > [15102.031607] Hardware name: To be filled by O.E.M. To be filled by O.E.M./Aptio CRB, BIOS 5.6.5 07/08/2015 > [15102.031628] Call Trace: > [15102.031676] dump_stack+0xc9/0x16b > [15102.031723] ? show_regs_print_info+0x5/0x5 > [15102.031801] ? lockdep_rcu_suspicious+0x117/0x160 > [15102.031855] pid_task+0x134/0x160 > [15102.031900] ? find_vpid+0xf0/0xf0 > [15102.032017] shutdown_umh.constprop.1+0x1e/0x53 [bpfilter] > [15102.032055] stop_umh+0x46/0x52 [bpfilter] > [15102.032092] __x64_sys_delete_module+0x47e/0x570 > [ ... ] > > Fixes: d2ba09c17a06 ("net: add skeleton of bpfilter kernel module") > Signed-off-by: Taehee Yoo <ap420073@gmail.com> thanks a lot for the fix Acked-by: Alexei Starovoitov <ast@kernel.org>
From: Taehee Yoo <ap420073@gmail.com> Date: Wed, 17 Oct 2018 00:35:10 +0900 > pid_task() dereferences rcu protected tasks array. > But there is no rcu_read_lock() in shutdown_umh() routine so that > rcu_read_lock() is needed. > get_pid_task() is wrapper function of pid_task. it holds rcu_read_lock() > then calls pid_task(). if task isn't NULL, it increases reference count > of task. > > test commands: > %modprobe bpfilter > %modprobe -rv bpfilter ... > Fixes: d2ba09c17a06 ("net: add skeleton of bpfilter kernel module") > Signed-off-by: Taehee Yoo <ap420073@gmail.com> Applied and queued up for -stable, thanks.
diff --git a/net/bpfilter/bpfilter_kern.c b/net/bpfilter/bpfilter_kern.c index b64e1649993b..94e88f510c5b 100644 --- a/net/bpfilter/bpfilter_kern.c +++ b/net/bpfilter/bpfilter_kern.c @@ -23,9 +23,11 @@ static void shutdown_umh(struct umh_info *info) if (!info->pid) return; - tsk = pid_task(find_vpid(info->pid), PIDTYPE_PID); - if (tsk) + tsk = get_pid_task(find_vpid(info->pid), PIDTYPE_PID); + if (tsk) { force_sig(SIGKILL, tsk); + put_task_struct(tsk); + } fput(info->pipe_to_umh); fput(info->pipe_from_umh); info->pid = 0;
pid_task() dereferences rcu protected tasks array. But there is no rcu_read_lock() in shutdown_umh() routine so that rcu_read_lock() is needed. get_pid_task() is wrapper function of pid_task. it holds rcu_read_lock() then calls pid_task(). if task isn't NULL, it increases reference count of task. test commands: %modprobe bpfilter %modprobe -rv bpfilter splat looks like: [15102.030932] ============================= [15102.030957] WARNING: suspicious RCU usage [15102.030985] 4.19.0-rc7+ #21 Not tainted [15102.031010] ----------------------------- [15102.031038] kernel/pid.c:330 suspicious rcu_dereference_check() usage! [15102.031063] other info that might help us debug this: [15102.031332] rcu_scheduler_active = 2, debug_locks = 1 [15102.031363] 1 lock held by modprobe/1570: [15102.031389] #0: 00000000580ef2b0 (bpfilter_lock){+.+.}, at: stop_umh+0x13/0x52 [bpfilter] [15102.031552] stack backtrace: [15102.031583] CPU: 1 PID: 1570 Comm: modprobe Not tainted 4.19.0-rc7+ #21 [15102.031607] Hardware name: To be filled by O.E.M. To be filled by O.E.M./Aptio CRB, BIOS 5.6.5 07/08/2015 [15102.031628] Call Trace: [15102.031676] dump_stack+0xc9/0x16b [15102.031723] ? show_regs_print_info+0x5/0x5 [15102.031801] ? lockdep_rcu_suspicious+0x117/0x160 [15102.031855] pid_task+0x134/0x160 [15102.031900] ? find_vpid+0xf0/0xf0 [15102.032017] shutdown_umh.constprop.1+0x1e/0x53 [bpfilter] [15102.032055] stop_umh+0x46/0x52 [bpfilter] [15102.032092] __x64_sys_delete_module+0x47e/0x570 [ ... ] Fixes: d2ba09c17a06 ("net: add skeleton of bpfilter kernel module") Signed-off-by: Taehee Yoo <ap420073@gmail.com> --- net/bpfilter/bpfilter_kern.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)