On Sun, Jul 27, 2025 at 11:36 PM Wang Liang [off-list ref] wrote:

在 2025/7/28 1:15, Willem de Bruijn 写道:

quoted

Willem de Bruijn wrote:

quoted

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?

Thank you for your analysis, it is totally right!

After segment in udp_queue_rcv_skb(), these segs are not UDP packets, which
leads the crash.

The packet with VIRTIO_NET_HDR_GSO_UDP type from tun device perhaps should
not enter udp_rcv_segment().

How about not do segment and pass the packet to udp_queue_rcv_one_skb()
directly, like this:

   diff --git a/include/linux/udp.h b/include/linux/udp.h
   index 4e1a672af4c5..0c27e5194657 100644
   --- a/include/linux/udp.h
   +++ b/include/linux/udp.h
   @@ -186,6 +186,9 @@ static inline bool udp_unexpected_gso(struct sock
*sk, struct sk_buff *skb)
           if (!skb_is_gso(skb))
                   return false;

   +       if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP)
   +               return false;
   +
           if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4 &&
               !udp_test_bit(ACCEPT_L4, sk))
                   return true;

That seemingly is what would have happened before introducing
udp_unexpected_gso. The same for SKB_GSO_UDP_L4 packets actually.

It is not in any way an intended use case. And given the longstanding
breakage clearly not actually used.

UDP sockets do not expect GSO packets, so queuing them might confuse them.

Specifically for UFO, it can be argued that they are the same as large
packets that were build by the IP defrag layer.

Still to avoid having to start maintaining an unintentional code path,
including having to likely fix more reports over time and with that
likely add complexity to the real UDP hot path as well, I think we
should instead drop these.