This patch adds a selftest for the change in the previous patch. The selftest is derived from a syzbot reproducer from [1] (among the 22 reproducers on that page, only 4 still reproduced on latest bpf tree, all being small variants of the same invariant violation). The test case failure without the previous patch is shown below. 0: R1=ctx() R10=fp0 0: (85) call bpf_get_prandom_u32#7 ; R0=scalar() 1: (bf) r5 = r0 ; R0=scalar(id=1) R5=scalar(id=1) 2: (57) r5 &= -4 ; R5=scalar(smax=0x7ffffffffffffffc,umax=0xfffffffffffffffc,smax32=0x7ffffffc,umax32=0xfffffffc,var_off=(0x0; 0xfffffffffffffffc)) 3: (bf) r7 = r0 ; R0=scalar(id=1) R7=scalar(id=1) 4: (57) r7 &= 1 ; R7=scalar(smin=smin32=0,smax=umax=smax32=umax32=1,var_off=(0x0; 0x1)) 5: (07) r7 += -43 ; R7=scalar(smin=smin32=-43,smax=smax32=-42,umin=0xffffffffffffffd5,umax=0xffffffffffffffd6,umin32=0xffffffd5,umax32=0xffffffd6,var_off=(0xffffffffffffffd4; 0x3)) 6: (5e) if w5 != w7 goto pc+1 verifier bug: REG INVARIANTS VIOLATION (false_reg1): range bounds violation u64=[0xffffffd5, 0xffffffffffffffd4] s64=[0x80000000ffffffd5, 0x7fffffffffffffd4] u32=[0xffffffd5, 0xffffffd4] s32=[0xffffffd5, 0xffffffd4] var_off=(0xffffffd4, 0xffffffff00000000) R5 and R7 are prepared such that their tnums intersection results in a known constant but that constant isn't within R7's u32 bounds. is_branch_taken isn't able to detect this case today, so the verifier walks the impossible fallthrough branch. After regs_refine_cond_op and reg_bounds_sync refine R5 on the assumption that the branch is taken, the impossibility becomes apparent and results in an invariant violation for R5: umin32 is greater than umax32. The previous patch fixes this by using regs_refine_cond_op and reg_bounds_sync in is_branch_taken to detect the impossible branch. The fallthrough branch is therefore correctly detected as dead code. Link: https://syzkaller.appspot.com/bug?extid=c950cc277150935cc0b5 [1] Signed-off-by: Paul Chaignon --- .../selftests/bpf/progs/verifier_bounds.c | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tools/testing/selftests/bpf/progs/verifier_bounds.c b/tools/testing/selftests/bpf/progs/verifier_bounds.c index 3724d5e5bcb3..818efa08404d 100644 --- a/tools/testing/selftests/bpf/progs/verifier_bounds.c +++ b/tools/testing/selftests/bpf/progs/verifier_bounds.c @@ -2070,4 +2070,28 @@ __naked void refinement_32bounds_not_overwriting_64bounds(void *ctx) : __clobber_all); } +/* Last jump can be detected as always taken because the intersection of R5 and + * R7 32bit tnums produces a constant that isn't within R7's s32 bounds. + */ +SEC("socket") +__description("dead branch: tnums give impossible constant if equal") +__success +__flag(BPF_F_TEST_REG_INVARIANTS) +__naked void tnums_equal_impossible_constant(void *ctx) +{ + asm volatile(" \ + call %[bpf_get_prandom_u32]; \ + r5 = r0; \ + r5 &= 0xfffffffffffffffc; /* var_off32=(0; 0xfffffffc) */ \ + r7 = r0; \ + r7 &= 0x1; /* var_off32=(0x0; 0x1) */ \ + r7 += -43; /* s32=[-43; -42] & var_off32=(0xffffffd4; 0x3) */ \ + if w5 != w7 goto +1; /* on fallthrough var_off32=-44, not in s32 */ \ + r10 = 0; \ + exit; \ +" : + : __imm(bpf_get_prandom_u32) + : __clobber_all); +} + char _license[] SEC("license") = "GPL"; -- 2.43.0