[PATCH v2 net-next 3/3] bpf: add unprivileged bpf tests
Add new tests samples/bpf/test_verifier: unpriv: return pointer checks that pointer cannot be returned from the eBPF program unpriv: add const to pointer unpriv: add pointer to pointer unpriv: neg pointer checks that pointer arithmetic is disallowed unpriv: cmp pointer with const unpriv: cmp pointer with pointer checks that comparison of pointers is disallowed Only one case allowed 'void *value = bpf_map_lookup_elem(..); if (value == 0) ...' unpriv: check that printk is disallowed since bpf_trace_printk is not available to unprivileged unpriv: pass pointer to helper function checks that pointers cannot be passed to functions that expect integers If function expects a pointer the verifier allows only that type of pointer. Like 1st argument of bpf_map_lookup_elem() must be pointer to map. (applies to non-root as well) unpriv: indirectly pass pointer on stack to helper function checks that pointer stored into stack cannot be used as part of key passed into bpf_map_lookup_elem() unpriv: mangle pointer on stack 1 unpriv: mangle pointer on stack 2 checks that writing into stack slot that already contains a pointer is disallowed unpriv: read pointer from stack in small chunks checks that < 8 byte read from stack slot that contains a pointer is disallowed unpriv: write pointer into ctx checks that storing pointers into skb->fields is disallowed unpriv: write pointer into map elem value checks that storing pointers into element values is disallowed For example: int bpf_prog(struct __sk_buff *skb) { u32 key = 0; u64 *value = bpf_map_lookup_elem(&map, &key); if (value) *value = (u64) skb; } will be rejected. unpriv: partial copy of pointer checks that doing 32-bit register mov from register containing a pointer is disallowed unpriv: pass pointer to tail_call checks that passing pointer as an index into bpf_tail_call is disallowed unpriv: cmp map pointer with zero checks that comparing map pointer with constant is disallowed unpriv: write into frame pointer checks that frame pointer is read-only (applies to root too) unpriv: cmp of frame pointer checks that R10 cannot be using in comparison unpriv: cmp of stack pointer checks that Rx = R10 - imm is ok, but comparing Rx is not unpriv: obfuscate stack pointer checks that Rx = R10 - imm is ok, but Rx -= imm is not Signed-off-by: Alexei Starovoitov --- v1-v2: - split tests into separate patch - add tail_call and other tests and explain tests in commit log --- samples/bpf/libbpf.h|8 + samples/bpf/test_verifier.c | 357 +-- 2 files changed, 355 insertions(+), 10 deletions(-) diff --git a/samples/bpf/libbpf.h b/samples/bpf/libbpf.h index 7235e292a03b..b7f63c70b4a2 100644 --- a/samples/bpf/libbpf.h +++ b/samples/bpf/libbpf.h @@ -64,6 +64,14 @@ extern char bpf_log_buf[LOG_BUF_SIZE]; .off = 0, \ .imm = 0 }) +#define BPF_MOV32_REG(DST, SRC)\ + ((struct bpf_insn) {\ + .code = BPF_ALU | BPF_MOV | BPF_X, \ + .dst_reg = DST, \ + .src_reg = SRC, \ + .off = 0, \ + .imm = 0 }) + /* Short form of mov, dst_reg = imm32 */ #define BPF_MOV64_IMM(DST, IMM)\ diff --git a/samples/bpf/test_verifier.c b/samples/bpf/test_verifier.c index ee0f110c9c54..563c507c0a09 100644 --- a/samples/bpf/test_verifier.c +++ b/samples/bpf/test_verifier.c @@ -15,20 +15,27 @@ #include #include #include +#include +#include #include "libbpf.h" #define MAX_INSNS 512 #define ARRAY_SIZE(x) (sizeof(x) / sizeof(*(x))) +#define MAX_FIXUPS 8 + struct bpf_test { const char *descr; struct bpf_insn insns[MAX_INSNS]; - int fixup[32]; + int fixup[MAX_FIXUPS]; + int prog_array_fixup[MAX_FIXUPS]; const char *errstr; + const char *errstr_unpriv; enum { + UNDEF, ACCEPT, REJECT - } result; + } result, result_unpriv; enum bpf_prog_type prog_type; }; @@ -96,6 +103,7 @@ static struct bpf_test tests[] = { BPF_EXIT_INSN(), }, .errstr = "invalid BPF_LD_IMM insn", + .errstr_unpriv = "R1 pointer comparison", .result = REJECT, }, { @@ -109,6 +117,7 @@ static struct bpf_test tests[] = { BPF_EXIT_INSN(), }, .errstr = "invalid BPF_LD_IMM insn", + .errstr_unpriv = "R1 pointer comparison", .result = REJECT, }, { @@ -219,6 +228,7 @@ static struct bpf_test tests[] = {
Re: [PATCH v2 net-next 3/3] bpf: add unprivileged bpf tests
On Wed, Oct 7, 2015 at 10:23 PM, Alexei Starovoitov wrote: > Add new tests samples/bpf/test_verifier: > > unpriv: return pointer > checks that pointer cannot be returned from the eBPF program > > unpriv: add const to pointer > unpriv: add pointer to pointer > unpriv: neg pointer > checks that pointer arithmetic is disallowed > > unpriv: cmp pointer with const > unpriv: cmp pointer with pointer > checks that comparison of pointers is disallowed > Only one case allowed 'void *value = bpf_map_lookup_elem(..); if (value == > 0) ...' > > unpriv: check that printk is disallowed > since bpf_trace_printk is not available to unprivileged > > unpriv: pass pointer to helper function > checks that pointers cannot be passed to functions that expect integers > If function expects a pointer the verifier allows only that type of pointer. > Like 1st argument of bpf_map_lookup_elem() must be pointer to map. > (applies to non-root as well) > > unpriv: indirectly pass pointer on stack to helper function > checks that pointer stored into stack cannot be used as part of key > passed into bpf_map_lookup_elem() > > unpriv: mangle pointer on stack 1 > unpriv: mangle pointer on stack 2 > checks that writing into stack slot that already contains a pointer > is disallowed > > unpriv: read pointer from stack in small chunks > checks that < 8 byte read from stack slot that contains a pointer is > disallowed > > unpriv: write pointer into ctx > checks that storing pointers into skb->fields is disallowed > > unpriv: write pointer into map elem value > checks that storing pointers into element values is disallowed > For example: > int bpf_prog(struct __sk_buff *skb) > { > u32 key = 0; > u64 *value = bpf_map_lookup_elem(&map, &key); > if (value) >*value = (u64) skb; > } > will be rejected. > > unpriv: partial copy of pointer > checks that doing 32-bit register mov from register containing > a pointer is disallowed > > unpriv: pass pointer to tail_call > checks that passing pointer as an index into bpf_tail_call > is disallowed > > unpriv: cmp map pointer with zero > checks that comparing map pointer with constant is disallowed > > unpriv: write into frame pointer > checks that frame pointer is read-only (applies to root too) > > unpriv: cmp of frame pointer > checks that R10 cannot be using in comparison > > unpriv: cmp of stack pointer > checks that Rx = R10 - imm is ok, but comparing Rx is not > > unpriv: obfuscate stack pointer > checks that Rx = R10 - imm is ok, but Rx -= imm is not > > Signed-off-by: Alexei Starovoitov > --- > v1-v2: > - split tests into separate patch > - add tail_call and other tests and explain tests in commit log > --- > samples/bpf/libbpf.h|8 + > samples/bpf/test_verifier.c | 357 > +-- Instead of living in samples/ could these be moved and hooked up to tools/testing/selftests/ instead? -Kees > 2 files changed, 355 insertions(+), 10 deletions(-) > > diff --git a/samples/bpf/libbpf.h b/samples/bpf/libbpf.h > index 7235e292a03b..b7f63c70b4a2 100644 > --- a/samples/bpf/libbpf.h > +++ b/samples/bpf/libbpf.h > @@ -64,6 +64,14 @@ extern char bpf_log_buf[LOG_BUF_SIZE]; > .off = 0, \ > .imm = 0 }) > > +#define BPF_MOV32_REG(DST, SRC)\ > + ((struct bpf_insn) {\ > + .code = BPF_ALU | BPF_MOV | BPF_X, \ > + .dst_reg = DST, \ > + .src_reg = SRC, \ > + .off = 0, \ > + .imm = 0 }) > + > /* Short form of mov, dst_reg = imm32 */ > > #define BPF_MOV64_IMM(DST, IMM)\ > diff --git a/samples/bpf/test_verifier.c b/samples/bpf/test_verifier.c > index ee0f110c9c54..563c507c0a09 100644 > --- a/samples/bpf/test_verifier.c > +++ b/samples/bpf/test_verifier.c > @@ -15,20 +15,27 @@ > #include > #include > #include > +#include > +#include > #include "libbpf.h" > > #define MAX_INSNS 512 > #define ARRAY_SIZE(x) (sizeof(x) / sizeof(*(x))) > > +#define MAX_FIXUPS 8 > + > struct bpf_test { > const char *descr; > struct bpf_insn insns[MAX_INSNS]; > - int fixup[32]; > + int fixup[MAX_FIXUPS]; > + int prog_array_fixup[MAX_FIXUPS]; > const char *errstr; > + const char *errstr_unpriv; > enum { > + UNDEF, > ACCEPT, > REJECT > - } result; > + } result, result_unpriv; > enum bpf_prog_type prog_type; > }; > > @@ -96,6 +103,7 @@ static struct bpf_test tests[] = { > BPF_EXIT_INSN(), > }, > .errstr = "invalid BPF_LD_IMM insn", > + .errstr_u
Re: [PATCH v2 net-next 3/3] bpf: add unprivileged bpf tests
On 10/8/15 10:46 AM, Kees Cook wrote: samples/bpf/libbpf.h|8 + > samples/bpf/test_verifier.c | 357 +-- Instead of living in samples/ could these be moved and hooked up to tools/testing/selftests/ instead? as a follow up patch. yes. of course. along with test_maps. Fengguang's todo already includes adding test_bpf.ko to 0-day buildbot. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html