Hi, Simon,

Finally got some time to deep dive into the verifier for this issue.
I filed an issue https://github.com/iovisor/bcc/issues/2463 with
possible suggestion to add a new checksum calculation helper
for xdp. I forgot what is your ultimate workaround for this issue.
Could you comment on the issue?

Thanks!

Yonghong

On Thu, Mar 7, 2019 at 9:37 AM Y Song <[email protected]> wrote:
>
> On Wed, Mar 6, 2019 at 7:08 AM <[email protected]> wrote:
> >
> > I'm playing with bcc to prototype an UDP load balancer.
> >
> > I'm facing an issue that I didn't succeed to understand...
> >
> > In my code I tried to validate my UDP packet using code like this :
> >
> >     struct udphdr *udp;
> >     udp = iph + 1;
> >     if (udp + 1 > data_end)
> >         return XDP_DROP;
> >     __u16 udp_len = bpf_ntohs(udp->len);
> >     //__u16 udp_len = 8;
> >     if (udp_len < 8)
> >         return XDP_DROP;
> >     if (udp_len > 512) // TODO use a more approriate max value
> >         return XDP_DROP;
> >     if ((void *) udp + udp_len > data_end)
> >         return XDP_DROP;
> >
> > And the verifier does not like it ..
>
> This is caused by compiler optimizations.
>
> >
> > 28: (71) r2 = *(u8 *)(r7 +23)
> > 29: (b7) r0 = 2
> > 30: (55) if r2 != 0x11 goto pc+334
> >  R0=inv2 R1=pkt_end(id=0,off=0,imm=0) R2=inv17 R3=inv5 
> > R6=ctx(id=0,off=0,imm=0) R7=pkt(id=0,off=0,r=34,imm=0) 
> > R8=pkt(id=0,off=34,r=34,imm=0) R9=pkt(id=0,off=14,r=34,imm=0) 
> > R10=fp0,call_-1
> > 31: (bf) r2 = r8
> > 32: (07) r2 += 8
> > 33: (b7) r0 = 1
> > 34: (2d) if r2 > r1 goto pc+330
> >  R0=inv1 R1=pkt_end(id=0,off=0,imm=0) R2=pkt(id=0,off=42,r=42,imm=0) 
> > R3=inv5 R6=ctx(id=0,off=0,imm=0) R7=pkt(id=0,off=0,r=42,imm=0) 
> > R8=pkt(id=0,off=34,r=42,imm=0) R9=pkt(id=0,off=14,r=42,imm=0) 
> > R10=fp0,call_-1
> > 35: (69) r3 = *(u16 *)(r7 +38)
> > 36: (dc) r3 = be16 r3
>
> r3 get the value from memory, its value could be any one as permitted
> by the type.
>
> > 37: (bf) r2 = r3
> > 38: (07) r2 += -8
> > 39: (57) r2 &= 65535
> > 40: (b7) r0 = 1
> > 41: (25) if r2 > 0x1f8 goto pc+323
>
> test is done by r2. We indeed get better range for r2 (below:
> R2=inv(id=0,umax_value=504,var_off=(0x0; 0x1ff)) )
> but r3 range is not tightened.
>
> >  R0=inv1 R1=pkt_end(id=0,off=0,imm=0) 
> > R2=inv(id=0,umax_value=504,var_off=(0x0; 0x1ff)) R3=inv(id=0) 
> > R6=ctx(id=0,off=0,imm=0) R7=pkt(id=0,off=0,r=42,imm=0) 
> > R8=pkt(id=0,off=34,r=42,imm=0) R9=pkt(id=0,off=14,r=42,imm=0) 
> > R10=fp0,call_-1
> > 42: (bf) r2 = r7
> > 43: (0f) r2 += r3
> > math between pkt pointer and register with unbounded min value is not 
> > allowed
>
> Here, we use r3 to do arith and the verifier not happy. If we use the
> tightened old r2, it may work.
> The compiler does the right thing, just verifier is not advanced enough.
>
> >
> > I'm pretty sure the issue is about udp_len, that's why I tried to validate 
> > its value before to use it ... but without success...
> > When I set udp_len to 8 (just for testing) this seems to works. Any idea 
> > about that ?
>
> Yes, you will need some source workaround. You could try below (untested):
>
>     if (udp_len > 512) // TODO use a more approriate max value
>           return XDP_DROP;
> +  udp_len = udp_len & 0x1ff;
>     if ((void *) udp + udp_len > data_end)
>          return XDP_DROP;
>
> >
> > Full code is available here : 
> > https://gist.github.com/sbernard31/d4fee7518a1ff130452211c0d355b3f7
> >
> > (I'm using python-bpfcc  0.8.0-4 from debian sid with a 4.19.12 kernel)
> > (I don't know if this is the right place for this kind of question, )
> >
> > 

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#1735): https://lists.iovisor.org/g/iovisor-dev/message/1735
Mute This Topic: https://lists.iovisor.org/mt/30285987/21656
Mute #verifier: https://lists.iovisor.org/mk?hashtag=verifier&subid=2590197
Group Owner: [email protected]
Unsubscribe: https://lists.iovisor.org/g/iovisor-dev/unsub  
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to