Hi, William, Do you have a test (just compile and load) to demonstrate the problem? I would like to understand why the compiler generates r1+0 and whether we could possibly avoid it.
Thanks, Yonghong On Sat, Apr 28, 2018 at 7:57 AM, William Tu via iovisor-dev <[email protected]> wrote: > Hi, > > We're hitting a BPF verifier error saying > "dereference of modified ctx ptr R1 off=36+0, ctx+const is allowed, > ctx+const+const is not > " > but actually the 2nd const is 0. I don't know why compiler generate (r1+0) > in this case: > 602: (61) r4 = *(u32 *)(r1 +0) > > verifier log > ======== > 594: (15) if r6 == 0x0 goto pc+8 > R0=inv(id=0) R1=inv7161128523638600565 > R3=inv(id=0,umax_value=65535,var_off=(0x0; 0xffff)) R4=inv0 > R6=ctx(id=0,off=0,imm=0) R7=inv2 > R8=map_value(id=0,off=0,ks=232,vs=4352,imm=0) R9=inv(id=0) > R10=fp0,call_-1 fp-352=map_value fp-360=ctx > 595: (bf) r1 = r6 > 596: (07) r1 += 40 > 597: (61) r2 = *(u32 *)(r6 +52) > 598: (b7) r4 = 0 > 599: (1d) if r2 == r4 goto pc+2 > R0=inv(id=0) R1=ctx(id=0,off=40,imm=0) > R2=inv(id=0,umax_value=4294967295,var_off=(0x0; 0xffffffff)) > R3=inv(id=0,umax_value=65535,var_off=(0x0; 0xffff)) R4=inv0 > R6=ctx(id=0,off=0,imm=0) R7=inv2 > R8=map_value(id=0,off=0,ks=232,vs=4352,imm=0) R9=inv(id=0) > R10=fp0,call_-1 fp-352=map_value fp-360=ctx > 600: (bf) r1 = r6 > 601: (07) r1 += 36 ----> > r1 has offset 36 > 602: (61) r4 = *(u32 *)(r1 +0) -----> > then r1 + 0 > dereference of modified ctx ptr R1 off=36+0, ctx+const is allowed, > ctx+const+const is not > > > llvm-objdump > ========== > ; if (!skb) > 594: if r6 == 0 goto 8 > ; return skb->ifindex; > 595: r1 = r6 > 596: r1 += 40 > ; if (skb->cb[OVS_CB_INGRESS]) { > 597: r2 = *(u32 *)(r6 + 52) > 598: r4 = 0 > 599: if r2 == r4 goto 2 > ; return skb->ingress_ifindex; > 600: r1 = r6 > 601: r1 += 36 > > LBB1_28: > 602: r4 = *(u32 *)(r1 + 0) > > > With the below patch fix the issue > ========================== > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -1619,7 +1619,7 @@ static int check_mem_access(struct > bpf_verifier_env *env, int insn_idx, u32 regn > /* ctx accesses must be at a fixed offset, so that we can > * determine what type of data were returned. > */ > - if (reg->off) { > + if (reg->off && reg->off != off) { > verbose(env, > "dereference of modified ctx ptr R%d > off=%d+%d, ctx+const is allowed, ctx+const+const is not\n", > regno, reg->off, off - reg->off); > > > Is this the right fix? > Thanks > William > _______________________________________________ > iovisor-dev mailing list > [email protected] > https://lists.iovisor.org/mailman/listinfo/iovisor-dev _______________________________________________ iovisor-dev mailing list [email protected] https://lists.iovisor.org/mailman/listinfo/iovisor-dev
