umd: Track user space drivers with struct pid

Use struct pid instead of user space pid values that are prone to wrap
araound.

In addition track the entire thread group instead of just the first
thread that is started by exec.  There are no multi-threaded user mode
drivers today but there is nothing preclucing user drivers from being
multi-threaded, so it is just a good idea to track the entire process.

Take a reference count on the tgid's in question to make it possible
to remove exit_umh in a future change.

As a struct pid is available directly use kill_pid_info.

The prior process signalling code was iffy in using a userspace pid
known to be in the initial pid namespace and then looking up it's task
in whatever the current pid namespace is.  It worked only because
kernel threads always run in the initial pid namespace.

As the tgid is now refcounted verify the tgid is NULL at the start of
fork_usermode_driver to avoid the possibility of silent pid leaks.

v1: https://lkml.kernel.org/r/87mu4qdlv2.fsf_-_@x220.int.ebiederm.org
v2: https://lkml.kernel.org/r/a70l4oy8.fsf_-_@x220.int.ebiederm.org
Link: https://lkml.kernel.org/r/20200702164140.4468-12-ebiederm@xmission.com
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Tested-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>

diff --git a/include/linux/usermode_driver.h b/include/linux/usermode_driver.h
index 97c919b7147cccb5aa01da86a98ce06728649a2f..45adbffb31d9cb9f974204588a49fe1b4b92b8c8 100644 (file)
--- a/include/linux/usermode_driver.h
+++ b/include/linux/usermode_driver.h
@@ -25,7 +25,7 @@ struct umd_info {
        struct list_head list;
        void (*cleanup)(struct umd_info *info);
        struct path wd;
-       pid_t pid;
+       struct pid *tgid;
 };
 int umd_load_blob(struct umd_info *info, const void *data, size_t len);
 int umd_unload_blob(struct umd_info *info);

diff --git a/kernel/exit.c b/kernel/exit.c
index a081deea52ca300927b26fdb972b336e019008aa..d3294b611df1841e43b5f7e328259d15e8430af7 100644 (file)
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -805,7 +805,8 @@ void __noreturn do_exit(long code)
        exit_task_namespaces(tsk);
        exit_task_work(tsk);
        exit_thread(tsk);
-       exit_umh(tsk);
+       if (group_dead)
+               exit_umh(tsk);
 
        /*
         * Flush inherited counters to the parent - before the parent

diff --git a/kernel/usermode_driver.c b/kernel/usermode_driver.c
index a86798759f836b77898cfeb3aee2071cafda53b1..f77f8d7ce9e324b499adaaf258442f001e52cfdf 100644 (file)
--- a/kernel/usermode_driver.c
+++ b/kernel/usermode_driver.c
@@ -133,7 +133,7 @@ static int umd_setup(struct subprocess_info *info, struct cred *new)
        set_fs_pwd(current->fs, &umd_info->wd);
        umd_info->pipe_to_umh = to_umh[1];
        umd_info->pipe_from_umh = from_umh[0];
-       umd_info->pid = task_pid_nr(current);
+       umd_info->tgid = get_pid(task_tgid(current));
        current->flags |= PF_UMH;
        return 0;
 }
@@ -146,6 +146,8 @@ static void umd_cleanup(struct subprocess_info *info)
        if (info->retval) {
                fput(umd_info->pipe_to_umh);
                fput(umd_info->pipe_from_umh);
+               put_pid(umd_info->tgid);
+               umd_info->tgid = NULL;
        }
 }
 
@@ -155,9 +157,9 @@ static void umd_cleanup(struct subprocess_info *info)
  *
  * Returns either negative error or zero which indicates success in
  * executing a usermode driver. In such case 'struct umd_info *info'
- * is populated with two pipes and a pid of the process. The caller is
+ * is populated with two pipes and a tgid of the process. The caller is
  * responsible for health check of the user process, killing it via
- * pid, and closing the pipes when user process is no longer needed.
+ * tgid, and closing the pipes when user process is no longer needed.
  */
 int fork_usermode_driver(struct umd_info *info)
 {
@@ -165,6 +167,9 @@ int fork_usermode_driver(struct umd_info *info)
        char **argv = NULL;
        int err;
 
+       if (WARN_ON_ONCE(info->tgid))
+               return -EBUSY;
+
        err = -ENOMEM;
        argv = argv_split(GFP_KERNEL, info->driver_name, NULL);
        if (!argv)
@@ -192,11 +197,11 @@ EXPORT_SYMBOL_GPL(fork_usermode_driver);
 void __exit_umh(struct task_struct *tsk)
 {
        struct umd_info *info;
-       pid_t pid = tsk->pid;
+       struct pid *tgid = task_tgid(tsk);
 
        mutex_lock(&umh_list_lock);
        list_for_each_entry(info, &umh_list, list) {
-               if (info->pid == pid) {
+               if (info->tgid == tgid) {
                        list_del(&info->list);
                        mutex_unlock(&umh_list_lock);
                        goto out;

diff --git a/net/bpfilter/bpfilter_kern.c b/net/bpfilter/bpfilter_kern.c
index 28883b00609db0589c2bb69fb616083c144dfc5a..08ea77c2b1370c209ee6a8049b13207366633872 100644 (file)
--- a/net/bpfilter/bpfilter_kern.c
+++ b/net/bpfilter/bpfilter_kern.c
@@ -15,16 +15,13 @@ extern char bpfilter_umh_end;
 
 static void shutdown_umh(void)
 {
-       struct task_struct *tsk;
+       struct umd_info *info = &bpfilter_ops.info;
+       struct pid *tgid = info->tgid;
 
        if (bpfilter_ops.stop)
                return;
 
-       tsk = get_pid_task(find_vpid(bpfilter_ops.info.pid), PIDTYPE_PID);
-       if (tsk) {
-               send_sig(SIGKILL, tsk, 1);
-               put_task_struct(tsk);
-       }
+       kill_pid(tgid, SIGKILL, 1);
 }
 
 static void __stop_umh(void)
@@ -48,7 +45,7 @@ static int __bpfilter_process_sockopt(struct sock *sk, int optname,
        req.cmd = optname;
        req.addr = (long __force __user)optval;
        req.len = optlen;
-       if (!bpfilter_ops.info.pid)
+       if (!bpfilter_ops.info.tgid)
                goto out;
        n = __kernel_write(bpfilter_ops.info.pipe_to_umh, &req, sizeof(req),
                           &pos);
@@ -81,7 +78,7 @@ static int start_umh(void)
        if (err)
                return err;
        bpfilter_ops.stop = false;
-       pr_info("Loaded bpfilter_umh pid %d\n", bpfilter_ops.info.pid);
+       pr_info("Loaded bpfilter_umh pid %d\n", pid_nr(bpfilter_ops.info.tgid));
 
        /* health check that usermode process started correctly */
        if (__bpfilter_process_sockopt(NULL, 0, NULL, 0, 0) != 0) {

diff --git a/net/ipv4/bpfilter/sockopt.c b/net/ipv4/bpfilter/sockopt.c
index 5050de28333d1fc0354913a3ed5b8dfa247614ca..56cbc43145f6d915e77aa168ae51cc7cc740e158 100644 (file)
--- a/net/ipv4/bpfilter/sockopt.c
+++ b/net/ipv4/bpfilter/sockopt.c
@@ -18,7 +18,8 @@ static void bpfilter_umh_cleanup(struct umd_info *info)
        bpfilter_ops.stop = true;
        fput(info->pipe_to_umh);
        fput(info->pipe_from_umh);
-       info->pid = 0;
+       put_pid(info->tgid);
+       info->tgid = NULL;
        mutex_unlock(&bpfilter_ops.lock);
 }