On Mon, Feb 26, 2018 at 8:08 PM, Sargun Dhillon <sar...@sargun.me> wrote: > On Mon, Feb 26, 2018 at 7:57 PM, Tycho Andersen <ty...@tycho.ws> wrote: >> On Mon, Feb 26, 2018 at 07:49:48PM -0800, Sargun Dhillon wrote: >>> On Mon, Feb 26, 2018 at 4:54 PM, Tycho Andersen <ty...@tycho.ws> wrote: >>> > On Mon, Feb 26, 2018 at 07:27:05AM +0000, Sargun Dhillon wrote: >>> >> +config SECCOMP_FILTER_EXTENDED >>> >> + bool "Extended BPF seccomp filters" >>> >> + depends on SECCOMP_FILTER && BPF_SYSCALL >>> >> + depends on !CHECKPOINT_RESTORE >>> > >>> > Why not just give -EINVAL or something in case one of these is >>> > requested, instead of making them incompatible at compile time? >>> > >>> > Tycho >>> There's already code to return -EMEDIUMTYPE if it's a non-classic, or >>> non-saved filter. Under the normal case, with CHECKPOINT_RESTORE >>> enabled, you should never be able to get that. I think it makes sense >>> to preserve this behaviour. >> >> Oh, right. So can't we just drop this, and the existing code will >> DTRT, i.e. give you -EMEDIUMTYPE because the new filters aren't >> supported, until they are? >> >> Tycho > My suggestion is we merge this as is, so we don't break checkpoint / > restore, and I will try to get the filter dumping patching in the same > development cycle as it comes at minimal risk. Otherwise, we risk > introducing a feature which could break checkpoint/restore, even in > unprivileged containers since anyone can load a BPF Seccomp filter.
There is no rush to merge such a drastic expansion of the seccomp attack surface. :) For me, the driving feature is if we can get Tycho's notifier implemented in eBPF. The speed improvements, as far as I'm concerned, aren't sufficient to add eBPF to seccomp. They are certainly a nice benefit, but seccomp must be very conservative about adding attack surface. -Kees -- Kees Cook Pixel Security