lkml.org 
[lkml]   [2020]   [May]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] seccomp: Add group_leader pid to seccomp_notif
    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

    \
     
     \ /
      Last update: 2020-05-18 14:06    [W:4.919 / U:0.132 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site