Re: [PATCH] seccomp: Add group_leader pid to seccomp_notif

Tycho Andersen
2020-05-18T13:20:41Z

On Mon, May 18, 2020 at 02:53:25PM +0200, Christian Brauner 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;
> > };
> >
> > ?
>
> Oh yes of course, sorry that was my stupid typo. I meant:
>
> struct seccomp_notif2 {
> __u32 notif_size;
> __u64 id;
> __u32 pid;
> __u32 flags;
> struct seccomp_data *data;
> __u32 data_size;
> __u32 new_field;
> }
>
> at which point things should just work imho.

Are you saying that data_size is an input? Because I don't think they
Just Work otherwise.

Tycho