From: Christian Brauner <christian.brauner@ubuntu.com>
To: Kees Cook <keescook@chromium.org>
Cc: Tycho Andersen <tycho@tycho.ws>, Aleksa Sarai <asarai@suse.de>,
linux-api@vger.kernel.org, containers@lists.linux-foundation.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] seccomp: Add group_leader pid to seccomp_notif
Date: Mon, 18 May 2020 14:05:45 +0200 [thread overview]
Message-ID: <20200518120545.w7fgyq56gwzzcdrf@wittgenstein> (raw)
In-Reply-To: <202005171428.68F30AA0@keescook>
On Sun, May 17, 2020 at 02:30:57PM -0700, Kees Cook wrote:
> On Sun, May 17, 2020 at 09:02:15AM -0600, Tycho Andersen wrote:
> > On Sun, May 17, 2020 at 08:46:03AM -0600, Tycho Andersen wrote:
> > > On Sun, May 17, 2020 at 04:33:11PM +0200, Christian Brauner wrote:
> > > > struct seccomp_notif2 {
> > > > __u32 notif_size;
> > > > __u64 id;
> > > > __u32 pid;
> > > > __u32 flags;
> > > > struct seccomp_data data;
> > > > __u32 data_size;
> > > > };
> > >
> > > I guess you need to put data_size before data, otherwise old userspace
> > > with a smaller struct seccomp_data will look in the wrong place.
> > >
> > > But yes, that'll work if you put two sizes in, which is probably
> > > reasonable since we're talking about two structs.
> >
> > Well, no, it doesn't either. Suppose we add a new field first to
> > struct seccomp_notif2:
> >
> > struct seccomp_notif2 {
> > __u32 notif_size;
> > __u64 id;
> > __u32 pid;
> > __u32 flags;
> > struct seccomp_data data;
> > __u32 data_size;
> > __u32 new_field;
> > };
> >
> > And next we add a new field to struct secccomp_data. When a userspace
> > compiled with just the new seccomp_notif2 field does:
> >
> > seccomp_notif2.new_field = ...;
> >
> > the compiler will put it in the wrong place for the kernel with the
> > new seccomp_data field too.
> >
> > Sort of feels like we should do:
> >
> > struct seccomp_notif2 {
> > struct seccomp_notif *notif;
> > struct seccomp_data *data;
> > };
>
> I'm going read this thread more carefully tomorrow, but I just wanted to
> mention that I'd *like* to extend seccomp_data for doing deep argument
> inspection of the new syscalls. I think it's the least bad of many
> designs, and I'll write that up in more detail. (I would *really* like
> to avoid extending seccomp's BPF language, and instead allow probing
> into the struct copied from userspace, etc.)
It's great to hear that you're on this. I haven't had time to work on
this since our kernel summit session. :/
And so far, I really like what I hear. I had the same general thought
that not extending seccomp's bpf is what we want. And to stress this
again before the mails come flooding in: we really need this in seccomp
itself not in any current or future LSM. :)
>
> Anyway, it's very related to this, so, yeah, probably we need a v2 of the
> notif API, but I'll try to get all the ideas here collected in one place.
Cool, I was kinda worried that people would think that's a crazy idea
but I really think we're better off with a redesign. And I think that's
totally ok and cleaner than hacking around it.
Christian
next prev parent reply other threads:[~2020-05-18 12:05 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-15 23:40 [PATCH] seccomp: Add group_leader pid to seccomp_notif Sargun Dhillon
2020-05-17 7:17 ` Kees Cook
2020-05-17 10:47 ` Christian Brauner
2020-05-17 11:21 ` Aleksa Sarai
2020-05-17 14:23 ` Tycho Andersen
2020-05-17 14:33 ` Christian Brauner
2020-05-17 14:35 ` Christian Brauner
2020-05-17 14:46 ` Tycho Andersen
2020-05-17 15:02 ` Tycho Andersen
2020-05-17 21:30 ` Kees Cook
2020-05-18 8:32 ` Sargun Dhillon
2020-05-18 12:45 ` Christian Brauner
2020-05-18 13:23 ` Tycho Andersen
2020-05-18 14:00 ` Christian Brauner
2020-05-18 12:05 ` Christian Brauner [this message]
2020-05-18 21:10 ` Kees Cook
2020-05-18 12:53 ` Christian Brauner
2020-05-18 13:20 ` Tycho Andersen
2020-05-18 21:37 ` Sargun Dhillon
2020-05-17 11:17 ` Sargun Dhillon
2020-05-18 23:08 ` Eric W. Biederman
2020-05-22 17:54 ` Sargun Dhillon
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=20200518120545.w7fgyq56gwzzcdrf@wittgenstein \
--to=christian.brauner@ubuntu.com \
--cc=asarai@suse.de \
--cc=containers@lists.linux-foundation.org \
--cc=keescook@chromium.org \
--cc=linux-api@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tycho@tycho.ws \
/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).