[CRIU] Q: Seccomp in multi-thread tasks

[Tycho Andersen]

On Mon, Jan 09, 2017 at 03:33:43PM +0300, Pavel Emelyanov wrote:
> On 01/09/2017 02:18 PM, Tycho Andersen wrote:
> > On Mon, Jan 09, 2017 at 12:47:24PM +0300, Pavel Emelyanov wrote:
> >> Hi, Tycho!
> >>
> >> While looking at the https://github.com/xemul/criu/issues/268 issue I've
> >> noticed that since commit 221af18e (seccomp: add support for SECCOMP_MODE_FILTER)
> >> the call to restore_seccomp() is removed from __export_restore_thread() which
> >> should result in seccomp not being restored for sub-threads.
> >>
> >> Am I correct with this? What was the reason for this removal?
> > 
> > I think it's because the filters are stored on the creds struct, which
> > CRIU only keeps one copy of right now. We require any filters to be
> > the same across all threads in a thread group, similar to how we do
> > the rest of the members of the creds struct (via
> > proc_status_creds_eq), so I think the idea was that we'd only have to
> > restore the seccomp policy once. That said, it looks wrong, since
> > there's no way to restore the seccomp policy across the whole thread
> > group, 
> 
> Yes, it also looked incorrect to me, but then I found the 
> SECCOMP_FILTER_FLAG_TSYNC flag in restore_seccomp(). What's it for?
> The comment says that it's to make the seccomp be the same across
> the thread group, how does it do this?

Oh, right :). I forgot about all of this. So then I think the current
code *should* be correct, because SECCOMP_FILTER_FLAG_TSYNC does
exactly this: it attaches the argument policy to all the threads in
the thread group. We could drop this and issue a sys_seccomp in each
thread, but since the kernel gives a mechanism to just do it once, I
used that.

As for how it works... some voodoo that Kees found that allows one
thread to modify the other's struct creds. The original motivation for
him was because they wanted some way to secure chrome, who loads
graphics drivers that sometimes spawn threads.

Tycho

> > so we should probably re-add it.
> > 
> > Tycho
> > 
> >> -- Pavel
> > .
> > 
>