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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <68865ecee5cc4_b1f6a29442@willemb.c.googlers.com.notmuch>
Date: Sun, 27 Jul 2025 13:15:58 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>, 
 Wang Liang <wangliang74@...wei.com>, 
 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()

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?


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ