From: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
To: "Eric W. Biederman" <ebiederm@xmission.com>,
Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: linux-kernel@vger.kernel.org, David Miller <davem@davemloft.net>,
Greg Kroah-Hartman <greg@kroah.com>,
Kees Cook <keescook@chromium.org>,
Andrew Morton <akpm@linux-foundation.org>,
Alexei Starovoitov <ast@kernel.org>,
Al Viro <viro@zeniv.linux.org.uk>, bpf <bpf@vger.kernel.org>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Jakub Kicinski <kuba@kernel.org>,
Masahiro Yamada <yamada.masahiro@socionext.com>,
Gary Lin <GLin@suse.com>, Bruno Meneguele <bmeneg@redhat.com>,
LSM List <linux-security-module@vger.kernel.org>,
Casey Schaufler <casey@schaufler-ca.com>,
Luis Chamberlain <mcgrof@kernel.org>,
Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH v2 00/15] Make the user mode driver code a better citizen
Date: Tue, 30 Jun 2020 15:16:57 +0900 [thread overview]
Message-ID: <8137153b-6163-b52c-a1f8-f3949dc321bd@i-love.sakura.ne.jp> (raw)
In-Reply-To: <87y2o51hkk.fsf@x220.int.ebiederm.org>
On 2020/06/30 10:13, Eric W. Biederman wrote:
> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>
>> On Mon, Jun 29, 2020 at 02:55:05PM -0500, Eric W. Biederman wrote:
>>>
>>> I have tested thes changes by booting with the code compiled in and
>>> by killing "bpfilter_umh" and running iptables -vnL to restart
>>> the userspace driver.
>>>
>>> I have compiled tested each change with and without CONFIG_BPFILTER
>>> enabled.
>>
>> With
>> CONFIG_BPFILTER=y
>> CONFIG_BPFILTER_UMH=m
>> it doesn't build:
>>
>> ERROR: modpost: "kill_pid_info" [net/bpfilter/bpfilter.ko] undefined!
>>
>> I've added:
>> +EXPORT_SYMBOL(kill_pid_info);
>> to continue testing...
kill_pid() is already exported.
>>
>> And then did:
>> while true; do iptables -L;rmmod bpfilter; done
>>
>> Unfortunately sometimes 'rmmod bpfilter' hangs in wait_event().
>>
>> I suspect patch 13 is somehow responsible:
>> + if (tgid) {
>> + kill_pid_info(SIGKILL, SEND_SIG_PRIV, tgid);
>> + wait_event(tgid->wait_pidfd, !pid_task(tgid, PIDTYPE_TGID));
>> + bpfilter_umh_cleanup(info);
>> + }
>>
>> I cannot figure out why it hangs. Some sort of race ?
>> Since adding short delay between kill and wait makes it work.
Because there is a race window that detach_pid() from __unhash_process() from
__exit_signal() from release_task() from exit_notify() from do_exit() is called
some time after wake_up_all(&pid->wait_pidfd) from do_notify_pidfd() from
do_notify_parent() from exit_notify() from do_exit() was called (in other words,
we can't use pid->wait_pidfd when pid_task() is used at wait_event()) ?
Below are changes I suggest.
diff --git a/kernel/umd.c b/kernel/umd.c
index ff79fb16d738..f688813b8830 100644
--- a/kernel/umd.c
+++ b/kernel/umd.c
@@ -26,7 +26,7 @@ static struct vfsmount *blob_to_mnt(const void *data, size_t len, const char *na
if (IS_ERR(mnt))
return mnt;
- file = file_open_root(mnt->mnt_root, mnt, name, O_CREAT | O_WRONLY, 0700);
+ file = file_open_root(mnt->mnt_root, mnt, name, O_CREAT | O_WRONLY | O_EXCL, 0700);
if (IS_ERR(file)) {
mntput(mnt);
return ERR_CAST(file);
@@ -52,16 +52,18 @@ static struct vfsmount *blob_to_mnt(const void *data, size_t len, const char *na
/**
* umd_load_blob - Remember a blob of bytes for fork_usermode_driver
- * @info: information about usermode driver
- * @data: a blob of bytes that can be executed as a file
- * @len: The lentgh of the blob
+ * @info: information about usermode driver (shouldn't be NULL)
+ * @data: a blob of bytes that can be executed as a file (shouldn't be NULL)
+ * @len: The lentgh of the blob (shouldn't be 0)
*
*/
int umd_load_blob(struct umd_info *info, const void *data, size_t len)
{
struct vfsmount *mnt;
- if (WARN_ON_ONCE(info->wd.dentry || info->wd.mnt))
+ if (!info || !info->driver_name || !data || !len)
+ return -EINVAL;
+ if (info->wd.dentry || info->wd.mnt)
return -EBUSY;
mnt = blob_to_mnt(data, len, info->driver_name);
@@ -76,15 +78,14 @@ EXPORT_SYMBOL_GPL(umd_load_blob);
/**
* umd_unload_blob - Disassociate @info from a previously loaded blob
- * @info: information about usermode driver
+ * @info: information about usermode driver (shouldn't be NULL)
*
*/
int umd_unload_blob(struct umd_info *info)
{
- if (WARN_ON_ONCE(!info->wd.mnt ||
- !info->wd.dentry ||
- info->wd.mnt->mnt_root != info->wd.dentry))
+ if (!info || !info->driver_name || !info->wd.dentry || !info->wd.mnt)
return -EINVAL;
+ BUG_ON(info->wd.mnt->mnt_root != info->wd.dentry);
kern_unmount(info->wd.mnt);
info->wd.mnt = NULL;
@@ -138,7 +139,7 @@ static void umd_cleanup(struct subprocess_info *info)
{
struct umd_info *umd_info = info->data;
- /* cleanup if umh_setup() was successful but exec failed */
+ /* cleanup if umd_setup() was successful but exec failed */
if (info->retval) {
fput(umd_info->pipe_to_umh);
fput(umd_info->pipe_from_umh);
@@ -163,7 +164,10 @@ int fork_usermode_driver(struct umd_info *info)
const char *argv[] = { info->driver_name, NULL };
int err;
- if (WARN_ON_ONCE(info->tgid))
+ if (!info || !info->driver_name || !info->wd.dentry || !info->wd.mnt)
+ return -EINVAL;
+ BUG_ON(info->wd.mnt->mnt_root != info->wd.dentry);
+ if (info->tgid)
return -EBUSY;
err = -ENOMEM;
diff --git a/net/bpfilter/bpfilter_kern.c b/net/bpfilter/bpfilter_kern.c
index 91474884ddb7..9dd70aacb81a 100644
--- a/net/bpfilter/bpfilter_kern.c
+++ b/net/bpfilter/bpfilter_kern.c
@@ -19,8 +19,13 @@ static void shutdown_umh(void)
struct pid *tgid = info->tgid;
if (tgid) {
- kill_pid_info(SIGKILL, SEND_SIG_PRIV, tgid);
- wait_event(tgid->wait_pidfd, !pid_task(tgid, PIDTYPE_TGID));
+ kill_pid(tgid, SIGKILL, 1);
+ while (({ bool done;
+ rcu_read_lock();
+ done = !pid_task(tgid, PIDTYPE_TGID);
+ rcu_read_unlock();
+ done; }))
+ schedule_timeout_uninterruptible(1);
bpfilter_umh_cleanup(info);
}
}
next prev parent reply other threads:[~2020-06-30 6:18 UTC|newest]
Thread overview: 193+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20200329005528.xeKtdz2A0%akpm@linux-foundation.org>
[not found] ` <13fb3ab7-9ab1-b25f-52f2-40a6ca5655e1@i-love.sakura.ne.jp>
[not found] ` <202006051903.C44988B@keescook>
2020-06-06 19:20 ` [RFC][PATCH] net/bpfilter: Remove this broken and apparently unmantained Eric W. Biederman
2020-06-06 20:19 ` Alexei Starovoitov
2020-06-06 22:33 ` Linus Torvalds
2020-06-07 1:49 ` Alexei Starovoitov
2020-06-07 2:19 ` Linus Torvalds
2020-06-07 16:09 ` Eric W. Biederman
2020-06-08 16:20 ` Alexei Starovoitov
2020-06-08 16:40 ` Greg KH
2020-06-08 18:35 ` Kees Cook
2020-06-09 1:26 ` Alexei Starovoitov
2020-06-09 15:37 ` Kees Cook
2020-06-09 19:51 ` Eric W. Biederman
2020-06-07 2:31 ` Tetsuo Handa
2020-06-08 16:23 ` Alexei Starovoitov
2020-06-08 23:22 ` Tetsuo Handa
2020-06-09 1:28 ` Alexei Starovoitov
2020-06-09 5:29 ` Tetsuo Handa
2020-06-09 22:32 ` Alexei Starovoitov
2020-06-09 23:30 ` Tetsuo Handa
2020-06-10 0:05 ` Alexei Starovoitov
2020-06-10 3:08 ` Tetsuo Handa
2020-06-10 3:32 ` Alexei Starovoitov
2020-06-10 7:30 ` Tetsuo Handa
2020-06-10 16:24 ` Casey Schaufler
2020-06-09 20:02 ` Eric W. Biederman
2020-06-09 23:56 ` Alexei Starovoitov
2020-06-10 21:12 ` Eric W. Biederman
2020-06-11 23:31 ` Alexei Starovoitov
2020-06-12 0:57 ` Tetsuo Handa
2020-06-13 3:38 ` Alexei Starovoitov
2020-06-13 4:22 ` Tetsuo Handa
2020-06-13 14:08 ` Eric W. Biederman
2020-06-13 15:33 ` Alexei Starovoitov
2020-06-13 16:14 ` Alexei Starovoitov
2020-06-14 14:51 ` Eric W. Biederman
2020-06-16 1:55 ` Alexei Starovoitov
2020-06-16 16:21 ` Alexei Starovoitov
2020-06-23 18:04 ` Eric W. Biederman
2020-06-23 18:35 ` Alexei Starovoitov
2020-06-23 18:53 ` Eric W. Biederman
2020-06-23 19:40 ` Alexei Starovoitov
2020-06-24 1:51 ` Tetsuo Handa
2020-06-24 4:00 ` Alexei Starovoitov
2020-06-24 4:58 ` Tetsuo Handa
2020-06-24 6:39 ` Alexei Starovoitov
2020-06-24 7:05 ` Tetsuo Handa
2020-06-24 15:41 ` Casey Schaufler
2020-06-24 17:54 ` Alexei Starovoitov
2020-06-24 19:48 ` Casey Schaufler
2020-06-24 6:05 ` Alexei Starovoitov
2020-06-24 14:18 ` Alexei Starovoitov
2020-06-24 12:13 ` Eric W. Biederman
2020-06-24 14:26 ` Alexei Starovoitov
2020-06-24 23:14 ` Tetsuo Handa
2020-06-25 1:35 ` Alexei Starovoitov
2020-06-25 6:38 ` Tetsuo Handa
2020-06-25 9:57 ` Greg KH
2020-06-25 11:03 ` Tetsuo Handa
2020-06-25 12:07 ` Greg KH
2020-06-25 14:21 ` Tetsuo Handa
2020-06-25 19:34 ` David Miller
2020-06-26 1:36 ` Linus Torvalds
2020-06-26 1:51 ` Alexei Starovoitov
2020-06-26 4:58 ` Tetsuo Handa
2020-06-26 5:41 ` Alexei Starovoitov
2020-06-26 6:20 ` Tetsuo Handa
2020-06-26 6:39 ` Alexei Starovoitov
2020-06-26 12:51 ` [PATCH 00/14] Make the user mode driver code a better citizen Eric W. Biederman
2020-06-26 12:53 ` [PATCH 01/14] umh: Capture the pid in umh_pipe_setup Eric W. Biederman
2020-06-26 12:53 ` [PATCH 02/14] umh: Move setting PF_UMH into umh_pipe_setup Eric W. Biederman
2020-06-26 12:54 ` [PATCH 03/14] umh: Rename the user mode driver helpers for clarity Eric W. Biederman
2020-06-26 12:54 ` [PATCH 04/14] umh: Remove call_usermodehelper_setup_file Eric W. Biederman
2020-06-26 12:55 ` [PATCH 05/14] umh: Separate the user mode driver and the user mode helper support Eric W. Biederman
2020-06-26 16:22 ` Tetsuo Handa
2020-06-26 16:45 ` Eric W. Biederman
2020-06-27 1:26 ` Tetsuo Handa
2020-06-27 4:21 ` Eric W. Biederman
2020-06-27 4:36 ` Tetsuo Handa
2020-06-26 12:55 ` [PATCH 06/14] umd: For clarity rename umh_info umd_info Eric W. Biederman
2020-06-26 15:37 ` Kees Cook
2020-06-26 16:31 ` Eric W. Biederman
2020-06-26 12:56 ` [PATCH 07/14] umd: Rename umd_info.cmdline umd_info.driver_name Eric W. Biederman
2020-06-26 12:56 ` [PATCH 08/14] umd: Transform fork_usermode_blob into fork_usermode_driver Eric W. Biederman
2020-06-26 12:57 ` [PATCH 09/14] umh: Stop calling do_execve_file Eric W. Biederman
2020-06-26 12:57 ` [PATCH 10/14] exec: Remove do_execve_file Eric W. Biederman
2020-06-26 12:58 ` [PATCH 11/14] bpfilter: Move bpfilter_umh back into init data Eric W. Biederman
2020-06-26 12:58 ` [PATCH 12/14] umd: Track user space drivers with struct pid Eric W. Biederman
2020-06-26 12:59 ` [PATCH 13/14] bpfilter: Take advantage of the facilities of " Eric W. Biederman
2020-06-26 12:59 ` [PATCH 14/14] umd: Remove exit_umh Eric W. Biederman
2020-06-26 13:48 ` [PATCH 00/14] Make the user mode driver code a better citizen Eric W. Biederman
2020-06-29 19:55 ` [PATCH v2 00/15] " Eric W. Biederman
2020-06-29 19:56 ` [PATCH v2 01/15] umh: Capture the pid in umh_pipe_setup Eric W. Biederman
2020-06-29 19:57 ` [PATCH v2 02/15] umh: Move setting PF_UMH into umh_pipe_setup Eric W. Biederman
2020-06-29 19:57 ` [PATCH v2 03/15] umh: Rename the user mode driver helpers for clarity Eric W. Biederman
2020-06-29 19:59 ` [PATCH v2 04/15] umh: Remove call_usermodehelper_setup_file Eric W. Biederman
2020-06-29 20:00 ` [PATCH v2 05/15] umh: Separate the user mode driver and the user mode helper support Eric W. Biederman
2020-06-30 16:58 ` Linus Torvalds
2020-07-01 17:18 ` Eric W. Biederman
2020-07-01 17:42 ` Alexei Starovoitov
2020-06-29 20:01 ` [PATCH v2 06/15] umd: For clarity rename umh_info umd_info Eric W. Biederman
2020-06-29 20:02 ` [PATCH v2 07/15] umd: Rename umd_info.cmdline umd_info.driver_name Eric W. Biederman
2020-06-29 20:03 ` [PATCH v2 08/15] umd: Transform fork_usermode_blob into fork_usermode_driver Eric W. Biederman
2020-06-29 20:03 ` [PATCH v2 09/15] umh: Stop calling do_execve_file Eric W. Biederman
2020-06-29 20:04 ` [PATCH v2 10/15] exec: Remove do_execve_file Eric W. Biederman
2020-06-30 5:43 ` Christoph Hellwig
2020-06-30 12:14 ` Eric W. Biederman
2020-06-30 13:38 ` Christoph Hellwig
2020-06-30 14:28 ` Eric W. Biederman
2020-06-30 16:55 ` Alexei Starovoitov
2020-06-29 20:05 ` [PATCH v2 11/15] bpfilter: Move bpfilter_umh back into init data Eric W. Biederman
2020-06-29 20:06 ` [PATCH v2 12/15] umd: Track user space drivers with struct pid Eric W. Biederman
2020-06-29 20:06 ` [PATCH v2 13/15] bpfilter: Take advantage of the facilities of " Eric W. Biederman
2020-06-29 20:07 ` [PATCH v2 14/15] umd: Remove exit_umh Eric W. Biederman
2020-06-29 20:08 ` [PATCH v2 15/15] umd: Stop using split_argv Eric W. Biederman
2020-06-29 22:12 ` [PATCH v2 00/15] Make the user mode driver code a better citizen Alexei Starovoitov
2020-06-30 1:13 ` Eric W. Biederman
2020-06-30 6:16 ` Tetsuo Handa [this message]
2020-06-30 12:29 ` Eric W. Biederman
2020-06-30 13:21 ` Tetsuo Handa
2020-07-02 13:08 ` Eric W. Biederman
2020-07-02 13:40 ` Tetsuo Handa
2020-07-02 16:02 ` Eric W. Biederman
2020-07-03 13:19 ` Tetsuo Handa
2020-07-03 22:25 ` Eric W. Biederman
2020-07-04 6:57 ` Tetsuo Handa
2020-07-08 4:46 ` Eric W. Biederman
2020-06-30 16:52 ` Alexei Starovoitov
2020-07-01 17:12 ` Eric W. Biederman
2020-07-02 16:40 ` [PATCH v3 00/16] " Eric W. Biederman
2020-07-02 16:41 ` [PATCH v3 01/16] umh: Capture the pid in umh_pipe_setup Eric W. Biederman
2020-07-02 16:41 ` [PATCH v3 02/16] umh: Move setting PF_UMH into umh_pipe_setup Eric W. Biederman
2020-07-02 16:41 ` [PATCH v3 03/16] umh: Rename the user mode driver helpers for clarity Eric W. Biederman
2020-07-02 16:41 ` [PATCH v3 04/16] umh: Remove call_usermodehelper_setup_file Eric W. Biederman
2020-07-02 16:41 ` [PATCH v3 05/16] umh: Separate the user mode driver and the user mode helper support Eric W. Biederman
2020-07-02 16:41 ` [PATCH v3 06/16] umd: For clarity rename umh_info umd_info Eric W. Biederman
2020-07-02 16:41 ` [PATCH v3 07/16] umd: Rename umd_info.cmdline umd_info.driver_name Eric W. Biederman
2020-07-02 16:41 ` [PATCH v3 08/16] umd: Transform fork_usermode_blob into fork_usermode_driver Eric W. Biederman
2020-07-02 16:41 ` [PATCH v3 09/16] umh: Stop calling do_execve_file Eric W. Biederman
2020-07-02 16:41 ` [PATCH v3 10/16] exec: Remove do_execve_file Eric W. Biederman
2020-07-08 6:35 ` Luis Chamberlain
2020-07-08 12:41 ` Luis Chamberlain
2020-07-08 13:08 ` Eric W. Biederman
2020-07-08 13:32 ` Luis Chamberlain
2020-07-12 21:02 ` Pavel Machek
2020-07-02 16:41 ` [PATCH v3 11/16] bpfilter: Move bpfilter_umh back into init data Eric W. Biederman
2020-07-02 16:41 ` [PATCH v3 12/16] umd: Track user space drivers with struct pid Eric W. Biederman
2020-07-02 16:41 ` [PATCH v3 13/16] exit: Factor thread_group_exited out of pidfd_poll Eric W. Biederman
2020-07-03 20:30 ` Alexei Starovoitov
2020-07-03 21:37 ` Eric W. Biederman
2020-07-04 0:03 ` Alexei Starovoitov
2020-07-04 15:50 ` Christian Brauner
2020-07-07 17:09 ` Eric W. Biederman
2020-07-08 0:05 ` Daniel Borkmann
2020-07-08 3:50 ` Eric W. Biederman
2020-07-04 16:00 ` Christian Brauner
2020-07-02 16:41 ` [PATCH v3 14/16] bpfilter: Take advantage of the facilities of struct pid Eric W. Biederman
2020-07-02 16:41 ` [PATCH v3 15/16] umd: Remove exit_umh Eric W. Biederman
2020-07-02 16:41 ` [PATCH v3 16/16] umd: Stop using split_argv Eric W. Biederman
2020-07-02 23:51 ` [PATCH v3 00/16] Make the user mode driver code a better citizen Tetsuo Handa
2020-07-09 22:05 ` [merged][PATCH " Eric W. Biederman
2020-07-14 19:42 ` Alexei Starovoitov
2020-07-08 5:20 ` [PATCH v2 00/15] " Luis Chamberlain
2020-06-26 14:10 ` [PATCH 00/14] " Greg Kroah-Hartman
2020-06-26 16:40 ` Alexei Starovoitov
2020-06-26 17:17 ` Eric W. Biederman
2020-06-26 18:22 ` Alexei Starovoitov
2020-06-27 11:38 ` Tetsuo Handa
2020-06-27 12:59 ` Eric W. Biederman
2020-06-27 13:57 ` Tetsuo Handa
2020-06-28 19:44 ` Alexei Starovoitov
2020-06-29 2:20 ` Tetsuo Handa
2020-06-29 20:19 ` Eric W. Biederman
2020-06-30 6:28 ` Tetsuo Handa
2020-06-30 12:32 ` Eric W. Biederman
2020-06-30 16:48 ` Alexei Starovoitov
2020-06-30 21:54 ` Tetsuo Handa
2020-06-30 21:57 ` Alexei Starovoitov
2020-06-30 22:58 ` Tetsuo Handa
2020-06-25 12:56 ` [RFC][PATCH] net/bpfilter: Remove this broken and apparently unmantained Stephen Smalley
2020-06-25 13:25 ` Greg Kroah-Hartman
2020-06-25 14:26 ` Stephen Smalley
2020-06-25 14:36 ` Stephen Smalley
2020-06-25 15:21 ` Tetsuo Handa
2020-06-25 16:03 ` Stephen Smalley
2020-06-25 16:06 ` Casey Schaufler
2020-06-26 11:30 ` Eric W. Biederman
2020-06-07 5:58 ` Eric W. Biederman
2020-06-07 11:56 ` Eric W. Biederman
2020-06-08 16:35 ` Alexei Starovoitov
2020-06-08 16:33 ` Alexei Starovoitov
2020-06-06 20:43 ` Matthew Wilcox
2020-06-07 15:51 ` Eric W. Biederman
2020-06-07 1:13 ` Tetsuo Handa
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8137153b-6163-b52c-a1f8-f3949dc321bd@i-love.sakura.ne.jp \
--to=penguin-kernel@i-love.sakura.ne.jp \
--cc=GLin@suse.com \
--cc=akpm@linux-foundation.org \
--cc=alexei.starovoitov@gmail.com \
--cc=ast@kernel.org \
--cc=bmeneg@redhat.com \
--cc=bpf@vger.kernel.org \
--cc=casey@schaufler-ca.com \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=ebiederm@xmission.com \
--cc=greg@kroah.com \
--cc=keescook@chromium.org \
--cc=kuba@kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=mcgrof@kernel.org \
--cc=torvalds@linux-foundation.org \
--cc=viro@zeniv.linux.org.uk \
--cc=yamada.masahiro@socionext.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).