netdev - Re: [PATCH net] net: check the minimum value of gso size in virtio_net_hdr_to

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <08399323-6bab-44f6-bc21-21faf68cd73d@huawei.com>
Date: Mon, 28 Jul 2025 11:36:34 +0800
From: Wang Liang <wangliang74@...wei.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>, <mst@...hat.com>,
	<jasowang@...hat.com>, <xuanzhuo@...ux.alibaba.com>, <eperezma@...hat.com>,
	<pabeni@...hat.com>, <davem@...emloft.net>, <willemb@...gle.com>,
	<atenart@...nel.org>
CC: <yuehaibing@...wei.com>, <zhangchangzhong@...wei.com>,
	<netdev@...r.kernel.org>, <virtualization@...ts.linux.dev>,
	<linux-kernel@...r.kernel.org>, <steffen.klassert@...unet.com>,
	<tobias@...ongswan.org>
Subject: Re: [PATCH net] net: check the minimum value of gso size in
 virtio_net_hdr_to_skb()


在 2025/7/28 1:15, Willem de Bruijn 写道:
> Willem de Bruijn wrote:
>> 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;