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

Message-ID: <27931e85-451f-4711-9681-38db2563efc2@redhat.com>
Date: Mon, 13 Oct 2025 10:11:31 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, netdev@...r.kernel.org
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
Subject: Re: [PATCH net v2 3/3] virtio-net: correct hdr_len handling for
 tunnel gso

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().

Thanks,

Paolo