Skip to content

Commit 71a8508

Browse files
TaeheeYoodavem330
authored andcommitted
net: bpfilter: disallow to remove bpfilter module while being used
The bpfilter.ko module can be removed while functions of the bpfilter.ko are executing. so panic can occurred. in order to protect that, locks can be used. a bpfilter_lock protects routines in the __bpfilter_process_sockopt() but it's not enough because __exit routine can be executed concurrently. Now, the bpfilter_umh can not run in parallel. So, the module do not removed while it's being used and it do not double-create UMH process. The members of the umh_info and the bpfilter_umh_ops are protected by the bpfilter_umh_ops.lock. test commands: while : do iptables -I FORWARD -m string --string ap --algo kmp & modprobe -rv bpfilter & done splat looks like: [ 298.623435] BUG: unable to handle kernel paging request at fffffbfff807440b [ 298.628512] #PF error: [normal kernel read fault] [ 298.633018] PGD 124327067 P4D 124327067 PUD 11c1a3067 PMD 119eb2067 PTE 0 [ 298.638859] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN PTI [ 298.638859] CPU: 0 PID: 2997 Comm: iptables Not tainted 4.20.0+ #154 [ 298.638859] RIP: 0010:__mutex_lock+0x6b9/0x16a0 [ 298.638859] Code: c0 00 00 e8 89 82 ff ff 80 bd 8f fc ff ff 00 0f 85 d9 05 00 00 48 8b 85 80 fc ff ff 48 bf 00 00 00 00 00 fc ff df 48 c1 e8 03 <80> 3c 38 00 0f 85 1d 0e 00 00 48 8b 85 c8 fc ff ff 49 39 47 58 c6 [ 298.638859] RSP: 0018:ffff88810e7777a0 EFLAGS: 00010202 [ 298.638859] RAX: 1ffffffff807440b RBX: ffff888111bd4d80 RCX: 0000000000000000 [ 298.638859] RDX: 1ffff110235ff806 RSI: ffff888111bd5538 RDI: dffffc0000000000 [ 298.638859] RBP: ffff88810e777b30 R08: 0000000080000002 R09: 0000000000000000 [ 298.638859] R10: 0000000000000000 R11: 0000000000000000 R12: fffffbfff168a42c [ 298.638859] R13: ffff888111bd4d80 R14: ffff8881040e9a05 R15: ffffffffc03a2000 [ 298.638859] FS: 00007f39e3758700(0000) GS:ffff88811ae00000(0000) knlGS:0000000000000000 [ 298.638859] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 298.638859] CR2: fffffbfff807440b CR3: 000000011243e000 CR4: 00000000001006f0 [ 298.638859] Call Trace: [ 298.638859] ? mutex_lock_io_nested+0x1560/0x1560 [ 298.638859] ? kasan_kmalloc+0xa0/0xd0 [ 298.638859] ? kmem_cache_alloc+0x1c2/0x260 [ 298.638859] ? __alloc_file+0x92/0x3c0 [ 298.638859] ? alloc_empty_file+0x43/0x120 [ 298.638859] ? alloc_file_pseudo+0x220/0x330 [ 298.638859] ? sock_alloc_file+0x39/0x160 [ 298.638859] ? __sys_socket+0x113/0x1d0 [ 298.638859] ? __x64_sys_socket+0x6f/0xb0 [ 298.638859] ? do_syscall_64+0x138/0x560 [ 298.638859] ? entry_SYSCALL_64_after_hwframe+0x49/0xbe [ 298.638859] ? __alloc_file+0x92/0x3c0 [ 298.638859] ? init_object+0x6b/0x80 [ 298.638859] ? cyc2ns_read_end+0x10/0x10 [ 298.638859] ? cyc2ns_read_end+0x10/0x10 [ 298.638859] ? hlock_class+0x140/0x140 [ 298.638859] ? sched_clock_local+0xd4/0x140 [ 298.638859] ? sched_clock_local+0xd4/0x140 [ 298.638859] ? check_flags.part.37+0x440/0x440 [ 298.638859] ? __lock_acquire+0x4f90/0x4f90 [ 298.638859] ? set_rq_offline.part.89+0x140/0x140 [ ... ] Fixes: d2ba09c ("net: add skeleton of bpfilter kernel module") Signed-off-by: Taehee Yoo <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent 61fbf59 commit 71a8508

File tree

3 files changed

+29
-23
lines changed

3 files changed

+29
-23
lines changed

include/linux/bpfilter.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ int bpfilter_ip_get_sockopt(struct sock *sk, int optname, char __user *optval,
1212
int __user *optlen);
1313
struct bpfilter_umh_ops {
1414
struct umh_info info;
15+
/* since ip_getsockopt() can run in parallel, serialize access to umh */
16+
struct mutex lock;
1517
int (*sockopt)(struct sock *sk, int optname,
1618
char __user *optval,
1719
unsigned int optlen, bool is_set);

net/bpfilter/bpfilter_kern.c

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,6 @@
1313
extern char bpfilter_umh_start;
1414
extern char bpfilter_umh_end;
1515

16-
/* since ip_getsockopt() can run in parallel, serialize access to umh */
17-
static DEFINE_MUTEX(bpfilter_lock);
18-
1916
static void shutdown_umh(void)
2017
{
2118
struct task_struct *tsk;
@@ -36,13 +33,6 @@ static void __stop_umh(void)
3633
shutdown_umh();
3734
}
3835

39-
static void stop_umh(void)
40-
{
41-
mutex_lock(&bpfilter_lock);
42-
__stop_umh();
43-
mutex_unlock(&bpfilter_lock);
44-
}
45-
4636
static int __bpfilter_process_sockopt(struct sock *sk, int optname,
4737
char __user *optval,
4838
unsigned int optlen, bool is_set)
@@ -58,7 +48,6 @@ static int __bpfilter_process_sockopt(struct sock *sk, int optname,
5848
req.cmd = optname;
5949
req.addr = (long __force __user)optval;
6050
req.len = optlen;
61-
mutex_lock(&bpfilter_lock);
6251
if (!bpfilter_ops.info.pid)
6352
goto out;
6453
n = __kernel_write(bpfilter_ops.info.pipe_to_umh, &req, sizeof(req),
@@ -80,7 +69,6 @@ static int __bpfilter_process_sockopt(struct sock *sk, int optname,
8069
}
8170
ret = reply.status;
8271
out:
83-
mutex_unlock(&bpfilter_lock);
8472
return ret;
8573
}
8674

@@ -99,7 +87,7 @@ static int start_umh(void)
9987

10088
/* health check that usermode process started correctly */
10189
if (__bpfilter_process_sockopt(NULL, 0, NULL, 0, 0) != 0) {
102-
stop_umh();
90+
shutdown_umh();
10391
return -EFAULT;
10492
}
10593

@@ -110,24 +98,30 @@ static int __init load_umh(void)
11098
{
11199
int err;
112100

113-
if (!bpfilter_ops.stop)
114-
return -EFAULT;
101+
mutex_lock(&bpfilter_ops.lock);
102+
if (!bpfilter_ops.stop) {
103+
err = -EFAULT;
104+
goto out;
105+
}
115106
err = start_umh();
116107
if (!err && IS_ENABLED(CONFIG_INET)) {
117108
bpfilter_ops.sockopt = &__bpfilter_process_sockopt;
118109
bpfilter_ops.start = &start_umh;
119110
}
120-
111+
out:
112+
mutex_unlock(&bpfilter_ops.lock);
121113
return err;
122114
}
123115

124116
static void __exit fini_umh(void)
125117
{
118+
mutex_lock(&bpfilter_ops.lock);
126119
if (IS_ENABLED(CONFIG_INET)) {
120+
shutdown_umh();
127121
bpfilter_ops.start = NULL;
128122
bpfilter_ops.sockopt = NULL;
129123
}
130-
stop_umh();
124+
mutex_unlock(&bpfilter_ops.lock);
131125
}
132126
module_init(load_umh);
133127
module_exit(fini_umh);

net/ipv4/bpfilter/sockopt.c

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,32 +14,41 @@ EXPORT_SYMBOL_GPL(bpfilter_ops);
1414

1515
static void bpfilter_umh_cleanup(struct umh_info *info)
1616
{
17+
mutex_lock(&bpfilter_ops.lock);
1718
bpfilter_ops.stop = true;
1819
fput(info->pipe_to_umh);
1920
fput(info->pipe_from_umh);
2021
info->pid = 0;
22+
mutex_unlock(&bpfilter_ops.lock);
2123
}
2224

2325
static int bpfilter_mbox_request(struct sock *sk, int optname,
2426
char __user *optval,
2527
unsigned int optlen, bool is_set)
2628
{
2729
int err;
28-
30+
mutex_lock(&bpfilter_ops.lock);
2931
if (!bpfilter_ops.sockopt) {
32+
mutex_unlock(&bpfilter_ops.lock);
3033
err = request_module("bpfilter");
34+
mutex_lock(&bpfilter_ops.lock);
3135

3236
if (err)
33-
return err;
34-
if (!bpfilter_ops.sockopt)
35-
return -ECHILD;
37+
goto out;
38+
if (!bpfilter_ops.sockopt) {
39+
err = -ECHILD;
40+
goto out;
41+
}
3642
}
3743
if (bpfilter_ops.stop) {
3844
err = bpfilter_ops.start();
3945
if (err)
40-
return err;
46+
goto out;
4147
}
42-
return bpfilter_ops.sockopt(sk, optname, optval, optlen, is_set);
48+
err = bpfilter_ops.sockopt(sk, optname, optval, optlen, is_set);
49+
out:
50+
mutex_unlock(&bpfilter_ops.lock);
51+
return err;
4352
}
4453

4554
int bpfilter_ip_set_sockopt(struct sock *sk, int optname, char __user *optval,
@@ -61,6 +70,7 @@ int bpfilter_ip_get_sockopt(struct sock *sk, int optname, char __user *optval,
6170

6271
static int __init bpfilter_sockopt_init(void)
6372
{
73+
mutex_init(&bpfilter_ops.lock);
6474
bpfilter_ops.stop = true;
6575
bpfilter_ops.info.cmdline = "bpfilter_umh";
6676
bpfilter_ops.info.cleanup = &bpfilter_umh_cleanup;

0 commit comments

Comments
 (0)