On Mon, Jul 28, 2025 at 1:16 AM Willem de Bruijn
[off-list ref] wrote:

Willem de Bruijn wrote:

quoted

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.1

Thanks 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?

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