[PATCH RFC 2/2] seccomp: implement SECCOMP_FILTER_FLAG_NO_INHERITANCE

[Paul Chaignon]

On Fri, Nov 15, 2019 at 06:55:55PM +0300, Dmitry V. Levin wrote:
> On Fri, Nov 15, 2019 at 04:46:14PM +0100, Paul Chaignon wrote:
> > On Thu, Nov 14, 2019 at 07:06:20PM +0100, Paul Chaignon wrote:
> > > On Mon, Nov 11, 2019 at 11:44:50PM +0300, Dmitry V. Levin wrote:
> > > > On Mon, Nov 11, 2019 at 04:20:20PM +0100, Paul Chaignon wrote:
> > 
> > [...]
> > 
> > > > >  /*
> > > > >   * All BPF programs must return a 32-bit value.
> > > > > diff --git a/kernel/fork.c b/kernel/fork.c
> > > > > index 55af6931c6ec..1df5b058b067 100644
> > > > > --- a/kernel/fork.c
> > > > > +++ b/kernel/fork.c
> > > > > @@ -1605,6 +1605,11 @@ static void copy_seccomp(struct task_struct *p)
> > > > >  	 */
> > > > >  	assert_spin_locked(&current->sighand->siglock);
> > > > >  
> > > > > +	if (!inherited_seccomp_filter(current)) {
> > > > > +		clear_tsk_thread_flag(p, TIF_SECCOMP);
> > > > > +		return;
> > > > > +	}
> > > > 
> > > > Since the task can have many seccomp filters (chained by seccomp_filter.prev)
> > > > and SECCOMP_FILTER_FLAG_NO_INHERIT flag affects only those filters that were
> > > > created using this flag, this code should copy only those filters that are
> > > > excluded from the fork inheritance.
> > > 
> > > Hm, I had the semantics all wrong.  I'll send a v2 with a new patch and
> > > test.
> > 
> > Actually, I think we should just mandate that all seccomp filters in the
> > list have the same value for flag SECCOMP_FILTER_FLAG_NO_FORK_INHERIT.
> > This should be as simple as checking that the flag has the same value for
> > a new filter and its parent.  In case of TSYNC we should be fine since all
> > threads need to have the same ancestors as the calling thread.
> > 
> > If we don't do that, we'll have to inherit partial lists, which prevents
> > us from using references since we'll need to modify field .prev of some
> > filters.  I'm guessing making actual copies of filters will not be
> > acceptable upstream...
> > 
> > One big issue with this approach is if we want to strace an application
> > that itself uses seccomp-bpf.  In that case, the application will likely
> > fail to set up its own filter because flag NO_FORK_INHERIT differ.
> 
> Seccomp filters are ubiquitous nowadays, software like systemd-nspawn
> generates and applies seccomp filters by default.  If the proposed feature
> does not support this workflow, does it have a chance to be accepted
> into the kernel?

Yep, that was my concern too...  I'll have a second look after the weekend
in case I missed something, but I don't see any other clear solution at
the moment :(

It's a shame.  We'll only ever be able to transparently enable seccomp
filter in strace when -f is given.

Paul