[CRIU] [PATCH v2 1/3] seccomp: add initial support for SECCOMP_MODE_STRICT

[Tycho Andersen]

On Fri, Jun 05, 2015 at 07:36:12PM +0300, Pavel Emelyanov wrote:
> On 06/05/2015 07:15 PM, Tycho Andersen wrote:
> > On Thu, Jun 04, 2015 at 03:34:07PM +0300, Pavel Emelyanov wrote:
> >>
> >>>>>  	}
> >>>>>  
> >>>>> @@ -1632,6 +1634,9 @@ static int attach_to_tasks(bool root_seized, enum trace_flags *flag)
> >>>>>  				return -1;
> >>>>>  			}
> >>>>>  
> >>>>> +			if (suspend_seccomp(pid) < 0)
> >>>>> +				return -1;
> >>>>
> >>>> BTW, why do we need to suspend seccomp on restore? The sigreturn is allowed in strict
> >>>> mode so if we just turn one ON before it nothing bad will (should) happen.
> >>>
> >>> Because the unmap of the parasite blob happens after this (in
> >>> finalize_restore() via ptrace), and if that isn't allowed the task is
> >>> killed.
> >>
> >> Ah, indeed :( But then things look even simpler and we don't need separate stage :)
> >>
> >> Look, the pie/restorer.c just turns seccomp on one line before calling sigreturn
> >> and does so. While doing this call it will be stopped up by criu with ptrace. And,
> >> once criu did this, it should check whether seccomp is ON, suspend it if required
> >> and continue task's execution.
> >>
> >> Does it make sense?
> > 
> > ...and while fixing this, I realized that my poor understanding of
> > kernel threads meant that I'm only c/r-ing the seccomp state from the
> > first thread in the threadgroup, when in fact we should do it for all
> > of them (I'm looking in /proc/pid/status instead of
> > /proc/pid/task/tid/status, and only doing the restore once per pid
> > instead of per thread).
> > 
> > This probably means that the lsm stuff is wrong too, since it seems
> > that can happen per thread as well, IIUC.
> 
> It looks like the whole creds is ... not perfect in Linux http://ewontfix.com/17/ .

Sheesh. Quite the rabbit hole.

> From the CRIU perspective this means that creds.img should be applied to all threads
> in a group (which is true for most of the stuff, the restore_creds() is called by
> each thread on restore, need to fix that for LSM too). And, probably, some threads
> may have their own creds.img.

Yep. If my reading is correct, everything in the same thread group
uses the same creds.img (except for lsm, which needs to be fixed).
Since each thread can have its own creds, it seems like we need to fix
this to be per-thread? (Although probably in practice mostly they're
the same, so we could do some sort of de-duping, or only write a per
thread creds when it differs from the first thread's creds.)

Does that sound right? I'll send a patch to fix LSM behavior to match
the other creds behavior for now.

Tycho