netdev - Re: [PATCH net v2 3/3] virtio-net: correct hdr

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

Message-ID: <1760410698.4630306-3-xuanzhuo@linux.alibaba.com>
Date: Tue, 14 Oct 2025 10:58:18 +0800
From: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
To: Paolo Abeni <pabeni@...hat.com>
Cc: "Michael S. Tsirkin" <mst@...hat.com>,
 Jason Wang <jasowang@...hat.com>,
 Eugenio Pérez <eperezma@...hat.com>,
 Andrew Lunn <andrew+netdev@...n.ch>,
 "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>,
 Willem de Bruijn <willemb@...gle.com>,
 Jiri Pirko <jiri@...nulli.us>,
 Alvaro Karsz <alvaro.karsz@...id-run.com>,
 Heng Qi <hengqi@...ux.alibaba.com>,
 virtualization@...ts.linux.dev,
 netdev@...r.kernel.org
Subject: Re: [PATCH net v2 3/3] virtio-net: correct hdr_len handling for tunnel gso

On Mon, 13 Oct 2025 10:11:31 +0200, Paolo Abeni <pabeni@...hat.com> wrote:
> On 10/13/25 4:06 AM, Xuan Zhuo wrote:
> > The commit a2fb4bc4e2a6a03 ("net: implement virtio helpers to handle UDP
> > GSO tunneling.") introduces support for the UDP GSO tunnel feature in
> > virtio-net.
> >
> > The virtio spec says:
> >
> >     If the \field{gso_type} has the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or
> >     VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit set, \field{hdr_len} accounts for
> >     all the headers up to and including the inner transport.
> >
> > The commit did not update the hdr_len to include the inner transport.
> >
> > Fixes: a2fb4bc4e2a6a03 ("net: implement virtio helpers to handle UDP GSO tunneling.")
> > Signed-off-by: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
>
> Side note: qemu support for UDP GSO tunneling is available in qemu since
> commit a5289563ad.
>
> > ---
> >  include/linux/virtio_net.h | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> >
> > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> > index e059e9c57937..765fd5f471a4 100644
> > --- a/include/linux/virtio_net.h
> > +++ b/include/linux/virtio_net.h
> > @@ -403,6 +403,7 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
> >  	struct virtio_net_hdr *hdr = (struct virtio_net_hdr *)vhdr;
> >  	unsigned int inner_nh, outer_th;
> >  	int tnl_gso_type;
> > +	u16 hdr_len;
> >  	int ret;
> >
> >  	tnl_gso_type = skb_shinfo(skb)->gso_type & (SKB_GSO_UDP_TUNNEL |
> > @@ -434,6 +435,23 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
> >  	outer_th = skb->transport_header - skb_headroom(skb);
> >  	vhdr->inner_nh_offset = cpu_to_le16(inner_nh);
> >  	vhdr->outer_th_offset = cpu_to_le16(outer_th);
> > +
> > +	switch (skb->inner_ipproto) {
> > +	case IPPROTO_TCP:
> > +		hdr_len = inner_tcp_hdrlen(skb);
> > +		break;
> > +
> > +	case IPPROTO_UDP:
> > +		hdr_len = sizeof(struct udphdr);
> > +		break;
> > +
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	hdr_len += skb_inner_transport_offset(skb);
> > +	hdr->hdr_len = __cpu_to_virtio16(little_endian, hdr_len);
>
> I'm not sure this is the correct fix.
>
> virtio_net_hdr_tnl_from_skb() just called virtio_net_hdr_from_skb() on
> the inner header. The virtio spec also specifies:
>
> """
>  If the VIRTIO_NET_F_GUEST_HDRLEN feature has been negotiated,
>  \field{hdr_len} indicates the header length that needs to be replicated
>  for each packet. It's the number of bytes from the beginning of the packet
>  to the beginning of the transport payload.
> """
>
> If `hdr_len` is currently wrong for UDP GSO packets, it's also wrong for
>  plain GSO packets (without UDP tunnel) and the its value should be
> possibly fixed in virtio_net_hdr_from_skb().

YES. This is what the commit #2 does.

Thanks.


>
> Thanks,
>
> Paolo
>