Re: [PATCH v3 0/4] seccomp trap to userspace

[Tycho Andersen <tycho@tycho.ws>]

Hi Kees,

On Fri, Jun 08, 2018 at 09:29:42AM -0700, Kees Cook wrote:
> On Thu, May 31, 2018 at 7:49 AM, Tycho Andersen <tycho@tycho.ws> wrote:
> > Hi all,
> >
> > Here's a v3 of the seccomp trap to userspace, with all the nits from v2
> > fixed. Open questions from v2 are still:
> >
> > 1. is it ok not to use netlink?
> 
> Yeah, I think there isn't a sensible way to reuse that API, which is
> too bad. Let's just try to keep this interface future-proofed. :)

Yes, I think it is, assuming that we always use a zero value as the
"do the same thing as before" value. Perhaps I should write that
assumption down somewhere...

> > 2. what should the fd passing API look like? (see patch notes on this
> >    one for details of why the current one might (?) be a problem)
> 
> The only thing in my mind is avoiding the problems with other fd
> passing API (e.g. when do rlimits get checked, etc).

My read of get_unused_fd_flags() is that it does check RLIMIT_NOFILE,
so I think we're ok there.

My biggest concern was just about the case where we want to do
something besides return an fd from a syscall (e.g. install an fd, but
return it via some pointer or something), but I'm not aware of
anywhere we do that today, so maybe I'm worrying about it too much.

> > As an added bonus, I've also written some stress testing, with lots of
> > tasks and listeners (1000 of each) sharing the same notification thread,
> > and not found any issues so far. Code is here:
> > https://github.com/tych0/kernel-utils/blob/master/seccomp/notify_stress.c
> > although I haven't included it in the patchset.
> 
> That's excellent, thanks!
> 
> > v2: https://lkml.org/lkml/2018/5/17/627
> >
> > Tycho Andersen (4):
> >   seccomp: add a return code to trap to userspace
> >   seccomp: make get_nth_filter available outside of CHECKPOINT_RESTORE
> >   seccomp: add a way to get a listener fd from ptrace
> >   seccomp: add support for passing fds via USER_NOTIF
> 
> I'm under a time crunch with the merge window, but after -rc2 I should
> have time to give this some close review. FWIW, I expect this to enter
> -next this cycle and get it into the 4.19 merge window: we need the
> feature and the alternatives have been well explored and don't look
> workable.

No rush. I am preparing a v4 with the various comments in this thread
fixed, hopefully I'll send it out early next week.

Tycho