| Message ID | 20200908175703.2463721-1-yhs@fb.com |
|---|---|
| State | Accepted |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | bpf: permit map_ptr arithmetic with opcode add and offset 0 | expand |
On Tue, Sep 8, 2020 at 10:58 AM Yonghong Song <yhs@fb.com> wrote: > > Change selftest map_ptr_kern.c with disabling inlining for > one of subtests, which will fail the test without previous > verifier change. Also added to verifier test for both > "map_ptr += scalar" and "scalar += map_ptr" arithmetic. > > Signed-off-by: Yonghong Song <yhs@fb.com> > --- Acked-by: Andrii Nakryiko <andriin@fb.com> > .../selftests/bpf/progs/map_ptr_kern.c | 10 +++++- > .../testing/selftests/bpf/verifier/map_ptr.c | 32 +++++++++++++++++++ > 2 files changed, 41 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/bpf/progs/map_ptr_kern.c b/tools/testing/selftests/bpf/progs/map_ptr_kern.c > index 982a2d8aa844..0b754106407d 100644 > --- a/tools/testing/selftests/bpf/progs/map_ptr_kern.c > +++ b/tools/testing/selftests/bpf/progs/map_ptr_kern.c > @@ -82,6 +82,14 @@ static inline int check_default(struct bpf_map *indirect, > return 1; > } > > +static __attribute__ ((noinline)) int just fyi: there is now __noinline defined in bpf_helpers.h, saving a bunch of typing > +check_default_noinline(struct bpf_map *indirect, struct bpf_map *direct) > +{ > + VERIFY(check(indirect, direct, sizeof(__u32), sizeof(__u32), > + MAX_ENTRIES)); > + return 1; > +} > + > typedef struct { > int counter; > } atomic_t; [...]
On Tue, Sep 08, 2020 at 04:11:21PM -0700, Andrii Nakryiko wrote: > On Tue, Sep 8, 2020 at 10:58 AM Yonghong Song <yhs@fb.com> wrote: > > > > Change selftest map_ptr_kern.c with disabling inlining for > > one of subtests, which will fail the test without previous > > verifier change. Also added to verifier test for both > > "map_ptr += scalar" and "scalar += map_ptr" arithmetic. > > > > Signed-off-by: Yonghong Song <yhs@fb.com> > > --- > > Acked-by: Andrii Nakryiko <andriin@fb.com> > > > .../selftests/bpf/progs/map_ptr_kern.c | 10 +++++- > > .../testing/selftests/bpf/verifier/map_ptr.c | 32 +++++++++++++++++++ > > 2 files changed, 41 insertions(+), 1 deletion(-) > > > > diff --git a/tools/testing/selftests/bpf/progs/map_ptr_kern.c b/tools/testing/selftests/bpf/progs/map_ptr_kern.c > > index 982a2d8aa844..0b754106407d 100644 > > --- a/tools/testing/selftests/bpf/progs/map_ptr_kern.c > > +++ b/tools/testing/selftests/bpf/progs/map_ptr_kern.c > > @@ -82,6 +82,14 @@ static inline int check_default(struct bpf_map *indirect, > > return 1; > > } > > > > +static __attribute__ ((noinline)) int > > just fyi: there is now __noinline defined in bpf_helpers.h, saving a > bunch of typing I fixed it manually while applying. Thanks everyone.
diff --git a/tools/testing/selftests/bpf/progs/map_ptr_kern.c b/tools/testing/selftests/bpf/progs/map_ptr_kern.c index 982a2d8aa844..0b754106407d 100644 --- a/tools/testing/selftests/bpf/progs/map_ptr_kern.c +++ b/tools/testing/selftests/bpf/progs/map_ptr_kern.c @@ -82,6 +82,14 @@ static inline int check_default(struct bpf_map *indirect, return 1; } +static __attribute__ ((noinline)) int +check_default_noinline(struct bpf_map *indirect, struct bpf_map *direct) +{ + VERIFY(check(indirect, direct, sizeof(__u32), sizeof(__u32), + MAX_ENTRIES)); + return 1; +} + typedef struct { int counter; } atomic_t; @@ -107,7 +115,7 @@ static inline int check_hash(void) struct bpf_map *map = (struct bpf_map *)&m_hash; int i; - VERIFY(check_default(&hash->map, map)); + VERIFY(check_default_noinline(&hash->map, map)); VERIFY(hash->n_buckets == MAX_ENTRIES); VERIFY(hash->elem_size == 64); diff --git a/tools/testing/selftests/bpf/verifier/map_ptr.c b/tools/testing/selftests/bpf/verifier/map_ptr.c index b52209db8250..637f9293bda8 100644 --- a/tools/testing/selftests/bpf/verifier/map_ptr.c +++ b/tools/testing/selftests/bpf/verifier/map_ptr.c @@ -60,3 +60,35 @@ .result = ACCEPT, .retval = 1, }, +{ + "bpf_map_ptr: r = 0, map_ptr = map_ptr + r", + .insns = { + BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0), + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8), + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_LD_MAP_FD(BPF_REG_1, 0), + BPF_ALU64_REG(BPF_ADD, BPF_REG_1, BPF_REG_0), + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem), + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_EXIT_INSN(), + }, + .fixup_map_hash_16b = { 4 }, + .result = ACCEPT, +}, +{ + "bpf_map_ptr: r = 0, r = r + map_ptr", + .insns = { + BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0), + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8), + BPF_MOV64_IMM(BPF_REG_1, 0), + BPF_LD_MAP_FD(BPF_REG_0, 0), + BPF_ALU64_REG(BPF_ADD, BPF_REG_1, BPF_REG_0), + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem), + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_EXIT_INSN(), + }, + .fixup_map_hash_16b = { 4 }, + .result = ACCEPT, +},
Change selftest map_ptr_kern.c with disabling inlining for one of subtests, which will fail the test without previous verifier change. Also added to verifier test for both "map_ptr += scalar" and "scalar += map_ptr" arithmetic. Signed-off-by: Yonghong Song <yhs@fb.com> --- .../selftests/bpf/progs/map_ptr_kern.c | 10 +++++- .../testing/selftests/bpf/verifier/map_ptr.c | 32 +++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-)