| Message ID | 2ed58e2e-40b7-a57b-a784-33d441b13a42@solarflare.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | bpf/verifier: fix bounds calculation on BPF_RSH | expand |
On Tue, Dec 05, 2017 at 07:15:57PM +0000, Edward Cree wrote: > Signed-off-by: Edward Cree <ecree@solarflare.com> > --- > tools/testing/selftests/bpf/test_align.c | 39 ++++++++++++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > > diff --git a/tools/testing/selftests/bpf/test_align.c b/tools/testing/selftests/bpf/test_align.c > index 8591c89c0828..24c6757b4c51 100644 > --- a/tools/testing/selftests/bpf/test_align.c > +++ b/tools/testing/selftests/bpf/test_align.c > @@ -601,6 +601,45 @@ static struct bpf_align_test tests[] = { > {20, "R5=pkt(id=2,off=0,r=4,umin_value=2,umax_value=1082,var_off=(0x2; 0x7fc))"}, > }, > }, > + { > + .descr = "unknown shift negative", > + /* This isn't really a test of the alignment code, rather of the > + * signed min/max value handling, but it makes use of the > + * register-state-extracting code in do_test_single(), which > + * test_verifier.c doesn't have. > + */ > + .insns = { > + LOAD_UNKNOWN(BPF_REG_3), > + BPF_ALU64_IMM(BPF_SUB, BPF_REG_3, 0xff), > + BPF_ALU64_IMM(BPF_LSH, BPF_REG_3, 1), > + LOAD_UNKNOWN(BPF_REG_4), > + BPF_ALU64_IMM(BPF_SUB, BPF_REG_4, 0xff), > + BPF_MOV64_REG(BPF_REG_5, BPF_REG_4), > + BPF_ALU64_IMM(BPF_RSH, BPF_REG_4, 1), > + BPF_ALU64_IMM(BPF_SUB, BPF_REG_5, 1), > + BPF_ALU64_IMM(BPF_RSH, BPF_REG_5, 1), > + BPF_MOV64_IMM(BPF_REG_0, 0), > + BPF_EXIT_INSN(), > + }, > + .prog_type = BPF_PROG_TYPE_SCHED_CLS, > + .matches = { > + {7, "R0=pkt(id=0,off=8,r=8,imm=0)"}, > + {7, "R3=inv(id=0,umax_value=255,var_off=(0x0; 0xff))"}, > + {8, "R3=inv(id=0,smin_value=-255,smax_value=0)"}, > + /* All the verifier knows is, it's even. While we could > + * conclude something tighter (the sign bit does not > + * change), the verifier doesn't bother right now. > + */ > + {9, "R3=inv(id=0,smax_value=9223372036854775806,umax_value=18446744073709551614,var_off=(0x0; 0xfffffffffffffffe))"}, > + {16, "R3=pkt_end(id=0,off=0,imm=0)"}, > + {16, "R4=inv(id=0,umax_value=255,var_off=(0x0; 0xff))"}, > + {17, "R4=inv(id=0,smin_value=-255,smax_value=0)"}, > + /* both 0 and 0x7f...fff are possible */ > + {19, "R4=inv(id=0,umax_value=9223372036854775807,var_off=(0x0; 0x7fffffffffffffff))"}, > + {20, "R5=inv(id=0,umin_value=18446744073709551360,var_off=(0xffffffffffffff00; 0xff))"}, > + {21, "R5=inv(id=0,umin_value=9223372036854775680,umax_value=9223372036854775807,var_off=(0x7fffffffffffff80; 0x7f))"}, hmm. it doesn't quite look right here and in this form it already conflicts with net-next. I would prefer to take only patch 1 into bpf->net and once bpf->net->linus->net-next merge happens to add the test there.
diff --git a/tools/testing/selftests/bpf/test_align.c b/tools/testing/selftests/bpf/test_align.c index 8591c89c0828..24c6757b4c51 100644 --- a/tools/testing/selftests/bpf/test_align.c +++ b/tools/testing/selftests/bpf/test_align.c @@ -601,6 +601,45 @@ static struct bpf_align_test tests[] = { {20, "R5=pkt(id=2,off=0,r=4,umin_value=2,umax_value=1082,var_off=(0x2; 0x7fc))"}, }, }, + { + .descr = "unknown shift negative", + /* This isn't really a test of the alignment code, rather of the + * signed min/max value handling, but it makes use of the + * register-state-extracting code in do_test_single(), which + * test_verifier.c doesn't have. + */ + .insns = { + LOAD_UNKNOWN(BPF_REG_3), + BPF_ALU64_IMM(BPF_SUB, BPF_REG_3, 0xff), + BPF_ALU64_IMM(BPF_LSH, BPF_REG_3, 1), + LOAD_UNKNOWN(BPF_REG_4), + BPF_ALU64_IMM(BPF_SUB, BPF_REG_4, 0xff), + BPF_MOV64_REG(BPF_REG_5, BPF_REG_4), + BPF_ALU64_IMM(BPF_RSH, BPF_REG_4, 1), + BPF_ALU64_IMM(BPF_SUB, BPF_REG_5, 1), + BPF_ALU64_IMM(BPF_RSH, BPF_REG_5, 1), + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_EXIT_INSN(), + }, + .prog_type = BPF_PROG_TYPE_SCHED_CLS, + .matches = { + {7, "R0=pkt(id=0,off=8,r=8,imm=0)"}, + {7, "R3=inv(id=0,umax_value=255,var_off=(0x0; 0xff))"}, + {8, "R3=inv(id=0,smin_value=-255,smax_value=0)"}, + /* All the verifier knows is, it's even. While we could + * conclude something tighter (the sign bit does not + * change), the verifier doesn't bother right now. + */ + {9, "R3=inv(id=0,smax_value=9223372036854775806,umax_value=18446744073709551614,var_off=(0x0; 0xfffffffffffffffe))"}, + {16, "R3=pkt_end(id=0,off=0,imm=0)"}, + {16, "R4=inv(id=0,umax_value=255,var_off=(0x0; 0xff))"}, + {17, "R4=inv(id=0,smin_value=-255,smax_value=0)"}, + /* both 0 and 0x7f...fff are possible */ + {19, "R4=inv(id=0,umax_value=9223372036854775807,var_off=(0x0; 0x7fffffffffffffff))"}, + {20, "R5=inv(id=0,umin_value=18446744073709551360,var_off=(0xffffffffffffff00; 0xff))"}, + {21, "R5=inv(id=0,umin_value=9223372036854775680,umax_value=9223372036854775807,var_off=(0x7fffffffffffff80; 0x7f))"}, + }, + }, }; static int probe_filter_length(const struct bpf_insn *fp)
Signed-off-by: Edward Cree <ecree@solarflare.com> --- tools/testing/selftests/bpf/test_align.c | 39 ++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)