| Message ID | 158983219168.6512.11784750707821433806.stgit@john-Precision-5820-Tower |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | verifier, improve ptr is_branch_taken logic | expand |
On Mon, May 18, 2020 at 1:07 PM John Fastabend <john.fastabend@gmail.com> wrote: > > When we have pointer type that is known to be non-null we only follow > the non-null branch. This adds tests to cover the map_value pointer > returned from a map lookup. To force an error if both branches are > followed we do an ALU op on R10. > > Signed-off-by: John Fastabend <john.fastabend@gmail.com> > --- LGTM. Here likelihood of someone comparing map value pointer against non-zero scalar is much less likely, so I won't bother you to add test for that. Acked-by: Andrii Nakryiko <andriin@fb.com> > .../testing/selftests/bpf/verifier/value_or_null.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/tools/testing/selftests/bpf/verifier/value_or_null.c b/tools/testing/selftests/bpf/verifier/value_or_null.c > index 860d4a7..3ecb70a 100644 > --- a/tools/testing/selftests/bpf/verifier/value_or_null.c > +++ b/tools/testing/selftests/bpf/verifier/value_or_null.c > @@ -150,3 +150,22 @@ > .result_unpriv = REJECT, > .flags = F_NEEDS_EFFICIENT_UNALIGNED_ACCESS, > }, > +{ > + "map lookup and null branch prediction", > + .insns = { > + BPF_MOV64_IMM(BPF_REG_1, 10), > + BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_1, -8), > + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10), > + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8), > + BPF_LD_MAP_FD(BPF_REG_1, 0), > + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem), > + BPF_MOV64_REG(BPF_REG_6, BPF_REG_0), > + BPF_JMP_IMM(BPF_JEQ, BPF_REG_6, 0, 2), > + BPF_JMP_IMM(BPF_JNE, BPF_REG_6, 0, 1), > + BPF_ALU64_IMM(BPF_ADD, BPF_REG_10, 10), > + BPF_EXIT_INSN(), > + }, > + .fixup_map_hash_8b = { 4 }, > + .prog_type = BPF_PROG_TYPE_SCHED_CLS, > + .result = ACCEPT, > +}, >
diff --git a/tools/testing/selftests/bpf/verifier/value_or_null.c b/tools/testing/selftests/bpf/verifier/value_or_null.c index 860d4a7..3ecb70a 100644 --- a/tools/testing/selftests/bpf/verifier/value_or_null.c +++ b/tools/testing/selftests/bpf/verifier/value_or_null.c @@ -150,3 +150,22 @@ .result_unpriv = REJECT, .flags = F_NEEDS_EFFICIENT_UNALIGNED_ACCESS, }, +{ + "map lookup and null branch prediction", + .insns = { + BPF_MOV64_IMM(BPF_REG_1, 10), + BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_1, -8), + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8), + BPF_LD_MAP_FD(BPF_REG_1, 0), + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem), + BPF_MOV64_REG(BPF_REG_6, BPF_REG_0), + BPF_JMP_IMM(BPF_JEQ, BPF_REG_6, 0, 2), + BPF_JMP_IMM(BPF_JNE, BPF_REG_6, 0, 1), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_10, 10), + BPF_EXIT_INSN(), + }, + .fixup_map_hash_8b = { 4 }, + .prog_type = BPF_PROG_TYPE_SCHED_CLS, + .result = ACCEPT, +},
When we have pointer type that is known to be non-null we only follow the non-null branch. This adds tests to cover the map_value pointer returned from a map lookup. To force an error if both branches are followed we do an ALU op on R10. Signed-off-by: John Fastabend <john.fastabend@gmail.com> --- .../testing/selftests/bpf/verifier/value_or_null.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)