On Mon, Jul 28, 2025 at 1:16 AM Willem de Bruijn <willemdebruijn.kernel@xxxxxxxxx> wrote: > > Willem de Bruijn wrote: > > Wang Liang wrote: > > > > > > 在 2025/7/24 21:29, Willem de Bruijn 写道: > > > > Wang Liang wrote: > > > >> When sending a packet with virtio_net_hdr to tun device, if the gso_type > > > >> in virtio_net_hdr is SKB_GSO_UDP and the gso_size is less than udphdr > > > >> size, below crash may happen. > > > >> > > > > gso_size is the size of the segment payload, excluding the transport > > > > header. > > > > > > > > This is probably not the right approach. > > > > > > > > Not sure how a GSO skb can be built that is shorter than even the > > > > transport header. Maybe an skb_dump of the GSO skb can be elucidating. > > > >> return -EINVAL; > > > >> > > > >> /* Too small packets are not really GSO ones. */ > > > >> -- > > > >> 2.34.1 > > > >> > > > > > > Thanks for your review! > > > > Thanks for the dump and repro. > > > > I can indeed reproduce, only with the UDP_ENCAP_ESPINUDP setsockopt. > > > > > Here is the skb_dump result: > > > > > > skb len=4 headroom=98 headlen=4 tailroom=282 > > > mac=(64,14) mac_len=14 net=(78,20) trans=98 > > > shinfo(txflags=0 nr_frags=0 gso(size=0 type=0 segs=0)) > > > csum(0x8c start=140 offset=0 ip_summed=1 complete_sw=0 valid=1 level=0) > > > > So this is as expected not the original GSO skb, but a segment, > > after udp_rcv_segment from udp_queue_rcv_skb. > > > > It is a packet with skb->data pointing to the transport header, and > > only 4B length. So this is an illegal UDP packet with length shorter > > than sizeof(struct udphdr). > > > > The packet does not enter xfrm4_gro_udp_encap_rcv, so we can exclude > > that. > > > > It does enter __xfrm4_udp_encap_rcv, which will return 1 because the > > pskb_may_pull will fail. There is a negative integer overflow just > > before that: > > > > len = skb->len - sizeof(struct udphdr); > > if (!pskb_may_pull(skb, sizeof(struct udphdr) + min(len, 8))) > > return 1; > > > > This is true for all the segments btw, not just the last one. On > > return of 1 here, the packet does not enter encap_rcv but gets > > passed to the socket as a normal UDP packet: > > > > /* If it's a keepalive packet, then just eat it. > > * If it's an encapsulated packet, then pass it to the > > * IPsec xfrm input. > > * Returns 0 if skb passed to xfrm or was dropped. > > * Returns >0 if skb should be passed to UDP. > > * Returns <0 if skb should be resubmitted (-ret is protocol) > > */ > > int xfrm4_udp_encap_rcv(struct sock *sk, struct sk_buff *skb) > > > > But so the real bug, an skb with 4B in the UDP layer happens before > > that. > > > > An skb_dump in udp_queue_rcv_skb of the GSO skb shows > > > > [ 174.151409] skb len=190 headroom=64 headlen=190 tailroom=66 > > [ 174.151409] mac=(64,14) mac_len=14 net=(78,20) trans=98 > > [ 174.151409] shinfo(txflags=0 nr_frags=0 gso(size=4 type=65538 segs=0)) > > [ 174.151409] csum(0x8c start=140 offset=0 ip_summed=3 complete_sw=0 valid=1 level=0) > > [ 174.151409] hash(0x0 sw=0 l4=0) proto=0x0800 pkttype=2 iif=8 > > [ 174.151409] priority=0x0 mark=0x0 alloc_cpu=1 vlan_all=0x0 > > [ 174.151409] encapsulation=0 inner(proto=0x0000, mac=0, net=0, trans=0) > > [ 174.152101] dev name=tun0 feat=0x00002000000048c1 > > > > And of segs[0] after segmentation > > > > [ 103.081442] skb len=38 headroom=64 headlen=38 tailroom=218 > > [ 103.081442] mac=(64,14) mac_len=14 net=(78,20) trans=98 > > [ 103.081442] shinfo(txflags=0 nr_frags=0 gso(size=0 type=0 segs=0)) > > [ 103.081442] csum(0x8c start=140 offset=0 ip_summed=1 complete_sw=0 valid=1 level=0) > > [ 103.081442] hash(0x0 sw=0 l4=0) proto=0x0800 pkttype=2 iif=8 > > [ 103.081442] priority=0x0 mark=0x0 alloc_cpu=0 vlan_all=0x0 > > [ 103.081442] encapsulation=0 inner(proto=0x0000, mac=0, net=0, trans=0) > > > > So here translen is already 38 - (98-64) == 38 - 34 == 4. > > > > So the bug happens in segmentation. > > > > [ongoing ..] > > Oh of course, this is udp fragmentation offload (UFO): > VIRTIO_NET_HDR_GSO_UDP. > > So only the first packet has an UDP header, and that explains why the > other packets are only 4B. > > They are not UDP packets, but they have already entered the UDP stack > due to this being GSO applied in udp_queue_rcv_skb. > > That was never intended to be used for UFO. Only for GRO, which does > not build such packets. > > Maybe we should just drop UFO (SKB_GSO_UDP) packets in this code path? > Just to make sure I understand this. Did you mean to disable UFO for guest -> host path? If yes, it seems can break some appllication. Thanks
