netdev - Re: XDP breakage with virtio due to 6870de435b90c083ae0f3f7f341287976ef56f03

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Sun, 22 Apr 2018 17:55:03 -0700
From:   "Nikita V. Shirokov" <tehnerd@...nerd.com>
To:     David Ahern <dsahern@...il.com>
Cc:     Jason Wang <jasowang@...hat.com>, daniel@...earbox.net,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: XDP breakage with virtio due to
 6870de435b90c083ae0f3f7f341287976ef56f03

On Sun, Apr 22, 2018 at 04:47:48PM -0600, David Ahern wrote:
> This commit breaks my FIB forwarding program:
> 
> commit 6870de435b90c083ae0f3f7f341287976ef56f03
> Author: Nikita V. Shirokov <tehnerd@...nerd.com>
> Date:   Tue Apr 17 21:42:20 2018 -0700
> 
>     bpf: make virtio compatible w/ bpf_xdp_adjust_tail
> 
>     w/ bpf_xdp_adjust_tail helper xdp's data_end pointer could be changed as
>     well (only "decrease" of pointer's location is going to be supported).
>     changing of this pointer will change packet's size.
>     for virtio driver we need to adjust XDP_PASS handling by recalculating
>     length of the packet if it was passed to the TCP/IP stack
> 
>     Reviewed-by: Jason Wang <jasowang@...hat.com>
>     Signed-off-by: Nikita V. Shirokov <tehnerd@...nerd.com>
>     Signed-off-by: Daniel Borkmann <daniel@...earbox.net>
> 
> ###
> 
> Some of the packets (e.g., ARP or those without a resolved neighbor) are
> passed to the networking stack. What shows up are clearly broken packets:
> 
> # tcpdump -n -i eth1
> tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
> listening on eth1, link-type EN10MB (Ethernet), capture size 262144 bytes
> 15:45:29.693238 [|ARP]
> 	0x0000:  0001 0800 0604 0001 42c0 ce2f 3fa9 0a64  ........B../?..d
> 15:45:30.710327 [|ARP]
> 	0x0000:  0001 0800 0604 0001 42c0 ce2f 3fa9 0a64  ........B../?..d
> 15:45:31.734296 [|ARP]
> 	0x0000:  0001 0800 0604 0001 42c0 ce2f 3fa9 0a64  ........B../?..d
> 15:45:32.908720 IP6 truncated-ip6 - 12 bytes
> missing!fe80::40c0:ceff:fe2f:3fa9 > ff02::1:ff00:2: ICMP6, neighbor
> solicitation[|icmp6]
> 15:45:33.910530 IP6 truncated-ip6 - 12 bytes missing!2001:db8:1::64 >
> ff02::1:ff00:2: ICMP6, neighbor solicitation[|icmp6]
> 15:45:34.934437 IP6 truncated-ip6 - 12 bytes missing!2001:db8:1::64 >
> ff02::1:ff00:2: ICMP6, neighbor solicitation[|icmp6]
> 15:45:35.958394 IP6 truncated-ip6 - 12 bytes missing!2001:db8:1::64 >
> ff02::1:ff00:2: ICMP6, neighbor solicitation[|icmp6]
> 
> Reverting the mentioned patch fixes the problem.
Hi, David.
thanks for reporting this. looks like in my calculation i've missed
vi->hdr_len during new lengths calculation (it was len = xdp->data_end -
xdp->data; but also shouldbe +vi->hdr_len). will run few more tests
before sending a fix.

--
Nikita