| Message ID | 158507165554.15666.6019652542965367828.stgit@john-Precision-5820-Tower |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | ALU32 bounds tracking support | expand |
On Tue, Mar 24, 2020 at 10:40:55AM -0700, John Fastabend wrote: > Its possible to have divergent ALU32 and ALU64 bounds when using JMP32 > instructins and ALU64 arithmatic operations. Sometimes the clang will > even generate this code. Because the case is a bit tricky lets add > a specific test for it. > > Here is pseudocode asm version to illustrate the idea, > > 1 r0 = 0xffffffff00000001; > 2 if w0 > 1 goto %l[fail]; > 3 r0 += 1 > 5 if w0 > 2 goto %l[fail] > 6 exit > > The intent here is the verifier will fail the load if the 32bit bounds > are not tracked correctly through ALU64 op. Similarly we can check the > 64bit bounds are correctly zero extended after ALU32 ops. > > 1 r0 = 0xffffffff00000001; > 2 w0 += 1 > 2 if r0 < 0xffffffff00000001 goto %l[fail]; This should be 3. > + "bounds check mixed 32bit and 64bit arithmatic. test2", > + .insns = { > + BPF_MOV64_IMM(BPF_REG_0, 0), > + BPF_MOV64_IMM(BPF_REG_1, -1), > + BPF_ALU64_IMM(BPF_LSH, BPF_REG_1, 32), > + BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 1), > + /* r1 = 0xffffFFFF00000001 */ > + BPF_MOV64_IMM(BPF_REG_2, 3), > + /* r1 = 0x2 */ > + BPF_ALU32_IMM(BPF_ADD, BPF_REG_1, 1), > + /* check ALU32 op zero extends 64bit bounds */ > + BPF_JMP_REG(BPF_JGT, BPF_REG_1, BPF_REG_2, 1), > + BPF_JMP_A(1), > + /* invalid ldx if bounds are lost above */ > + BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_0, -1), > + BPF_EXIT_INSN(), > + }, > + .result = ACCEPT > +}, >
diff --git a/tools/testing/selftests/bpf/verifier/bounds.c b/tools/testing/selftests/bpf/verifier/bounds.c index cf72fcc..4d0d095 100644 --- a/tools/testing/selftests/bpf/verifier/bounds.c +++ b/tools/testing/selftests/bpf/verifier/bounds.c @@ -500,3 +500,42 @@ .errstr = "map_value pointer and 1000000000000", .result = REJECT }, +{ + "bounds check mixed 32bit and 64bit arithmatic. test1", + .insns = { + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_MOV64_IMM(BPF_REG_1, -1), + BPF_ALU64_IMM(BPF_LSH, BPF_REG_1, 32), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 1), + /* r1 = 0xffffFFFF00000001 */ + BPF_JMP32_IMM(BPF_JGT, BPF_REG_1, 1, 3), + /* check ALU64 op keeps 32bit bounds */ + BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 1), + BPF_JMP32_IMM(BPF_JGT, BPF_REG_1, 2, 1), + BPF_JMP_A(1), + /* invalid ldx if bounds are lost above */ + BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_0, -1), + BPF_EXIT_INSN(), + }, + .result = ACCEPT +}, +{ + "bounds check mixed 32bit and 64bit arithmatic. test2", + .insns = { + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_MOV64_IMM(BPF_REG_1, -1), + BPF_ALU64_IMM(BPF_LSH, BPF_REG_1, 32), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 1), + /* r1 = 0xffffFFFF00000001 */ + BPF_MOV64_IMM(BPF_REG_2, 3), + /* r1 = 0x2 */ + BPF_ALU32_IMM(BPF_ADD, BPF_REG_1, 1), + /* check ALU32 op zero extends 64bit bounds */ + BPF_JMP_REG(BPF_JGT, BPF_REG_1, BPF_REG_2, 1), + BPF_JMP_A(1), + /* invalid ldx if bounds are lost above */ + BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_0, -1), + BPF_EXIT_INSN(), + }, + .result = ACCEPT +},
Its possible to have divergent ALU32 and ALU64 bounds when using JMP32 instructins and ALU64 arithmatic operations. Sometimes the clang will even generate this code. Because the case is a bit tricky lets add a specific test for it. Here is pseudocode asm version to illustrate the idea, 1 r0 = 0xffffffff00000001; 2 if w0 > 1 goto %l[fail]; 3 r0 += 1 5 if w0 > 2 goto %l[fail] 6 exit The intent here is the verifier will fail the load if the 32bit bounds are not tracked correctly through ALU64 op. Similarly we can check the 64bit bounds are correctly zero extended after ALU32 ops. 1 r0 = 0xffffffff00000001; 2 w0 += 1 2 if r0 < 0xffffffff00000001 goto %l[fail]; 6 exit The above will fail if we do not correctly zero extend 64bit bounds after 32bit op. Signed-off-by: John Fastabend <john.fastabend@gmail.com> --- tools/testing/selftests/bpf/verifier/bounds.c | 39 +++++++++++++++++++++++++ 1 file changed, 39 insertions(+)