From: Tycho Andersen <tycho@tycho.ws>
To: Kees Cook <keescook@chromium.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
Linux Containers <containers@lists.linux-foundation.org>,
Linux API <linux-api@vger.kernel.org>,
Andy Lutomirski <luto@amacapital.net>,
Oleg Nesterov <oleg@redhat.com>,
"Eric W . Biederman" <ebiederm@xmission.com>,
"Serge E . Hallyn" <serge@hallyn.com>,
Christian Brauner <christian.brauner@ubuntu.com>,
Tyler Hicks <tyhicks@canonical.com>,
Akihiro Suda <suda.akihiro@lab.ntt.co.jp>,
Jann Horn <jannh@google.com>,
"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH v7 5/6] seccomp: add a way to pass FDs via a notification fd
Date: Thu, 27 Sep 2018 16:15:40 -0600 [thread overview]
Message-ID: <20180927221540.GE15491@cisco.cisco.com> (raw)
In-Reply-To: <CAGXu5jLs1sOKMktS2MG+rer5n+cy=A8YL0LH5X0MJCdNTv8WuA@mail.gmail.com>
On Thu, Sep 27, 2018 at 03:09:06PM -0700, Kees Cook wrote:
> On Thu, Sep 27, 2018 at 8:11 AM, Tycho Andersen <tycho@tycho.ws> wrote:
> > This patch adds a way to insert FDs into the tracee's process (also
> > close/overwrite fds for the tracee). This functionality is necessary to
> > mock things like socketpair() or dup2() or similar, but since it depends on
> > external (vfs) patches, I've left it as a separate patch as before so the
> > core functionality can still be merged while we argue about this. Except
> > this time it doesn't add any ugliness to the API :)
> >
> > v7: new in v7
> >
> > Signed-off-by: Tycho Andersen <tycho@tycho.ws>
> > CC: Kees Cook <keescook@chromium.org>
> > CC: Andy Lutomirski <luto@amacapital.net>
> > CC: Oleg Nesterov <oleg@redhat.com>
> > CC: Eric W. Biederman <ebiederm@xmission.com>
> > CC: "Serge E. Hallyn" <serge@hallyn.com>
> > CC: Christian Brauner <christian.brauner@ubuntu.com>
> > CC: Tyler Hicks <tyhicks@canonical.com>
> > CC: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>
> > ---
> > .../userspace-api/seccomp_filter.rst | 16 +++
> > include/uapi/linux/seccomp.h | 9 ++
> > kernel/seccomp.c | 54 ++++++++
> > tools/testing/selftests/seccomp/seccomp_bpf.c | 126 ++++++++++++++++++
> > 4 files changed, 205 insertions(+)
> >
> > diff --git a/Documentation/userspace-api/seccomp_filter.rst b/Documentation/userspace-api/seccomp_filter.rst
> > index d2e61f1c0a0b..383a8dbae304 100644
> > --- a/Documentation/userspace-api/seccomp_filter.rst
> > +++ b/Documentation/userspace-api/seccomp_filter.rst
> > @@ -237,6 +237,13 @@ The interface for a seccomp notification fd consists of two structures:
> > __s64 val;
> > };
> >
> > + struct seccomp_notif_put_fd {
> > + __u64 id;
> > + __s32 fd;
> > + __u32 fd_flags;
> > + __s32 to_replace;
> > + };
> > +
> > Users can read via ``ioctl(SECCOMP_NOTIF_RECV)`` (or ``poll()``) on a seccomp
> > notification fd to receive a ``struct seccomp_notif``, which contains five
> > members: the input length of the structure, a unique-per-filter ``id``, the
> > @@ -256,6 +263,15 @@ mentioned above in this document: all arguments being read from the tracee's
> > memory should be read into the tracer's memory before any policy decisions are
> > made. This allows for an atomic decision on syscall arguments.
> >
> > +Userspace can also insert (or overwrite) file descriptors of the tracee using
> > +``ioctl(SECCOMP_NOTIF_PUT_FD)``. The ``id`` member is the request/pid to insert
> > +the fd into. The ``fd`` is the fd in the listener's table to send or ``-1`` if
> > +an fd should be closed instead. The ``to_replace`` fd is the fd in the tracee's
> > +table that should be overwritten, or -1 if a new fd is installed. ``fd_flags``
> > +should be the flags that the fd in the tracee's table is opened with (e.g.
> > +``O_CLOEXEC`` or similar). The return value from this ioctl is the fd number
> > +that was installed.
> > +
> > Sysctls
> > =======
> >
> > diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
> > index d4ccb32fe089..91d77f041fbb 100644
> > --- a/include/uapi/linux/seccomp.h
> > +++ b/include/uapi/linux/seccomp.h
> > @@ -77,6 +77,13 @@ struct seccomp_notif_resp {
> > __s64 val;
> > };
> >
> > +struct seccomp_notif_put_fd {
> > + __u64 id;
> > + __s32 fd;
> > + __u32 fd_flags;
> > + __s32 to_replace;
> > +};
> > +
> > #define SECCOMP_IOC_MAGIC 0xF7
> >
> > /* Flags for seccomp notification fd ioctl. */
> > @@ -86,5 +93,7 @@ struct seccomp_notif_resp {
> > struct seccomp_notif_resp)
> > #define SECCOMP_NOTIF_ID_VALID _IOR(SECCOMP_IOC_MAGIC, 2, \
> > __u64)
> > +#define SECCOMP_NOTIF_PUT_FD _IOR(SECCOMP_IOC_MAGIC, 3, \
> > + struct seccomp_notif_put_fd)
> >
> > #endif /* _UAPI_LINUX_SECCOMP_H */
> > diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> > index 17685803a2af..07a05ad59731 100644
> > --- a/kernel/seccomp.c
> > +++ b/kernel/seccomp.c
> > @@ -41,6 +41,8 @@
> > #include <linux/tracehook.h>
> > #include <linux/uaccess.h>
> > #include <linux/anon_inodes.h>
> > +#include <linux/fdtable.h>
> > +#include <net/cls_cgroup.h>
> >
> > enum notify_state {
> > SECCOMP_NOTIFY_INIT,
> > @@ -1684,6 +1686,56 @@ static long seccomp_notify_id_valid(struct seccomp_filter *filter,
> > return ret;
> > }
> >
> > +static long seccomp_notify_put_fd(struct seccomp_filter *filter,
> > + unsigned long arg)
> > +{
> > + struct seccomp_notif_put_fd req;
> > + void __user *buf = (void __user *)arg;
> > + struct seccomp_knotif *knotif = NULL;
> > + long ret;
> > +
> > + if (copy_from_user(&req, buf, sizeof(req)))
> > + return -EFAULT;
> > +
> > + if (req.fd < 0 && req.to_replace < 0)
> > + return -EINVAL;
> > +
> > + ret = mutex_lock_interruptible(&filter->notify_lock);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = -ENOENT;
> > + list_for_each_entry(knotif, &filter->notif->notifications, list) {
> > + struct file *file = NULL;
> > +
> > + if (knotif->id != req.id)
> > + continue;
> > +
> > + if (req.fd >= 0)
> > + file = fget(req.fd);
>
> Shouldn't we test for !file here?
Yes. Derp.
Tycho
next prev parent reply other threads:[~2018-09-28 4:36 UTC|newest]
Thread overview: 91+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-27 15:11 [PATCH v7 0/6] seccomp trap to userspace Tycho Andersen
2018-09-27 15:11 ` [PATCH v7 1/6] seccomp: add a return code to " Tycho Andersen
2018-09-27 21:31 ` Kees Cook
2018-09-27 22:48 ` Tycho Andersen
2018-09-27 23:10 ` Kees Cook
2018-09-28 14:39 ` Tycho Andersen
2018-10-08 14:58 ` Christian Brauner
2018-10-09 14:28 ` Tycho Andersen
2018-10-09 16:24 ` Christian Brauner
2018-10-09 16:29 ` Tycho Andersen
2018-10-17 20:29 ` Tycho Andersen
2018-10-17 22:21 ` Kees Cook
2018-10-17 22:33 ` Tycho Andersen
2018-10-21 16:04 ` Tycho Andersen
2018-10-22 9:42 ` Christian Brauner
2018-09-27 21:51 ` Jann Horn
2018-09-27 22:45 ` Kees Cook
2018-09-27 23:08 ` Tycho Andersen
2018-09-27 23:04 ` Tycho Andersen
2018-09-27 23:37 ` Jann Horn
2018-09-29 0:28 ` Aleksa Sarai
2018-09-27 15:11 ` [PATCH v7 2/6] seccomp: make get_nth_filter available outside of CHECKPOINT_RESTORE Tycho Andersen
2018-09-27 16:51 ` Jann Horn
2018-09-27 21:42 ` Kees Cook
2018-10-08 13:55 ` Christian Brauner
2018-09-27 15:11 ` [PATCH v7 3/6] seccomp: add a way to get a listener fd from ptrace Tycho Andersen
2018-09-27 16:20 ` Jann Horn
2018-09-27 16:34 ` Tycho Andersen
2018-09-27 17:35 ` Jann Horn
2018-09-27 18:09 ` Tycho Andersen
2018-09-27 21:53 ` Kees Cook
2018-10-08 15:16 ` Christian Brauner
2018-10-08 15:33 ` Jann Horn
2018-10-08 16:21 ` Christian Brauner
2018-10-08 16:42 ` Jann Horn
2018-10-08 18:18 ` Christian Brauner
2018-10-09 12:39 ` Jann Horn
2018-10-09 13:28 ` Christian Brauner
2018-10-09 13:36 ` Jann Horn
2018-10-09 13:49 ` Christian Brauner
2018-10-09 13:50 ` Jann Horn
2018-10-09 14:09 ` Christian Brauner
2018-10-09 15:26 ` Jann Horn
2018-10-09 16:20 ` Christian Brauner
2018-10-09 16:26 ` Jann Horn
2018-10-10 12:54 ` Christian Brauner
2018-10-10 13:09 ` Christian Brauner
2018-10-10 13:10 ` Jann Horn
2018-10-10 13:18 ` Christian Brauner
2018-10-10 15:31 ` Paul Moore
2018-10-10 15:33 ` Jann Horn
2018-10-10 15:39 ` Christian Brauner
2018-10-10 16:54 ` Tycho Andersen
2018-10-10 17:15 ` Christian Brauner
2018-10-10 17:26 ` Tycho Andersen
2018-10-10 18:28 ` Christian Brauner
2018-10-11 7:24 ` Paul Moore
2018-10-11 13:39 ` Jann Horn
2018-10-11 23:10 ` Paul Moore
2018-10-12 1:02 ` Andy Lutomirski
2018-10-12 20:02 ` Tycho Andersen
2018-10-12 20:06 ` Jann Horn
2018-10-12 20:11 ` Christian Brauner
2018-10-08 18:00 ` Tycho Andersen
2018-10-08 18:41 ` Christian Brauner
2018-10-10 17:45 ` Andy Lutomirski
2018-10-10 18:26 ` Christian Brauner
2018-09-27 15:11 ` [PATCH v7 4/6] files: add a replace_fd_files() function Tycho Andersen
2018-09-27 16:49 ` Jann Horn
2018-09-27 18:04 ` Tycho Andersen
2018-09-27 21:59 ` Kees Cook
2018-09-28 2:20 ` Kees Cook
2018-09-28 2:46 ` Jann Horn
2018-09-28 5:23 ` Tycho Andersen
2018-09-27 15:11 ` [PATCH v7 5/6] seccomp: add a way to pass FDs via a notification fd Tycho Andersen
2018-09-27 16:39 ` Jann Horn
2018-09-27 22:13 ` Tycho Andersen
2018-09-27 19:28 ` Jann Horn
2018-09-27 22:14 ` Tycho Andersen
2018-09-27 22:17 ` Jann Horn
2018-09-27 22:49 ` Tycho Andersen
2018-09-27 22:09 ` Kees Cook
2018-09-27 22:15 ` Tycho Andersen [this message]
2018-09-27 15:11 ` [PATCH v7 6/6] samples: add an example of seccomp user trap Tycho Andersen
2018-09-27 22:11 ` Kees Cook
2018-09-28 21:57 ` [PATCH v7 0/6] seccomp trap to userspace Michael Kerrisk (man-opages)
2018-09-28 22:03 ` Tycho Andersen
2018-09-28 22:16 ` Michael Kerrisk (man-pages)
2018-09-28 22:34 ` Kees Cook
2018-09-28 22:46 ` Michael Kerrisk (man-pages)
2018-09-28 22:48 ` Jann Horn
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=20180927221540.GE15491@cisco.cisco.com \
--to=tycho@tycho.ws \
--cc=christian.brauner@ubuntu.com \
--cc=containers@lists.linux-foundation.org \
--cc=ebiederm@xmission.com \
--cc=jannh@google.com \
--cc=keescook@chromium.org \
--cc=linux-api@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=oleg@redhat.com \
--cc=serge@hallyn.com \
--cc=suda.akihiro@lab.ntt.co.jp \
--cc=tyhicks@canonical.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).