Re: WARNING: proc registration bug in snmp6_register_dev
critical section.
This remained resources will be released by netdev_run_todo() but it is
not protected by RTNL mutex. So that creating a new interface
routine(rtnl_newlink()) can be executed concurrently.
This routine would try to create the same proc entry
("/proc/net/dev_snmp6/")
At this point, this warning could occur.
In order to fix this problem, ->dellink() can be used.
But the patch would cause conflict with 34a9c361dd48
("hsr: remove hsr interface if all slaves are removed").
That commit is not merged into "net" branch yet.
So, I will send fix patch after the merge.
Test commands:
#SHELL1
ip link add dummy0 type dummy
ip link add dummy1 type dummy
ip link set dummy0 mtu 1300
while :
do
ip link add hsr0 type hsr slave1 dummy0 slave2 dummy1
done
#SHELL2
while :
do
ip link del hsr0
done
Thank you,
Taehee Yoo
>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkal...@googlegroups.com.
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
[PATCH net v4 4/4] 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 fbfff807440b
[ 298.628512] #PF error: [normal kernel read fault]
[ 298.633018] PGD 124327067 P4D 124327067 PUD 11c1a3067 PMD 119eb2067 PTE 0
[ 298.638859] Oops: [#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:88810ea0 EFLAGS: 00010202
[ 298.638859] RAX: 1807440b RBX: 888111bd4d80 RCX:
[ 298.638859] RDX: 1110235ff806 RSI: 888111bd5538 RDI: dc00
[ 298.638859] RBP: 88810e777b30 R08: 8002 R09:
[ 298.638859] R10: R11: R12: fbfff168a42c
[ 298.638859] R13: 888111bd4d80 R14: 8881040e9a05 R15: c03a2000
[ 298.638859] FS: 7f39e3758700() GS:88811ae0()
knlGS:
[ 298.638859] CS: 0010 DS: ES: CR0: 80050033
[ 298.638859] CR2: fbfff807440b CR3: 00011243e000 CR4: 001006f0
[ 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: d2ba09c17a06 ("net: add skeleton of bpfilter kernel module")
Signed-off-by: Taehee Yoo
---
include/linux/bpfilter.h | 2 ++
net/bpfilter/bpfilter_kern.c | 28 +++-
net/ipv4/bpfilter/sockopt.c | 22 --
3 files changed, 29 insertions(+), 23 deletions(-)
diff --git a/include/linux/bpfilter.h b/include/linux/bpfilter.h
index 8ebcbdd70bdc..d815622cd31e 100644
--- a/include/linux/bpfilter.h
+++ b/include/linux/bpfilter.h
@@ -12,6 +12,8 @@ int bpfilter_ip_get_sockopt(struct sock *sk, int optname,
char __user *optval,
int __user *optlen);
struct bpfilter_umh_ops {
struct umh_info info;
+ /* since ip_getsockopt() can run in parallel, serialize access to umh */
+ struct mutex lock;
int (*sockopt)(struct sock *sk, int optname,
char __user *optval,
unsigned int optlen, bool is_set);
diff --git a/net/bpfilter/bpfilter_kern.c b/net/bpfilter/bpfilter_kern.c
index c0fcde910a7a..7ee4fea93637 100644
--- a/net/bpfilter/bpfilter_kern.c
+++ b/net/bpfilter/bpfilter_kern.c
@@ -13,9 +13,6 @@
extern char bpfilter_umh_start;
extern char bpfilter_umh_end;
-/* since ip_getsockopt() can run in parallel, serialize access to umh */
-static DEFINE_MUTEX(bpfilter_lock);
-
static void shutdown_umh(void)
{
struct task_struct *tsk;
@@ -36,13 +33,6 @@ static void __stop_umh(void)
shutdown_umh();
}
-static void stop_umh(void)
-{
- mutex_lock(_lock);
- __stop_umh();
- mutex_unlock(_lock);
-}
-
static int __bpfilter_process_sockopt(struct sock *sk, int optname,
char __user *optval,
unsigned int optlen, bool is_set)
@@ -58,7 +48,6 @@ static int __bpfilter_process_sockopt(struct sock *sk, int
optname,
req.cmd = optname;
req.addr = (long __force __user)optval;
[PATCH net v4 3/4] net: bpfilter: restart bpfilter_umh when error occurred
The bpfilter_umh will be stopped via __stop_umh() when the bpfilter
error occurred.
The bpfilter_umh() couldn't start again because there is no restart
routine.
The section of the bpfilter_umh_{start/end} is no longer .init.rodata
because these area should be reused in the restart routine. hence
the section name is changed to .bpfilter_umh.
The bpfilter_ops->start() is restart callback. it will be called when
bpfilter_umh is stopped.
The stop bit means bpfilter_umh is stopped. this bit is set by both
start and stop routine.
Before this patch,
Test commands:
$ iptables -vnL
$ kill -9
$ iptables -vnL
[ 480.045136] bpfilter: write fail -32
$ iptables -vnL
All iptables commands will fail.
After this patch,
Test commands:
$ iptables -vnL
$ kill -9
$ iptables -vnL
$ iptables -vnL
Now, all iptables commands will work.
Fixes: d2ba09c17a06 ("net: add skeleton of bpfilter kernel module")
Signed-off-by: Taehee Yoo
---
v4 : check stop flag in the load_umh() to avoid a double-create UMH
include/linux/bpfilter.h | 2 ++
net/bpfilter/bpfilter_kern.c | 37 +++-
net/bpfilter/bpfilter_umh_blob.S | 2 +-
net/ipv4/bpfilter/sockopt.c | 11 +-
4 files changed, 40 insertions(+), 12 deletions(-)
diff --git a/include/linux/bpfilter.h b/include/linux/bpfilter.h
index 70ffeed280e9..8ebcbdd70bdc 100644
--- a/include/linux/bpfilter.h
+++ b/include/linux/bpfilter.h
@@ -15,6 +15,8 @@ struct bpfilter_umh_ops {
int (*sockopt)(struct sock *sk, int optname,
char __user *optval,
unsigned int optlen, bool is_set);
+ int (*start)(void);
+ bool stop;
};
extern struct bpfilter_umh_ops bpfilter_ops;
#endif
diff --git a/net/bpfilter/bpfilter_kern.c b/net/bpfilter/bpfilter_kern.c
index a68940b74c01..c0fcde910a7a 100644
--- a/net/bpfilter/bpfilter_kern.c
+++ b/net/bpfilter/bpfilter_kern.c
@@ -16,13 +16,14 @@ extern char bpfilter_umh_end;
/* since ip_getsockopt() can run in parallel, serialize access to umh */
static DEFINE_MUTEX(bpfilter_lock);
-static void shutdown_umh(struct umh_info *info)
+static void shutdown_umh(void)
{
struct task_struct *tsk;
- if (!info->pid)
+ if (bpfilter_ops.stop)
return;
- tsk = get_pid_task(find_vpid(info->pid), PIDTYPE_PID);
+
+ tsk = get_pid_task(find_vpid(bpfilter_ops.info.pid), PIDTYPE_PID);
if (tsk) {
force_sig(SIGKILL, tsk);
put_task_struct(tsk);
@@ -31,10 +32,8 @@ static void shutdown_umh(struct umh_info *info)
static void __stop_umh(void)
{
- if (IS_ENABLED(CONFIG_INET)) {
- bpfilter_ops.sockopt = NULL;
- shutdown_umh(_ops.info);
- }
+ if (IS_ENABLED(CONFIG_INET))
+ shutdown_umh();
}
static void stop_umh(void)
@@ -85,7 +84,7 @@ static int __bpfilter_process_sockopt(struct sock *sk, int
optname,
return ret;
}
-static int __init load_umh(void)
+static int start_umh(void)
{
int err;
@@ -95,6 +94,7 @@ static int __init load_umh(void)
_ops.info);
if (err)
return err;
+ bpfilter_ops.stop = false;
pr_info("Loaded bpfilter_umh pid %d\n", bpfilter_ops.info.pid);
/* health check that usermode process started correctly */
@@ -102,14 +102,31 @@ static int __init load_umh(void)
stop_umh();
return -EFAULT;
}
- if (IS_ENABLED(CONFIG_INET))
- bpfilter_ops.sockopt = &__bpfilter_process_sockopt;
return 0;
}
+static int __init load_umh(void)
+{
+ int err;
+
+ if (!bpfilter_ops.stop)
+ return -EFAULT;
+ err = start_umh();
+ if (!err && IS_ENABLED(CONFIG_INET)) {
+ bpfilter_ops.sockopt = &__bpfilter_process_sockopt;
+ bpfilter_ops.start = _umh;
+ }
+
+ return err;
+}
+
static void __exit fini_umh(void)
{
+ if (IS_ENABLED(CONFIG_INET)) {
+ bpfilter_ops.start = NULL;
+ bpfilter_ops.sockopt = NULL;
+ }
stop_umh();
}
module_init(load_umh);
diff --git a/net/bpfilter/bpfilter_umh_blob.S b/net/bpfilter/bpfilter_umh_blob.S
index 40311d10d2f2..7f1c521dcc2f 100644
--- a/net/bpfilter/bpfilter_umh_blob.S
+++ b/net/bpfilter/bpfilter_umh_blob.S
@@ -1,5 +1,5 @@
/* SPDX-License-Identifier: GPL-2.0 */
- .section .init.rodata, "a"
+ .section .bpfilter_umh, "a"
.global bpfilter_umh_start
bpfilter_umh_start:
.incbin "net/bpfilter/bpfilter_umh"
diff --git a/net/ipv4/bpfilter/sockopt.c b/net/ipv4/bpfilter/sockopt.c
index c326cfbc0f62..de84ede4e765 100644
--- a/net/ipv4/bpfilter/sockopt.c
+++ b/net/ipv4/bpfilter/sockopt.c
@@ -14,6 +14,7 @@ EXPORT_SYMBOL_GPL(bpfilter_ops);
static void bpfilter_umh_cleanup(struct umh_info
[PATCH net v4 2/4] net: bpfilter: use cleanup callback to release umh_info
Now, UMH process is killed, do_exit() calls the umh_info->cleanup callback
to release members of the umh_info.
This patch makes bpfilter_umh's cleanup routine to use the
umh_info->cleanup callback.
Signed-off-by: Taehee Yoo
---
include/linux/bpfilter.h | 11 ---
net/bpfilter/bpfilter_kern.c | 23 ++-
net/ipv4/bpfilter/sockopt.c | 33 ++---
3 files changed, 44 insertions(+), 23 deletions(-)
diff --git a/include/linux/bpfilter.h b/include/linux/bpfilter.h
index f02cee0225d4..70ffeed280e9 100644
--- a/include/linux/bpfilter.h
+++ b/include/linux/bpfilter.h
@@ -3,13 +3,18 @@
#define _LINUX_BPFILTER_H
#include
+#include
struct sock;
int bpfilter_ip_set_sockopt(struct sock *sk, int optname, char __user *optval,
unsigned int optlen);
int bpfilter_ip_get_sockopt(struct sock *sk, int optname, char __user *optval,
int __user *optlen);
-extern int (*bpfilter_process_sockopt)(struct sock *sk, int optname,
- char __user *optval,
- unsigned int optlen, bool is_set);
+struct bpfilter_umh_ops {
+ struct umh_info info;
+ int (*sockopt)(struct sock *sk, int optname,
+ char __user *optval,
+ unsigned int optlen, bool is_set);
+};
+extern struct bpfilter_umh_ops bpfilter_ops;
#endif
diff --git a/net/bpfilter/bpfilter_kern.c b/net/bpfilter/bpfilter_kern.c
index 7acfc83087d5..a68940b74c01 100644
--- a/net/bpfilter/bpfilter_kern.c
+++ b/net/bpfilter/bpfilter_kern.c
@@ -13,7 +13,6 @@
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);
@@ -28,16 +27,13 @@ static void shutdown_umh(struct umh_info *info)
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();
+ bpfilter_ops.sockopt = NULL;
+ shutdown_umh(_ops.info);
}
}
@@ -64,9 +60,10 @@ static int __bpfilter_process_sockopt(struct sock *sk, int
optname,
req.addr = (long __force __user)optval;
req.len = optlen;
mutex_lock(_lock);
- if (!info.pid)
+ if (!bpfilter_ops.info.pid)
goto out;
- n = __kernel_write(info.pipe_to_umh, , sizeof(req), );
+ n = __kernel_write(bpfilter_ops.info.pipe_to_umh, , sizeof(req),
+ );
if (n != sizeof(req)) {
pr_err("write fail %zd\n", n);
__stop_umh();
@@ -74,7 +71,8 @@ static int __bpfilter_process_sockopt(struct sock *sk, int
optname,
goto out;
}
pos = 0;
- n = kernel_read(info.pipe_from_umh, , sizeof(reply), );
+ n = kernel_read(bpfilter_ops.info.pipe_from_umh, , sizeof(reply),
+ );
if (n != sizeof(reply)) {
pr_err("read fail %zd\n", n);
__stop_umh();
@@ -92,13 +90,12 @@ static int __init load_umh(void)
int err;
/* fork usermode process */
- info.cmdline = "bpfilter_umh";
err = fork_usermode_blob(_umh_start,
_umh_end - _umh_start,
-);
+_ops.info);
if (err)
return err;
- pr_info("Loaded bpfilter_umh pid %d\n", info.pid);
+ pr_info("Loaded bpfilter_umh pid %d\n", bpfilter_ops.info.pid);
/* health check that usermode process started correctly */
if (__bpfilter_process_sockopt(NULL, 0, NULL, 0, 0) != 0) {
@@ -106,7 +103,7 @@ static int __init load_umh(void)
return -EFAULT;
}
if (IS_ENABLED(CONFIG_INET))
- bpfilter_process_sockopt = &__bpfilter_process_sockopt;
+ bpfilter_ops.sockopt = &__bpfilter_process_sockopt;
return 0;
}
diff --git a/net/ipv4/bpfilter/sockopt.c b/net/ipv4/bpfilter/sockopt.c
index 5e04ed25bc0e..c326cfbc0f62 100644
--- a/net/ipv4/bpfilter/sockopt.c
+++ b/net/ipv4/bpfilter/sockopt.c
@@ -1,28 +1,37 @@
// SPDX-License-Identifier: GPL-2.0
+#include
+#include
#include
#include
#include
#include
#include
+#include
+#include
-int (*bpfilter_process_sockopt)(struct sock *sk, int optname,
- char __user *optval,
- unsigned int optlen, bool is_set);
-EXPORT_SYMBOL_GPL(bpfilter_process_sockopt);
+struct bpfilter_umh_ops bpfilter_ops;
+EXPORT_SYMBOL_GPL(bpfilter_ops);
+
+static void bpfi
[PATCH net v4 1/4] umh: add exit routine for UMH process
A UMH process which is created by the fork_usermode_blob() such as
bpfilter needs to release members of the umh_info when process is
terminated.
But the do_exit() does not release members of the umh_info. hence module
which uses UMH needs own code to detect whether UMH process is
terminated or not.
But this implementation needs extra code for checking the status of
UMH process. it eventually makes the code more complex.
The new PF_UMH flag is added and it is used to identify UMH processes.
The exit_umh() does not release members of the umh_info.
Hence umh_info->cleanup callback should release both members of the
umh_info and the private data.
Suggested-by: David S. Miller
Signed-off-by: Taehee Yoo
---
v4 : declare the exit_umh() as static inline
include/linux/sched.h | 9 +
include/linux/umh.h | 2 ++
kernel/exit.c | 1 +
kernel/umh.c | 33 +++--
4 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 89541d248893..e35e35b9fc48 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1406,6 +1406,7 @@ extern struct pid *cad_pid;
#define PF_RANDOMIZE 0x0040 /* Randomize virtual address
space */
#define PF_SWAPWRITE 0x0080 /* Allowed to write to swap */
#define PF_MEMSTALL0x0100 /* Stalled due to lack of
memory */
+#define PF_UMH 0x0200 /* I'm an Usermodehelper
process */
#define PF_NO_SETAFFINITY 0x0400 /* Userland is not allowed to
meddle with cpus_allowed */
#define PF_MCE_EARLY 0x0800 /* Early kill for mce process
policy */
#define PF_MUTEX_TESTER0x2000 /* Thread belongs to
the rt mutex tester */
@@ -1904,6 +1905,14 @@ static inline void rseq_execve(struct task_struct *t)
#endif
+void __exit_umh(struct task_struct *tsk);
+
+static inline void exit_umh(struct task_struct *tsk)
+{
+ if (unlikely(tsk->flags & PF_UMH))
+ __exit_umh(tsk);
+}
+
#ifdef CONFIG_DEBUG_RSEQ
void rseq_syscall(struct pt_regs *regs);
diff --git a/include/linux/umh.h b/include/linux/umh.h
index 235f51b62c71..0c08de356d0d 100644
--- a/include/linux/umh.h
+++ b/include/linux/umh.h
@@ -47,6 +47,8 @@ struct umh_info {
const char *cmdline;
struct file *pipe_to_umh;
struct file *pipe_from_umh;
+ struct list_head list;
+ void (*cleanup)(struct umh_info *info);
pid_t pid;
};
int fork_usermode_blob(void *data, size_t len, struct umh_info *info);
diff --git a/kernel/exit.c b/kernel/exit.c
index 8a01b671dc1f..dad70419195c 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -866,6 +866,7 @@ void __noreturn do_exit(long code)
exit_task_namespaces(tsk);
exit_task_work(tsk);
exit_thread(tsk);
+ exit_umh(tsk);
/*
* Flush inherited counters to the parent - before the parent
diff --git a/kernel/umh.c b/kernel/umh.c
index 0baa672e023c..d937cbad903a 100644
--- a/kernel/umh.c
+++ b/kernel/umh.c
@@ -37,6 +37,8 @@ static kernel_cap_t usermodehelper_bset = CAP_FULL_SET;
static kernel_cap_t usermodehelper_inheritable = CAP_FULL_SET;
static DEFINE_SPINLOCK(umh_sysctl_lock);
static DECLARE_RWSEM(umhelper_sem);
+static LIST_HEAD(umh_list);
+static DEFINE_MUTEX(umh_list_lock);
static void call_usermodehelper_freeinfo(struct subprocess_info *info)
{
@@ -100,10 +102,12 @@ static int call_usermodehelper_exec_async(void *data)
commit_creds(new);
sub_info->pid = task_pid_nr(current);
- if (sub_info->file)
+ if (sub_info->file) {
retval = do_execve_file(sub_info->file,
sub_info->argv, sub_info->envp);
- else
+ if (!retval)
+ current->flags |= PF_UMH;
+ } else
retval = do_execve(getname_kernel(sub_info->path),
(const char __user *const __user
*)sub_info->argv,
(const char __user *const __user
*)sub_info->envp);
@@ -517,6 +521,11 @@ int fork_usermode_blob(void *data, size_t len, struct
umh_info *info)
goto out;
err = call_usermodehelper_exec(sub_info, UMH_WAIT_EXEC);
+ if (!err) {
+ mutex_lock(_list_lock);
+ list_add(>list, _list);
+ mutex_unlock(_list_lock);
+ }
out:
fput(file);
return err;
@@ -679,6 +688,26 @@ static int proc_cap_handler(struct ctl_table *table, int
write,
return 0;
}
+void __exit_umh(struct task_struct *tsk)
+{
+ struct umh_info *info;
+ pid_t pid = tsk->pid;
+
+ mutex_lock(_list_lock);
+ list_for_each_entry(info, _list, list) {
+ if (info->pid == pid) {
+ list_del(>list);
+ mutex_
[PATCH net v4 0/4] net: bpfilter: fix two bugs in bpfilter
This patches fix two bugs in the bpfilter_umh which are related in iptables command. The first patch adds an exit code for UMH process. This provides an opportunity to cleanup members of the umh_info to modules which use the UMH. In order to identify UMH processes, a new flag PF_UMH is added. The second patch makes the bpfilter_umh use UMH cleanup callback. The third patch adds re-start routine for the bpfilter_umh. The bpfilter_umh does not re-start after error occurred. because there is no re-start routine in the module. The fourth patch ensures that the bpfilter.ko module will not removed while it's being used. The bpfilter.ko is not protected by locks or module reference counter. Therefore that can be removed while module is being used. In order to protect that, mutex is used. The first and second patch are preparation patches for the third and fourth patch. TEST #1 while : do modprobe bpfilter kill -9 iptables -vnL done TEST #2 while : do iptables -I FORWARD -m string --string ap --algo kmp & iptables -F & modprobe -rv bpfilter & done TEST #3 while : do modprobe bpfilter & modprobe -rv bpfilter & done The TEST1 makes a failure of iptables command. This is fixed by the third patch. The TEST2 makes a panic because of a race condition in the bpfilter_umh module. This is fixed by the fourth patch. The TEST3 makes a double-create UMH process. This is fixed by the third and fourth patch. v4 : - declare the exit_umh() as static inline - check stop flag in the load_umh() to avoid a double-create UMH v3 : - Avoid unnecessary list lookup for non-UMH processes - Add a new PF_UMH flag v2 : add the first and second patch v1 : Initial patch Taehee Yoo (4): umh: add exit routine for UMH process net: bpfilter: use cleanup callback to release umh_info net: bpfilter: restart bpfilter_umh when error occurred net: bpfilter: disallow to remove bpfilter module while being used include/linux/bpfilter.h | 15 +-- include/linux/sched.h| 9 include/linux/umh.h | 2 + kernel/exit.c| 1 + kernel/umh.c | 33 +- net/bpfilter/bpfilter_kern.c | 76 ++-- net/bpfilter/bpfilter_umh_blob.S | 2 +- net/ipv4/bpfilter/sockopt.c | 58 +++- 8 files changed, 146 insertions(+), 50 deletions(-) -- 2.17.1
Re: [PATCH net v3 1/4] umh: add exit routine for UMH process
On Tue, 8 Jan 2019 at 00:25, David Miller wrote:
>
> From: Taehee Yoo
> Date: Mon, 7 Jan 2019 21:10:14 +0900
>
> > @@ -679,6 +688,29 @@ static int proc_cap_handler(struct ctl_table *table,
> > int write,
> > return 0;
> > }
> >
> > +void exit_umh(struct task_struct *tsk)
> > +{
> > + struct umh_info *info;
> > + pid_t pid = tsk->pid;
> > +
> > + if (!(tsk->flags & PF_UMH))
> > + return;
>
> Let's really make this low cost.
>
> In linux/sched.h or similar:
>
> void __exit_umh(struct task_struct *tsk);
>
> static inline void exit_umh(struct task_struct *tsk)
> {
> if (unlikely(tsk->flags & PF_UMH))
> __exit_umh(tsk);
> }
>
> Thank you.
Thanks a lot for the review!
I will send a v4 patch.
[PATCH net v3 4/4] 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 fbfff807440b
[ 298.628512] #PF error: [normal kernel read fault]
[ 298.633018] PGD 124327067 P4D 124327067 PUD 11c1a3067 PMD 119eb2067 PTE 0
[ 298.638859] Oops: [#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:88810ea0 EFLAGS: 00010202
[ 298.638859] RAX: 1807440b RBX: 888111bd4d80 RCX:
[ 298.638859] RDX: 1110235ff806 RSI: 888111bd5538 RDI: dc00
[ 298.638859] RBP: 88810e777b30 R08: 8002 R09:
[ 298.638859] R10: R11: R12: fbfff168a42c
[ 298.638859] R13: 888111bd4d80 R14: 8881040e9a05 R15: c03a2000
[ 298.638859] FS: 7f39e3758700() GS:88811ae0()
knlGS:
[ 298.638859] CS: 0010 DS: ES: CR0: 80050033
[ 298.638859] CR2: fbfff807440b CR3: 00011243e000 CR4: 001006f0
[ 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: d2ba09c17a06 ("net: add skeleton of bpfilter kernel module")
Signed-off-by: Taehee Yoo
---
include/linux/bpfilter.h | 2 ++
net/bpfilter/bpfilter_kern.c | 20 ++--
net/ipv4/bpfilter/sockopt.c | 21 -
3 files changed, 24 insertions(+), 19 deletions(-)
diff --git a/include/linux/bpfilter.h b/include/linux/bpfilter.h
index 8ebcbdd70bdc..d815622cd31e 100644
--- a/include/linux/bpfilter.h
+++ b/include/linux/bpfilter.h
@@ -12,6 +12,8 @@ int bpfilter_ip_get_sockopt(struct sock *sk, int optname,
char __user *optval,
int __user *optlen);
struct bpfilter_umh_ops {
struct umh_info info;
+ /* since ip_getsockopt() can run in parallel, serialize access to umh */
+ struct mutex lock;
int (*sockopt)(struct sock *sk, int optname,
char __user *optval,
unsigned int optlen, bool is_set);
diff --git a/net/bpfilter/bpfilter_kern.c b/net/bpfilter/bpfilter_kern.c
index 33d6b159ba88..eedb83863cb0 100644
--- a/net/bpfilter/bpfilter_kern.c
+++ b/net/bpfilter/bpfilter_kern.c
@@ -13,9 +13,6 @@
extern char bpfilter_umh_start;
extern char bpfilter_umh_end;
-/* since ip_getsockopt() can run in parallel, serialize access to umh */
-static DEFINE_MUTEX(bpfilter_lock);
-
static void shutdown_umh(void)
{
struct task_struct *tsk;
@@ -36,13 +33,6 @@ static void __stop_umh(void)
shutdown_umh();
}
-static void stop_umh(void)
-{
- mutex_lock(_lock);
- __stop_umh();
- mutex_unlock(_lock);
-}
-
static int __bpfilter_process_sockopt(struct sock *sk, int optname,
char __user *optval,
unsigned int optlen, bool is_set)
@@ -58,7 +48,6 @@ static int __bpfilter_process_sockopt(struct sock *sk, int
optname,
req.cmd = optname;
req.addr = (long __force __user)optval;
[PATCH net v3 2/4] net: bpfilter: use cleanup callback to release umh_info
Now, UMH process is killed, do_exit() calls the umh_info->cleanup callback
to release members of the umh_info.
This patch makes bpfilter_umh's cleanup routine to use the
umh_info->cleanup callback.
Signed-off-by: Taehee Yoo
---
include/linux/bpfilter.h | 11 ---
net/bpfilter/bpfilter_kern.c | 23 ++-
net/ipv4/bpfilter/sockopt.c | 33 ++---
3 files changed, 44 insertions(+), 23 deletions(-)
diff --git a/include/linux/bpfilter.h b/include/linux/bpfilter.h
index f02cee0225d4..70ffeed280e9 100644
--- a/include/linux/bpfilter.h
+++ b/include/linux/bpfilter.h
@@ -3,13 +3,18 @@
#define _LINUX_BPFILTER_H
#include
+#include
struct sock;
int bpfilter_ip_set_sockopt(struct sock *sk, int optname, char __user *optval,
unsigned int optlen);
int bpfilter_ip_get_sockopt(struct sock *sk, int optname, char __user *optval,
int __user *optlen);
-extern int (*bpfilter_process_sockopt)(struct sock *sk, int optname,
- char __user *optval,
- unsigned int optlen, bool is_set);
+struct bpfilter_umh_ops {
+ struct umh_info info;
+ int (*sockopt)(struct sock *sk, int optname,
+ char __user *optval,
+ unsigned int optlen, bool is_set);
+};
+extern struct bpfilter_umh_ops bpfilter_ops;
#endif
diff --git a/net/bpfilter/bpfilter_kern.c b/net/bpfilter/bpfilter_kern.c
index 7acfc83087d5..a68940b74c01 100644
--- a/net/bpfilter/bpfilter_kern.c
+++ b/net/bpfilter/bpfilter_kern.c
@@ -13,7 +13,6 @@
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);
@@ -28,16 +27,13 @@ static void shutdown_umh(struct umh_info *info)
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();
+ bpfilter_ops.sockopt = NULL;
+ shutdown_umh(_ops.info);
}
}
@@ -64,9 +60,10 @@ static int __bpfilter_process_sockopt(struct sock *sk, int
optname,
req.addr = (long __force __user)optval;
req.len = optlen;
mutex_lock(_lock);
- if (!info.pid)
+ if (!bpfilter_ops.info.pid)
goto out;
- n = __kernel_write(info.pipe_to_umh, , sizeof(req), );
+ n = __kernel_write(bpfilter_ops.info.pipe_to_umh, , sizeof(req),
+ );
if (n != sizeof(req)) {
pr_err("write fail %zd\n", n);
__stop_umh();
@@ -74,7 +71,8 @@ static int __bpfilter_process_sockopt(struct sock *sk, int
optname,
goto out;
}
pos = 0;
- n = kernel_read(info.pipe_from_umh, , sizeof(reply), );
+ n = kernel_read(bpfilter_ops.info.pipe_from_umh, , sizeof(reply),
+ );
if (n != sizeof(reply)) {
pr_err("read fail %zd\n", n);
__stop_umh();
@@ -92,13 +90,12 @@ static int __init load_umh(void)
int err;
/* fork usermode process */
- info.cmdline = "bpfilter_umh";
err = fork_usermode_blob(_umh_start,
_umh_end - _umh_start,
-);
+_ops.info);
if (err)
return err;
- pr_info("Loaded bpfilter_umh pid %d\n", info.pid);
+ pr_info("Loaded bpfilter_umh pid %d\n", bpfilter_ops.info.pid);
/* health check that usermode process started correctly */
if (__bpfilter_process_sockopt(NULL, 0, NULL, 0, 0) != 0) {
@@ -106,7 +103,7 @@ static int __init load_umh(void)
return -EFAULT;
}
if (IS_ENABLED(CONFIG_INET))
- bpfilter_process_sockopt = &__bpfilter_process_sockopt;
+ bpfilter_ops.sockopt = &__bpfilter_process_sockopt;
return 0;
}
diff --git a/net/ipv4/bpfilter/sockopt.c b/net/ipv4/bpfilter/sockopt.c
index 5e04ed25bc0e..c326cfbc0f62 100644
--- a/net/ipv4/bpfilter/sockopt.c
+++ b/net/ipv4/bpfilter/sockopt.c
@@ -1,28 +1,37 @@
// SPDX-License-Identifier: GPL-2.0
+#include
+#include
#include
#include
#include
#include
#include
+#include
+#include
-int (*bpfilter_process_sockopt)(struct sock *sk, int optname,
- char __user *optval,
- unsigned int optlen, bool is_set);
-EXPORT_SYMBOL_GPL(bpfilter_process_sockopt);
+struct bpfilter_umh_ops bpfilter_ops;
+EXPORT_SYMBOL_GPL(bpfilter_ops);
+
+static void bpfi
[PATCH net v3 3/4] net: bpfilter: restart bpfilter_umh when error occurred
The bpfilter_umh will be stopped via __stop_umh() when the bpfilter
error occurred.
The bpfilter_umh() couldn't start again because there is no restart
routine.
The section of the bpfilter_umh_{start/end} is no longer .init.rodata
because these area should be reused in the restart routine. hence
the section name is changed to .bpfilter_umh.
The bpfilter_ops->start() is restart callback. it will be called when
bpfilter_umh is stopped.
The stop bit means bpfilter_umh is stopped. this bit is set by both
start and stop routine.
Before this patch,
Test commands:
$ iptables -vnL
$ kill -9
$ iptables -vnL
[ 480.045136] bpfilter: write fail -32
$ iptables -vnL
All iptables commands will fail.
After this patch,
Test commands:
$ iptables -vnL
$ kill -9
$ iptables -vnL
$ iptables -vnL
Now, all iptables commands will work.
Fixes: d2ba09c17a06 ("net: add skeleton of bpfilter kernel module")
Signed-off-by: Taehee Yoo
---
include/linux/bpfilter.h | 2 ++
net/bpfilter/bpfilter_kern.c | 35 +++-
net/bpfilter/bpfilter_umh_blob.S | 2 +-
net/ipv4/bpfilter/sockopt.c | 11 +-
4 files changed, 38 insertions(+), 12 deletions(-)
diff --git a/include/linux/bpfilter.h b/include/linux/bpfilter.h
index 70ffeed280e9..8ebcbdd70bdc 100644
--- a/include/linux/bpfilter.h
+++ b/include/linux/bpfilter.h
@@ -15,6 +15,8 @@ struct bpfilter_umh_ops {
int (*sockopt)(struct sock *sk, int optname,
char __user *optval,
unsigned int optlen, bool is_set);
+ int (*start)(void);
+ bool stop;
};
extern struct bpfilter_umh_ops bpfilter_ops;
#endif
diff --git a/net/bpfilter/bpfilter_kern.c b/net/bpfilter/bpfilter_kern.c
index a68940b74c01..33d6b159ba88 100644
--- a/net/bpfilter/bpfilter_kern.c
+++ b/net/bpfilter/bpfilter_kern.c
@@ -16,13 +16,14 @@ extern char bpfilter_umh_end;
/* since ip_getsockopt() can run in parallel, serialize access to umh */
static DEFINE_MUTEX(bpfilter_lock);
-static void shutdown_umh(struct umh_info *info)
+static void shutdown_umh(void)
{
struct task_struct *tsk;
- if (!info->pid)
+ if (bpfilter_ops.stop)
return;
- tsk = get_pid_task(find_vpid(info->pid), PIDTYPE_PID);
+
+ tsk = get_pid_task(find_vpid(bpfilter_ops.info.pid), PIDTYPE_PID);
if (tsk) {
force_sig(SIGKILL, tsk);
put_task_struct(tsk);
@@ -31,10 +32,8 @@ static void shutdown_umh(struct umh_info *info)
static void __stop_umh(void)
{
- if (IS_ENABLED(CONFIG_INET)) {
- bpfilter_ops.sockopt = NULL;
- shutdown_umh(_ops.info);
- }
+ if (IS_ENABLED(CONFIG_INET))
+ shutdown_umh();
}
static void stop_umh(void)
@@ -85,7 +84,7 @@ static int __bpfilter_process_sockopt(struct sock *sk, int
optname,
return ret;
}
-static int __init load_umh(void)
+static int start_umh(void)
{
int err;
@@ -95,6 +94,7 @@ static int __init load_umh(void)
_ops.info);
if (err)
return err;
+ bpfilter_ops.stop = false;
pr_info("Loaded bpfilter_umh pid %d\n", bpfilter_ops.info.pid);
/* health check that usermode process started correctly */
@@ -102,14 +102,29 @@ static int __init load_umh(void)
stop_umh();
return -EFAULT;
}
- if (IS_ENABLED(CONFIG_INET))
- bpfilter_ops.sockopt = &__bpfilter_process_sockopt;
return 0;
}
+static int __init load_umh(void)
+{
+ int err;
+
+ err = start_umh();
+ if (!err && IS_ENABLED(CONFIG_INET)) {
+ bpfilter_ops.sockopt = &__bpfilter_process_sockopt;
+ bpfilter_ops.start = _umh;
+ }
+
+ return err;
+}
+
static void __exit fini_umh(void)
{
+ if (IS_ENABLED(CONFIG_INET)) {
+ bpfilter_ops.start = NULL;
+ bpfilter_ops.sockopt = NULL;
+ }
stop_umh();
}
module_init(load_umh);
diff --git a/net/bpfilter/bpfilter_umh_blob.S b/net/bpfilter/bpfilter_umh_blob.S
index 40311d10d2f2..7f1c521dcc2f 100644
--- a/net/bpfilter/bpfilter_umh_blob.S
+++ b/net/bpfilter/bpfilter_umh_blob.S
@@ -1,5 +1,5 @@
/* SPDX-License-Identifier: GPL-2.0 */
- .section .init.rodata, "a"
+ .section .bpfilter_umh, "a"
.global bpfilter_umh_start
bpfilter_umh_start:
.incbin "net/bpfilter/bpfilter_umh"
diff --git a/net/ipv4/bpfilter/sockopt.c b/net/ipv4/bpfilter/sockopt.c
index c326cfbc0f62..de84ede4e765 100644
--- a/net/ipv4/bpfilter/sockopt.c
+++ b/net/ipv4/bpfilter/sockopt.c
@@ -14,6 +14,7 @@ EXPORT_SYMBOL_GPL(bpfilter_ops);
static void bpfilter_umh_cleanup(struct umh_info *info)
{
+ bpfilter_ops.stop = true;
fput(info->pipe_to_umh);
fput(info->pipe_from_umh);
info-&g
[PATCH net v3 1/4] umh: add exit routine for UMH process
A UMH process which is created by the fork_usermode_blob() such as
bpfilter needs to release members of the umh_info when process is
terminated.
But the do_exit() does not release members of the umh_info. hence module
which uses UMH needs own code to detect whether UMH process is
terminated or not.
But this implementation needs extra code for checking the status of
UMH process. it eventually makes the code more complex.
The new PF_UMH flag is added and it is used to identify UMH processes.
The exit_umh() does not release members of the umh_info.
Hence umh_info->cleanup callback should release both members of the
umh_info and the private data.
Suggested-by: David S. Miller
Signed-off-by: Taehee Yoo
---
v3 :
- Avoid unnecessary list lookup for non-UMH processes
- Add a new PF_UMH flag
include/linux/sched.h | 1 +
include/linux/umh.h | 4
kernel/exit.c | 1 +
kernel/umh.c | 36 ++--
4 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 89541d248893..965da2d54c06 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1406,6 +1406,7 @@ extern struct pid *cad_pid;
#define PF_RANDOMIZE 0x0040 /* Randomize virtual address
space */
#define PF_SWAPWRITE 0x0080 /* Allowed to write to swap */
#define PF_MEMSTALL0x0100 /* Stalled due to lack of
memory */
+#define PF_UMH 0x0200 /* I'm an Usermodehelper
process */
#define PF_NO_SETAFFINITY 0x0400 /* Userland is not allowed to
meddle with cpus_allowed */
#define PF_MCE_EARLY 0x0800 /* Early kill for mce process
policy */
#define PF_MUTEX_TESTER0x2000 /* Thread belongs to
the rt mutex tester */
diff --git a/include/linux/umh.h b/include/linux/umh.h
index 235f51b62c71..c645f0a19103 100644
--- a/include/linux/umh.h
+++ b/include/linux/umh.h
@@ -47,6 +47,8 @@ struct umh_info {
const char *cmdline;
struct file *pipe_to_umh;
struct file *pipe_from_umh;
+ struct list_head list;
+ void (*cleanup)(struct umh_info *info);
pid_t pid;
};
int fork_usermode_blob(void *data, size_t len, struct umh_info *info);
@@ -75,6 +77,8 @@ static inline void usermodehelper_enable(void)
__usermodehelper_set_disable_depth(UMH_ENABLED);
}
+void exit_umh(struct task_struct *tsk);
+
extern int usermodehelper_read_trylock(void);
extern long usermodehelper_read_lock_wait(long timeout);
extern void usermodehelper_read_unlock(void);
diff --git a/kernel/exit.c b/kernel/exit.c
index 8a01b671dc1f..dad70419195c 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -866,6 +866,7 @@ void __noreturn do_exit(long code)
exit_task_namespaces(tsk);
exit_task_work(tsk);
exit_thread(tsk);
+ exit_umh(tsk);
/*
* Flush inherited counters to the parent - before the parent
diff --git a/kernel/umh.c b/kernel/umh.c
index 0baa672e023c..d96e8cd14384 100644
--- a/kernel/umh.c
+++ b/kernel/umh.c
@@ -37,6 +37,8 @@ static kernel_cap_t usermodehelper_bset = CAP_FULL_SET;
static kernel_cap_t usermodehelper_inheritable = CAP_FULL_SET;
static DEFINE_SPINLOCK(umh_sysctl_lock);
static DECLARE_RWSEM(umhelper_sem);
+static LIST_HEAD(umh_list);
+static DEFINE_MUTEX(umh_list_lock);
static void call_usermodehelper_freeinfo(struct subprocess_info *info)
{
@@ -100,10 +102,12 @@ static int call_usermodehelper_exec_async(void *data)
commit_creds(new);
sub_info->pid = task_pid_nr(current);
- if (sub_info->file)
+ if (sub_info->file) {
retval = do_execve_file(sub_info->file,
sub_info->argv, sub_info->envp);
- else
+ if (!retval)
+ current->flags |= PF_UMH;
+ } else
retval = do_execve(getname_kernel(sub_info->path),
(const char __user *const __user
*)sub_info->argv,
(const char __user *const __user
*)sub_info->envp);
@@ -517,6 +521,11 @@ int fork_usermode_blob(void *data, size_t len, struct
umh_info *info)
goto out;
err = call_usermodehelper_exec(sub_info, UMH_WAIT_EXEC);
+ if (!err) {
+ mutex_lock(_list_lock);
+ list_add(>list, _list);
+ mutex_unlock(_list_lock);
+ }
out:
fput(file);
return err;
@@ -679,6 +688,29 @@ static int proc_cap_handler(struct ctl_table *table, int
write,
return 0;
}
+void exit_umh(struct task_struct *tsk)
+{
+ struct umh_info *info;
+ pid_t pid = tsk->pid;
+
+ if (!(tsk->flags & PF_UMH))
+ return;
+
+ mutex_lock(_list_lock);
+ list_for_each_entry(info, _list, list) {
+ if (info->pid
[PATCH net v3 0/4] net: bpfilter: fix two bugs in bpfilter
This patches fix two bugs in the bpfilter_umh which are related in iptables command. The first patch adds an exit code for UMH process. This provides an opportunity to cleanup members of the umh_info to modules which use the UMH. In order to identify UMH processes, a new flag PF_UMH is added. The second patch makes the bpfilter_umh use UMH cleanup callback. The third patch adds re-start routine for the bpfilter_umh. The bpfilter_umh does not re-start after error occurred. because there is no re-start routine in the module. The fourth patch ensures that the bpfilter.ko module will not removed while it's being used. The bpfilter.ko is not protected by locks or module reference counter. Therefore that can be removed while module is being used. In order to protect that, mutex is used. The first and second patch are preparation patches for the third and fourth patch. TEST #1 while : do modprobe bpfilter kill -9 iptables -vnL done TEST #2 while : do iptables -I FORWARD -m string --string ap --algo kmp & iptables -F & modprobe -rv bpfilter & done The TEST1 makes a failure of iptables command. This is fixed by the third patch. The TEST2 makes a panic because of a race condition in the bpfilter_umh module. This is fixed by the fourth patch. v3 : - Avoid unnecessary list lookup for non-UMH processes - Add a new PF_UMH flag v2 : add the first and second patch v1 : Initial patch Taehee Yoo (4): umh: add exit routine for UMH process net: bpfilter: use cleanup callback to release umh_info net: bpfilter: restart bpfilter_umh when error occurred net: bpfilter: disallow to remove bpfilter module while being used include/linux/bpfilter.h | 15 +-- include/linux/sched.h| 1 + include/linux/umh.h | 4 ++ kernel/exit.c| 1 + kernel/umh.c | 36 +++- net/bpfilter/bpfilter_kern.c | 72 +--- net/bpfilter/bpfilter_umh_blob.S | 2 +- net/ipv4/bpfilter/sockopt.c | 59 +- 8 files changed, 140 insertions(+), 50 deletions(-) -- 2.17.1
Re: [PATCH net 1/4] umh: add exit routine for UMH process
On Mon, 7 Jan 2019 at 01:55, David Miller wrote: > > From: Taehee Yoo > Date: Sun, 6 Jan 2019 14:34:52 +0900 > > > How about adding a new PF_UMH flag for task_struct->flags to identify > > UMH process? > > By using this flag, the exit_umh() can avoid unnecessary lookups. > > Yes, that might be more efficient and eliminate the high cost for > non-UMH tasks. I will send a v3 patch. Thank you!
Re: [PATCH net 1/4] umh: add exit routine for UMH process
On Sun, 6 Jan 2019 at 07:10, David Miller wrote:
>
> From: Taehee Yoo
> Date: Mon, 31 Dec 2018 01:31:43 +0900
>
> > +void exit_umh(struct task_struct *tsk)
> > +{
> > + struct umh_info *info;
> > + pid_t pid = tsk->pid;
> > +
> > + mutex_lock(_list_lock);
> > + list_for_each_entry(info, _list, list) {
>
Thank you for review!
> So this is probably too expensive of a cost for every process exit.
> The problem is that the cost will be taken even if the process is
> not a UMH.
>
Yes, I agree with you.
> I've taken my time to respond in hopes that I could come up with a
> good alternative to suggest, but so far I don't have any better ideas.
>
> I'll keep thinking about this some more, please let me know if you
> have any ideas.
Thanks a lot for spending time to think about better ideas!
How about adding a new PF_UMH flag for task_struct->flags to identify
UMH process?
By using this flag, the exit_umh() can avoid unnecessary lookups.
Thanks again.
Re: [PATCH net 0/4] net: bpfilter: fix two bugs in bpfilter
On Sat, 5 Jan 2019 at 05:54, David Miller wrote: > > From: Taehee Yoo > Date: Mon, 31 Dec 2018 01:30:45 +0900 > > > This patches fix two bugs in the bpfilter_umh which are related in > > iptables command. > ... > > I am still thinking about these patches, sorry for taking so long to > give a response. > > Thank you. Thank you for letting me know!
[PATCH net 2/4] net: bpfilter: use cleanup callback to release umh_info
Now, UMH process is killed, do_exit() calls the umh_info->cleanup callback
to release members of the umh_info.
This patch makes bpfilter_umh's cleanup routine to use the
umh_info->cleanup callback.
Signed-off-by: Taehee Yoo
---
include/linux/bpfilter.h | 11 ---
net/bpfilter/bpfilter_kern.c | 23 ++-
net/ipv4/bpfilter/sockopt.c | 33 ++---
3 files changed, 44 insertions(+), 23 deletions(-)
diff --git a/include/linux/bpfilter.h b/include/linux/bpfilter.h
index f02cee0225d4..70ffeed280e9 100644
--- a/include/linux/bpfilter.h
+++ b/include/linux/bpfilter.h
@@ -3,13 +3,18 @@
#define _LINUX_BPFILTER_H
#include
+#include
struct sock;
int bpfilter_ip_set_sockopt(struct sock *sk, int optname, char __user *optval,
unsigned int optlen);
int bpfilter_ip_get_sockopt(struct sock *sk, int optname, char __user *optval,
int __user *optlen);
-extern int (*bpfilter_process_sockopt)(struct sock *sk, int optname,
- char __user *optval,
- unsigned int optlen, bool is_set);
+struct bpfilter_umh_ops {
+ struct umh_info info;
+ int (*sockopt)(struct sock *sk, int optname,
+ char __user *optval,
+ unsigned int optlen, bool is_set);
+};
+extern struct bpfilter_umh_ops bpfilter_ops;
#endif
diff --git a/net/bpfilter/bpfilter_kern.c b/net/bpfilter/bpfilter_kern.c
index 7acfc83087d5..a68940b74c01 100644
--- a/net/bpfilter/bpfilter_kern.c
+++ b/net/bpfilter/bpfilter_kern.c
@@ -13,7 +13,6 @@
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);
@@ -28,16 +27,13 @@ static void shutdown_umh(struct umh_info *info)
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();
+ bpfilter_ops.sockopt = NULL;
+ shutdown_umh(_ops.info);
}
}
@@ -64,9 +60,10 @@ static int __bpfilter_process_sockopt(struct sock *sk, int
optname,
req.addr = (long __force __user)optval;
req.len = optlen;
mutex_lock(_lock);
- if (!info.pid)
+ if (!bpfilter_ops.info.pid)
goto out;
- n = __kernel_write(info.pipe_to_umh, , sizeof(req), );
+ n = __kernel_write(bpfilter_ops.info.pipe_to_umh, , sizeof(req),
+ );
if (n != sizeof(req)) {
pr_err("write fail %zd\n", n);
__stop_umh();
@@ -74,7 +71,8 @@ static int __bpfilter_process_sockopt(struct sock *sk, int
optname,
goto out;
}
pos = 0;
- n = kernel_read(info.pipe_from_umh, , sizeof(reply), );
+ n = kernel_read(bpfilter_ops.info.pipe_from_umh, , sizeof(reply),
+ );
if (n != sizeof(reply)) {
pr_err("read fail %zd\n", n);
__stop_umh();
@@ -92,13 +90,12 @@ static int __init load_umh(void)
int err;
/* fork usermode process */
- info.cmdline = "bpfilter_umh";
err = fork_usermode_blob(_umh_start,
_umh_end - _umh_start,
-);
+_ops.info);
if (err)
return err;
- pr_info("Loaded bpfilter_umh pid %d\n", info.pid);
+ pr_info("Loaded bpfilter_umh pid %d\n", bpfilter_ops.info.pid);
/* health check that usermode process started correctly */
if (__bpfilter_process_sockopt(NULL, 0, NULL, 0, 0) != 0) {
@@ -106,7 +103,7 @@ static int __init load_umh(void)
return -EFAULT;
}
if (IS_ENABLED(CONFIG_INET))
- bpfilter_process_sockopt = &__bpfilter_process_sockopt;
+ bpfilter_ops.sockopt = &__bpfilter_process_sockopt;
return 0;
}
diff --git a/net/ipv4/bpfilter/sockopt.c b/net/ipv4/bpfilter/sockopt.c
index 5e04ed25bc0e..c326cfbc0f62 100644
--- a/net/ipv4/bpfilter/sockopt.c
+++ b/net/ipv4/bpfilter/sockopt.c
@@ -1,28 +1,37 @@
// SPDX-License-Identifier: GPL-2.0
+#include
+#include
#include
#include
#include
#include
#include
+#include
+#include
-int (*bpfilter_process_sockopt)(struct sock *sk, int optname,
- char __user *optval,
- unsigned int optlen, bool is_set);
-EXPORT_SYMBOL_GPL(bpfilter_process_sockopt);
+struct bpfilter_umh_ops bpfilter_ops;
+EXPORT_SYMBOL_GPL(bpfilter_ops);
+
+static void bpfi
[PATCH net 4/4] 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 fbfff807440b
[ 298.628512] #PF error: [normal kernel read fault]
[ 298.633018] PGD 124327067 P4D 124327067 PUD 11c1a3067 PMD 119eb2067 PTE 0
[ 298.638859] Oops: [#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:88810ea0 EFLAGS: 00010202
[ 298.638859] RAX: 1807440b RBX: 888111bd4d80 RCX:
[ 298.638859] RDX: 1110235ff806 RSI: 888111bd5538 RDI: dc00
[ 298.638859] RBP: 88810e777b30 R08: 8002 R09:
[ 298.638859] R10: R11: R12: fbfff168a42c
[ 298.638859] R13: 888111bd4d80 R14: 8881040e9a05 R15: c03a2000
[ 298.638859] FS: 7f39e3758700() GS:88811ae0()
knlGS:
[ 298.638859] CS: 0010 DS: ES: CR0: 80050033
[ 298.638859] CR2: fbfff807440b CR3: 00011243e000 CR4: 001006f0
[ 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: d2ba09c17a06 ("net: add skeleton of bpfilter kernel module")
Signed-off-by: Taehee Yoo
---
include/linux/bpfilter.h | 2 ++
net/bpfilter/bpfilter_kern.c | 20 ++--
net/ipv4/bpfilter/sockopt.c | 21 -
3 files changed, 24 insertions(+), 19 deletions(-)
diff --git a/include/linux/bpfilter.h b/include/linux/bpfilter.h
index 8ebcbdd70bdc..d815622cd31e 100644
--- a/include/linux/bpfilter.h
+++ b/include/linux/bpfilter.h
@@ -12,6 +12,8 @@ int bpfilter_ip_get_sockopt(struct sock *sk, int optname,
char __user *optval,
int __user *optlen);
struct bpfilter_umh_ops {
struct umh_info info;
+ /* since ip_getsockopt() can run in parallel, serialize access to umh */
+ struct mutex lock;
int (*sockopt)(struct sock *sk, int optname,
char __user *optval,
unsigned int optlen, bool is_set);
diff --git a/net/bpfilter/bpfilter_kern.c b/net/bpfilter/bpfilter_kern.c
index 33d6b159ba88..eedb83863cb0 100644
--- a/net/bpfilter/bpfilter_kern.c
+++ b/net/bpfilter/bpfilter_kern.c
@@ -13,9 +13,6 @@
extern char bpfilter_umh_start;
extern char bpfilter_umh_end;
-/* since ip_getsockopt() can run in parallel, serialize access to umh */
-static DEFINE_MUTEX(bpfilter_lock);
-
static void shutdown_umh(void)
{
struct task_struct *tsk;
@@ -36,13 +33,6 @@ static void __stop_umh(void)
shutdown_umh();
}
-static void stop_umh(void)
-{
- mutex_lock(_lock);
- __stop_umh();
- mutex_unlock(_lock);
-}
-
static int __bpfilter_process_sockopt(struct sock *sk, int optname,
char __user *optval,
unsigned int optlen, bool is_set)
@@ -58,7 +48,6 @@ static int __bpfilter_process_sockopt(struct sock *sk, int
optname,
req.cmd = optname;
req.addr = (long __force __user)optval;
[PATCH net 3/4] net: bpfilter: restart bpfilter_umh when error occurred
The bpfilter_umh will be stopped via __stop_umh() when the bpfilter
error occurred.
The bpfilter_umh() couldn't start again because there is no restart
routine.
The section of the bpfilter_umh_{start/end} is no longer .init.rodata
because these area should be reused in the restart routine. hence
the section name is changed to .bpfilter_umh.
The bpfilter_ops->start() is restart callback. it will be called when
bpfilter_umh is stopped.
The stop bit means bpfilter_umh is stopped. this bit is set by both
start and stop routine.
Before this patch,
Test commands:
$ iptables -vnL
$ kill -9
$ iptables -vnL
[ 480.045136] bpfilter: write fail -32
$ iptables -vnL
All iptables commands will fail.
After this patch,
Test commands:
$ iptables -vnL
$ kill -9
$ iptables -vnL
$ iptables -vnL
Now, all iptables commands will work.
Fixes: d2ba09c17a06 ("net: add skeleton of bpfilter kernel module")
Signed-off-by: Taehee Yoo
---
include/linux/bpfilter.h | 2 ++
net/bpfilter/bpfilter_kern.c | 35 +++-
net/bpfilter/bpfilter_umh_blob.S | 2 +-
net/ipv4/bpfilter/sockopt.c | 11 +-
4 files changed, 38 insertions(+), 12 deletions(-)
diff --git a/include/linux/bpfilter.h b/include/linux/bpfilter.h
index 70ffeed280e9..8ebcbdd70bdc 100644
--- a/include/linux/bpfilter.h
+++ b/include/linux/bpfilter.h
@@ -15,6 +15,8 @@ struct bpfilter_umh_ops {
int (*sockopt)(struct sock *sk, int optname,
char __user *optval,
unsigned int optlen, bool is_set);
+ int (*start)(void);
+ bool stop;
};
extern struct bpfilter_umh_ops bpfilter_ops;
#endif
diff --git a/net/bpfilter/bpfilter_kern.c b/net/bpfilter/bpfilter_kern.c
index a68940b74c01..33d6b159ba88 100644
--- a/net/bpfilter/bpfilter_kern.c
+++ b/net/bpfilter/bpfilter_kern.c
@@ -16,13 +16,14 @@ extern char bpfilter_umh_end;
/* since ip_getsockopt() can run in parallel, serialize access to umh */
static DEFINE_MUTEX(bpfilter_lock);
-static void shutdown_umh(struct umh_info *info)
+static void shutdown_umh(void)
{
struct task_struct *tsk;
- if (!info->pid)
+ if (bpfilter_ops.stop)
return;
- tsk = get_pid_task(find_vpid(info->pid), PIDTYPE_PID);
+
+ tsk = get_pid_task(find_vpid(bpfilter_ops.info.pid), PIDTYPE_PID);
if (tsk) {
force_sig(SIGKILL, tsk);
put_task_struct(tsk);
@@ -31,10 +32,8 @@ static void shutdown_umh(struct umh_info *info)
static void __stop_umh(void)
{
- if (IS_ENABLED(CONFIG_INET)) {
- bpfilter_ops.sockopt = NULL;
- shutdown_umh(_ops.info);
- }
+ if (IS_ENABLED(CONFIG_INET))
+ shutdown_umh();
}
static void stop_umh(void)
@@ -85,7 +84,7 @@ static int __bpfilter_process_sockopt(struct sock *sk, int
optname,
return ret;
}
-static int __init load_umh(void)
+static int start_umh(void)
{
int err;
@@ -95,6 +94,7 @@ static int __init load_umh(void)
_ops.info);
if (err)
return err;
+ bpfilter_ops.stop = false;
pr_info("Loaded bpfilter_umh pid %d\n", bpfilter_ops.info.pid);
/* health check that usermode process started correctly */
@@ -102,14 +102,29 @@ static int __init load_umh(void)
stop_umh();
return -EFAULT;
}
- if (IS_ENABLED(CONFIG_INET))
- bpfilter_ops.sockopt = &__bpfilter_process_sockopt;
return 0;
}
+static int __init load_umh(void)
+{
+ int err;
+
+ err = start_umh();
+ if (!err && IS_ENABLED(CONFIG_INET)) {
+ bpfilter_ops.sockopt = &__bpfilter_process_sockopt;
+ bpfilter_ops.start = _umh;
+ }
+
+ return err;
+}
+
static void __exit fini_umh(void)
{
+ if (IS_ENABLED(CONFIG_INET)) {
+ bpfilter_ops.start = NULL;
+ bpfilter_ops.sockopt = NULL;
+ }
stop_umh();
}
module_init(load_umh);
diff --git a/net/bpfilter/bpfilter_umh_blob.S b/net/bpfilter/bpfilter_umh_blob.S
index 40311d10d2f2..7f1c521dcc2f 100644
--- a/net/bpfilter/bpfilter_umh_blob.S
+++ b/net/bpfilter/bpfilter_umh_blob.S
@@ -1,5 +1,5 @@
/* SPDX-License-Identifier: GPL-2.0 */
- .section .init.rodata, "a"
+ .section .bpfilter_umh, "a"
.global bpfilter_umh_start
bpfilter_umh_start:
.incbin "net/bpfilter/bpfilter_umh"
diff --git a/net/ipv4/bpfilter/sockopt.c b/net/ipv4/bpfilter/sockopt.c
index c326cfbc0f62..de84ede4e765 100644
--- a/net/ipv4/bpfilter/sockopt.c
+++ b/net/ipv4/bpfilter/sockopt.c
@@ -14,6 +14,7 @@ EXPORT_SYMBOL_GPL(bpfilter_ops);
static void bpfilter_umh_cleanup(struct umh_info *info)
{
+ bpfilter_ops.stop = true;
fput(info->pipe_to_umh);
fput(info->pipe_from_umh);
info-&g
[PATCH net 1/4] umh: add exit routine for UMH process
A UMH process which is created by the fork_usermode_blob() such as
bpfilter needs to release members of the umh_info when process is
terminated.
But the do_exit() does not release members of the umh_info. hence module
which uses UMH needs own code to detect whether UMH process is
terminated or not.
But this implementation needs extra code for checking the status of
UMH process. it eventually makes the code more complex.
The exit_umh() does not release members of the umh_info.
Hence umh_info->cleanup callback should release both members of the
umh_info and the private data.
Suggested-by: David S. Miller
Signed-off-by: Taehee Yoo
---
include/linux/umh.h | 4
kernel/exit.c | 1 +
kernel/umh.c| 27 +++
3 files changed, 32 insertions(+)
diff --git a/include/linux/umh.h b/include/linux/umh.h
index 235f51b62c71..c645f0a19103 100644
--- a/include/linux/umh.h
+++ b/include/linux/umh.h
@@ -47,6 +47,8 @@ struct umh_info {
const char *cmdline;
struct file *pipe_to_umh;
struct file *pipe_from_umh;
+ struct list_head list;
+ void (*cleanup)(struct umh_info *info);
pid_t pid;
};
int fork_usermode_blob(void *data, size_t len, struct umh_info *info);
@@ -75,6 +77,8 @@ static inline void usermodehelper_enable(void)
__usermodehelper_set_disable_depth(UMH_ENABLED);
}
+void exit_umh(struct task_struct *tsk);
+
extern int usermodehelper_read_trylock(void);
extern long usermodehelper_read_lock_wait(long timeout);
extern void usermodehelper_read_unlock(void);
diff --git a/kernel/exit.c b/kernel/exit.c
index 0e21e6d21f35..63ce4c958390 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -866,6 +866,7 @@ void __noreturn do_exit(long code)
exit_task_namespaces(tsk);
exit_task_work(tsk);
exit_thread(tsk);
+ exit_umh(tsk);
/*
* Flush inherited counters to the parent - before the parent
diff --git a/kernel/umh.c b/kernel/umh.c
index 0baa672e023c..9b2238e440eb 100644
--- a/kernel/umh.c
+++ b/kernel/umh.c
@@ -37,6 +37,8 @@ static kernel_cap_t usermodehelper_bset = CAP_FULL_SET;
static kernel_cap_t usermodehelper_inheritable = CAP_FULL_SET;
static DEFINE_SPINLOCK(umh_sysctl_lock);
static DECLARE_RWSEM(umhelper_sem);
+static LIST_HEAD(umh_list);
+static DEFINE_MUTEX(umh_list_lock);
static void call_usermodehelper_freeinfo(struct subprocess_info *info)
{
@@ -517,6 +519,11 @@ int fork_usermode_blob(void *data, size_t len, struct
umh_info *info)
goto out;
err = call_usermodehelper_exec(sub_info, UMH_WAIT_EXEC);
+ if (!err) {
+ mutex_lock(_list_lock);
+ list_add(>list, _list);
+ mutex_unlock(_list_lock);
+ }
out:
fput(file);
return err;
@@ -679,6 +686,26 @@ static int proc_cap_handler(struct ctl_table *table, int
write,
return 0;
}
+void exit_umh(struct task_struct *tsk)
+{
+ struct umh_info *info;
+ pid_t pid = tsk->pid;
+
+ mutex_lock(_list_lock);
+ list_for_each_entry(info, _list, list) {
+ if (info->pid == pid) {
+ list_del(>list);
+ mutex_unlock(_list_lock);
+ goto out;
+ }
+ }
+ mutex_unlock(_list_lock);
+ return;
+out:
+ if (info->cleanup)
+ info->cleanup(info);
+}
+
struct ctl_table usermodehelper_table[] = {
{
.procname = "bset",
--
2.17.1
[PATCH net 0/4] net: bpfilter: fix two bugs in bpfilter
This patches fix two bugs in the bpfilter_umh which are related in iptables command. The first patch adds an exit code for UMH process. This provides an opportunity to cleanup members of the umh_info to modules which use the UMH. The second patch makes the bpfilter_umh use UMH cleanup callback. The third patch adds re-start routine for the bpfilter_umh. The bpfilter_umh does not re-start after error occurred. because there is no re-start routine in the module. The fourth patch ensures that the bpfilter.ko module will not removed while it's being used. The bpfilter.ko is not protected by locks or module reference counter. Therefore that can be removed while module is being used. In order to protect that, mutex is used. The first and second patch are preparation patches for the third and fourth patch. TEST #1 while : do modprobe bpfilter kill -9 iptables -vnL done TEST #2 while : do iptables -I FORWARD -m string --string ap --algo kmp & iptables -F & modprobe -rv bpfilter & done The TEST1 makes a failure of iptables command. This is fixed by the third patch. The TEST2 makes a panic because of a race condition in the bpfilter_umh module. This is fixed by the fourth patch. Taehee Yoo (4): umh: add exit routine for UMH process net: bpfilter: use cleanup callback to release umh_info net: bpfilter: restart bpfilter_umh when error occurred net: bpfilter: disallow to remove bpfilter module while being used include/linux/bpfilter.h | 15 +-- include/linux/umh.h | 4 ++ kernel/exit.c| 1 + kernel/umh.c | 27 net/bpfilter/bpfilter_kern.c | 72 +--- net/bpfilter/bpfilter_umh_blob.S | 2 +- net/ipv4/bpfilter/sockopt.c | 59 +- 7 files changed, 132 insertions(+), 48 deletions(-) -- 2.17.1
