Re: BPF verifier: ctx+const+const is not allowed


Yonghong Song
 

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
<iovisor-dev@...> 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
iovisor-dev@...
https://lists.iovisor.org/mailman/listinfo/iovisor-dev

Join iovisor-dev@lists.iovisor.org to automatically receive all group messages.