[PATCH RFC 2/2] seccomp: implement SECCOMP_FILTER_FLAG_NO_INHERITANCE

[Paul Chaignon]

Thanks for the review Dmitry!

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:
> > Userspace ptracer can use seccomp's SECCOMP_RET_TRACE action to stop the
> > tracee only at syscalls of interest.  Strace v5.3, for example, supports
> > this with the --seccomp-bpf option.  However, since seccomp filters are
> > inherited by children tasks, this behavior forces the userspace ptracer to
> > trace children tasks as well.
> > 
> > This patch adds a new seccomp syscall flag to SECCOMP_SET_MODE_FILTER to
> > prevent children tasks of the process from inheriting the filter.  It
> > allows ptracers to use seccomp's SECCOMP_RET_TRACE even when not following
> > children tasks.
> > 
> > Signed-off-by: Paul Chaignon <paul.chaignon at gmail.com>
> > ---
> >  include/linux/seccomp.h                       |  8 +++-
> >  include/uapi/linux/seccomp.h                  |  1 +
> >  kernel/fork.c                                 |  5 +++
> >  kernel/seccomp.c                              | 14 ++++++
> >  tools/testing/selftests/seccomp/seccomp_bpf.c | 44 ++++++++++++++++++-
> >  5 files changed, 70 insertions(+), 2 deletions(-)
> > 

[...]

> > diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
> > index 90734aa5aa36..e7f715fb7b34 100644
> > --- a/include/uapi/linux/seccomp.h
> > +++ b/include/uapi/linux/seccomp.h
> > @@ -22,6 +22,7 @@
> >  #define SECCOMP_FILTER_FLAG_LOG			(1UL << 1)
> >  #define SECCOMP_FILTER_FLAG_SPEC_ALLOW		(1UL << 2)
> >  #define SECCOMP_FILTER_FLAG_NEW_LISTENER	(1UL << 3)
> > +#define SECCOMP_FILTER_FLAG_NO_INHERIT		(1UL << 4)
> 
> NO_INHERIT is too generic, it doesn't clearly say that we want to skip
> fork inheritance.

I'm not sure I understand.  Is there another type of inheritance for
seccomp filters that I missed?

> 
> >  /*
> >   * 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.

[...]

Paul