Re: [PATCH net] net: check the minimum value of gso size in virtio_net_hdr_to_skb()
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: 2025-07-27 17:16:01
Also in:
lkml, virtualization
Willem de Bruijn wrote:
Wang Liang wrote:quoted
在 2025/7/24 21:29, Willem de Bruijn 写道:quoted
Wang Liang wrote:quoted
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.quoted
return -EINVAL; /* Too small packets are not really GSO ones. */ -- 2.34.1Thanks for your review!Thanks for the dump and repro. I can indeed reproduce, only with the UDP_ENCAP_ESPINUDP setsockopt.quoted
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?