Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

linux: Backport fixes for bpfilter and usermode helper to prevent iptable failures with "no child processes" #1495

Open
wants to merge 2 commits into
base: 3.0
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
From 5b4cb650e569db2e6a09d2fa0ef8eb789a0ac5d8 Mon Sep 17 00:00:00 2001
From: Taehee Yoo <[email protected]>
Date: Wed, 9 Jan 2019 02:24:34 +0900
Subject: [PATCH] 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 <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
---
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 <uapi/linux/bpfilter.h>
+#include <linux/umh.h>

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)
send_sig(SIGKILL, tsk, 1);
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);
+ bpfilter_ops.sockopt = NULL;
+ shutdown_umh(&bpfilter_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(&bpfilter_lock);
- if (!info.pid)
+ if (!bpfilter_ops.info.pid)
goto out;
- n = __kernel_write(info.pipe_to_umh, &req, sizeof(req), &pos);
+ n = __kernel_write(bpfilter_ops.info.pipe_to_umh, &req, sizeof(req),
+ &pos);
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, &reply, sizeof(reply), &pos);
+ n = kernel_read(bpfilter_ops.info.pipe_from_umh, &reply, sizeof(reply),
+ &pos);
if (n != sizeof(reply)) {
pr_err("read fail %zd\n", n);
__stop_umh();
@@ -94,10 +94,10 @@ static int __init load_umh(void)
/* fork usermode process */
err = fork_usermode_blob(&bpfilter_umh_start,
&bpfilter_umh_end - &bpfilter_umh_start,
- &info);
+ &bpfilter_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 <linux/init.h>
+#include <linux/module.h>
#include <linux/uaccess.h>
#include <linux/bpfilter.h>
#include <uapi/linux/bpf.h>
#include <linux/wait.h>
#include <linux/kmod.h>
+#include <linux/fs.h>
+#include <linux/file.h>

-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 bpfilter_umh_cleanup(struct umh_info *info)
+{
+ fput(info->pipe_to_umh);
+ fput(info->pipe_from_umh);
+ info->pid = 0;
+}

static int bpfilter_mbox_request(struct sock *sk, int optname,
char __user *optval,
unsigned int optlen, bool is_set)
{
- if (!bpfilter_process_sockopt) {
+ if (!bpfilter_ops.sockopt) {
int err = request_module("bpfilter");

if (err)
return err;
- if (!bpfilter_process_sockopt)
+ if (!bpfilter_ops.sockopt)
return -ECHILD;
}
- return bpfilter_process_sockopt(sk, optname, optval, optlen, is_set);
+ return bpfilter_ops.sockopt(sk, optname, optval, optlen, is_set);
}

int bpfilter_ip_set_sockopt(struct sock *sk, int optname, char __user *optval,
@@ -41,3 +50,13 @@ int bpfilter_ip_get_sockopt(struct sock *sk, int optname, char __user *optval,

return bpfilter_mbox_request(sk, optname, optval, len, false);
}
+
+static int __init bpfilter_sockopt_init(void)
+{
+ bpfilter_ops.info.cmdline = "bpfilter_umh";
+ bpfilter_ops.info.cleanup = &bpfilter_umh_cleanup;
+
+ return 0;
+}
+
+module_init(bpfilter_sockopt_init);
--
2.41.0
93 changes: 93 additions & 0 deletions SPECS/linux/0001-umh-Add-command-line-to-user-mode-helpers.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
From 876dcf2f3aaa0f68d437b368b93a4c4b81521191 Mon Sep 17 00:00:00 2001
From: Olivier Brunel <[email protected]>
Date: Sat, 20 Oct 2018 19:39:56 +0200
Subject: [PATCH] umh: Add command line to user mode helpers

User mode helpers were spawned without a command line, and because
an empty command line is used by many tools to identify processes as
kernel threads, this could cause some issues.

Notably during killing spree on shutdown, since such helper would then
be skipped (i.e. not killed) which would result in the process remaining
alive, and thus preventing unmouting of the rootfs (as experienced with
the bpfilter umh).

Fixes: 449325b52b7a ("umh: introduce fork_usermode_blob() helper")
Signed-off-by: Olivier Brunel <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
---
include/linux/umh.h | 1 +
kernel/umh.c | 16 ++++++++++++++--
2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/include/linux/umh.h b/include/linux/umh.h
index 5c812acbb80a..235f51b62c71 100644
--- a/include/linux/umh.h
+++ b/include/linux/umh.h
@@ -44,6 +44,7 @@ struct subprocess_info *call_usermodehelper_setup_file(struct file *file,
int (*init)(struct subprocess_info *info, struct cred *new),
void (*cleanup)(struct subprocess_info *), void *data);
struct umh_info {
+ const char *cmdline;
struct file *pipe_to_umh;
struct file *pipe_from_umh;
pid_t pid;
diff --git a/kernel/umh.c b/kernel/umh.c
index c449858946af..0baa672e023c 100644
--- a/kernel/umh.c
+++ b/kernel/umh.c
@@ -405,11 +405,19 @@ struct subprocess_info *call_usermodehelper_setup_file(struct file *file,
void (*cleanup)(struct subprocess_info *info), void *data)
{
struct subprocess_info *sub_info;
+ struct umh_info *info = data;
+ const char *cmdline = (info->cmdline) ? info->cmdline : "usermodehelper";

sub_info = kzalloc(sizeof(struct subprocess_info), GFP_KERNEL);
if (!sub_info)
return NULL;

+ sub_info->argv = argv_split(GFP_KERNEL, cmdline, NULL);
+ if (!sub_info->argv) {
+ kfree(sub_info);
+ return NULL;
+ }
+
INIT_WORK(&sub_info->work, call_usermodehelper_exec_work);
sub_info->path = "none";
sub_info->file = file;
@@ -458,10 +466,11 @@ static int umh_pipe_setup(struct subprocess_info *info, struct cred *new)
return 0;
}

-static void umh_save_pid(struct subprocess_info *info)
+static void umh_clean_and_save_pid(struct subprocess_info *info)
{
struct umh_info *umh_info = info->data;

+ argv_free(info->argv);
umh_info->pid = info->pid;
}

@@ -471,6 +480,9 @@ static void umh_save_pid(struct subprocess_info *info)
* @len: length of the blob
* @info: information about usermode process (shouldn't be NULL)
*
+ * If info->cmdline is set it will be used as command line for the
+ * user process, else "usermodehelper" is used.
+ *
* Returns either negative error or zero which indicates success
* in executing a blob of bytes as a usermode process. In such
* case 'struct umh_info *info' is populated with two pipes
@@ -500,7 +512,7 @@ int fork_usermode_blob(void *data, size_t len, struct umh_info *info)

err = -ENOMEM;
sub_info = call_usermodehelper_setup_file(file, umh_pipe_setup,
- umh_save_pid, info);
+ umh_clean_and_save_pid, info);
if (!sub_info)
goto out;

--
2.41.0

Loading