On Fri, Sep 11, 2015 at 06:03:59PM +0200, Daniel Borkmann wrote:

On 09/11/2015 04:44 PM, Tycho Andersen wrote:

quoted

On Fri, Sep 11, 2015 at 03:02:36PM +0200, Daniel Borkmann wrote:

quoted

On 09/11/2015 02:20 AM, Tycho Andersen wrote:

quoted

In the next patch, we're going to add a way to access the underlying
filters via bpf fds. This means that we need to ref-count both the
struct seccomp_filter objects and the struct bpf_prog objects separately,
in case a process dies but a filter is still referred to by another
process.

Additionally, we mark classic converted seccomp filters as seccomp eBPF
programs, since they are a subset of what is supported in seccomp eBPF.

Signed-off-by: Tycho Andersen <redacted>
CC: Kees Cook <redacted>
CC: Will Drewry <redacted>
CC: Oleg Nesterov <redacted>
CC: Andy Lutomirski <redacted>
CC: Pavel Emelyanov <redacted>
CC: Serge E. Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org>
CC: Alexei Starovoitov <redacted>
CC: Daniel Borkmann <redacted>
---
 kernel/seccomp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 245df6b..afaeddf 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c

@@ -378,6 +378,8 @@ static struct seccomp_filter *seccomp_prepare_filter(struct sock_fprog *fprog)
 	}

 	atomic_set(&sfilter->usage, 1);
+	atomic_set(&sfilter->prog->aux->refcnt, 1);
+	sfilter->prog->type = BPF_PROG_TYPE_SECCOMP;

So, if you do this, then this breaks the assumption of eBPF JITs
that, currently, all classic converted BPF programs always have a
prog->type of BPF_PROG_TYPE_UNSPEC (see: bpf_prog_was_classic()).

Currently, JITs make use of this information to determine whether
A and X mappings for such programs should or should not be cleared
in the prologue (s390 currently).

In the seccomp_prepare_filter() stage, we're already past that, so
it will not cause an issue, but we certainly would need to be very
careful in future, if bpf_prog_was_classic() is then used at a later
stage when we already have a generated bpf_prog somewhere, as then
this assumption will break.

The only reason we need to do this is to allow BPF_DUMP_PROG to work,
since we were restricting it to only allow dumping of seccomp
programs, since those don't have maps. Instead, perhaps we could allow
dumping of BPF_PROG_TYPE_SECCOMP and BPF_PROG_TYPE_UNSPEC?

There are possibilities that BPF_PROG_TYPE_UNSPEC is calling helpers
already today, at least in networking case, not seccomp. So, since
you want to export [classic -> eBPF] only for seccomp, put fds on them
and dump these via bpf(2), you could allow that (with a big comment
stating why it's safe), but mid-term we really need to sanitize all
this stuff properly as this is needed for other types, too.

Sorry, just to be clear, you're suggesting that the patch is ok modulo
a comment describing the jit issues?

Tycho