![Taehee Yoo](/assets/img/avatar_default.png)
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: d2ba09c17a
("net: add skeleton of bpfilter kernel module")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
120 lines
2.5 KiB
C
120 lines
2.5 KiB
C
// SPDX-License-Identifier: GPL-2.0
|
|
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
|
|
#include <linux/init.h>
|
|
#include <linux/module.h>
|
|
#include <linux/umh.h>
|
|
#include <linux/bpfilter.h>
|
|
#include <linux/sched.h>
|
|
#include <linux/sched/signal.h>
|
|
#include <linux/fs.h>
|
|
#include <linux/file.h>
|
|
#include "msgfmt.h"
|
|
|
|
extern char bpfilter_umh_start;
|
|
extern char bpfilter_umh_end;
|
|
|
|
static struct umh_info info;
|
|
/* since ip_getsockopt() can run in parallel, serialize access to umh */
|
|
static DEFINE_MUTEX(bpfilter_lock);
|
|
|
|
static void shutdown_umh(struct umh_info *info)
|
|
{
|
|
struct task_struct *tsk;
|
|
|
|
if (!info->pid)
|
|
return;
|
|
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;
|
|
}
|
|
|
|
static void __stop_umh(void)
|
|
{
|
|
if (IS_ENABLED(CONFIG_INET)) {
|
|
bpfilter_process_sockopt = NULL;
|
|
shutdown_umh(&info);
|
|
}
|
|
}
|
|
|
|
static void stop_umh(void)
|
|
{
|
|
mutex_lock(&bpfilter_lock);
|
|
__stop_umh();
|
|
mutex_unlock(&bpfilter_lock);
|
|
}
|
|
|
|
static int __bpfilter_process_sockopt(struct sock *sk, int optname,
|
|
char __user *optval,
|
|
unsigned int optlen, bool is_set)
|
|
{
|
|
struct mbox_request req;
|
|
struct mbox_reply reply;
|
|
loff_t pos;
|
|
ssize_t n;
|
|
int ret = -EFAULT;
|
|
|
|
req.is_set = is_set;
|
|
req.pid = current->pid;
|
|
req.cmd = optname;
|
|
req.addr = (long __force __user)optval;
|
|
req.len = optlen;
|
|
mutex_lock(&bpfilter_lock);
|
|
if (!info.pid)
|
|
goto out;
|
|
n = __kernel_write(info.pipe_to_umh, &req, sizeof(req), &pos);
|
|
if (n != sizeof(req)) {
|
|
pr_err("write fail %zd\n", n);
|
|
__stop_umh();
|
|
ret = -EFAULT;
|
|
goto out;
|
|
}
|
|
pos = 0;
|
|
n = kernel_read(info.pipe_from_umh, &reply, sizeof(reply), &pos);
|
|
if (n != sizeof(reply)) {
|
|
pr_err("read fail %zd\n", n);
|
|
__stop_umh();
|
|
ret = -EFAULT;
|
|
goto out;
|
|
}
|
|
ret = reply.status;
|
|
out:
|
|
mutex_unlock(&bpfilter_lock);
|
|
return ret;
|
|
}
|
|
|
|
static int __init load_umh(void)
|
|
{
|
|
int err;
|
|
|
|
/* fork usermode process */
|
|
err = fork_usermode_blob(&bpfilter_umh_start,
|
|
&bpfilter_umh_end - &bpfilter_umh_start,
|
|
&info);
|
|
if (err)
|
|
return err;
|
|
pr_info("Loaded bpfilter_umh pid %d\n", info.pid);
|
|
|
|
/* health check that usermode process started correctly */
|
|
if (__bpfilter_process_sockopt(NULL, 0, NULL, 0, 0) != 0) {
|
|
stop_umh();
|
|
return -EFAULT;
|
|
}
|
|
if (IS_ENABLED(CONFIG_INET))
|
|
bpfilter_process_sockopt = &__bpfilter_process_sockopt;
|
|
|
|
return 0;
|
|
}
|
|
|
|
static void __exit fini_umh(void)
|
|
{
|
|
stop_umh();
|
|
}
|
|
module_init(load_umh);
|
|
module_exit(fini_umh);
|
|
MODULE_LICENSE("GPL");
|