mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] vhost/net: initialize the whole negotiated RX vnet header
@ 2026-10-23 11:37 Guikun Yang
  2026-10-23 12:13 ` Michael S. Tsirkin
  0 siblings, 1 reply; 2+ messages in thread
From: Guikun Yang @ 2026-10-23 11:37 UTC (permalink / raw)
  To: Michael S. Tsirkin, Jason Wang, Eugenio Perez
  Cc: kvm, virtualization, netdev, linux-kernel

When VHOST_NET_F_VIRTIO_NET_HDR is negotiated, handle_rx() builds the
virtio-net header itself and writes it into the guest buffer.  Since
commit bbca931fce26 ("vhost/net: enable gso over UDP tunnel support.")
vhost_net_set_features() raises vhost_hlen to
sizeof(struct virtio_net_hdr_v1_hash_tunnel) as soon as one of the UDP
tunnel GSO features is enabled, but handle_rx() still copies only
sizeof(struct virtio_net_hdr) (10 bytes) and then writes num_buffers at
offset 10.

The remaining 12 bytes of the 24 byte header (hash_value_lo,
hash_value_hi, hash_report, padding, outer_th_offset and
inner_nh_offset) are therefore never initialized, and the guest sees
whatever its RX buffer contained before in fields whose layout the
negotiated feature set defines.

Reproduced on 7.3.0-rc7 with a userspace vhost-net "guest" that fills its
RX buffer with 0xaa and negotiates VHOST_NET_F_VIRTIO_NET_HDR,
VIRTIO_F_VERSION_1 and VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO: offsets 0..9
hold the vnet header, num_buffers is at 10..11, the payload correctly
starts at offset 24, and offsets 12..23 still contain 0xaa.  Without the
tunnel features the payload starts at offset 12 and nothing is left
uninitialized.

Copy the whole vhost_hlen out of a zero initialized header of the
largest layout, and rewind the iterator so that num_buffers keeps its
position right after the legacy header.

Fixes: bbca931fce26 ("vhost/net: enable gso over UDP tunnel support.")
Signed-off-by: Guikun Yang <yangguikun19@qq.com>
---
 drivers/vhost/net.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -1161,9 +1161,15 @@ static void handle_rx(struct vhost_net *net)
 		.msg_controllen = 0,
 		.msg_flags = MSG_DONTWAIT,
 	};
-	struct virtio_net_hdr hdr = {
-		.flags = 0,
-		.gso_type = VIRTIO_NET_HDR_GSO_NONE
+	/*
+	 * The negotiated header is struct virtio_net_hdr when only the legacy
+	 * layout is used, but struct virtio_net_hdr_v1_hash_tunnel (or the
+	 * v1/v1_hash layouts) when the corresponding features are enabled:
+	 * size it for the largest layout and let vhost_hlen select how much
+	 * of it is sent.
+	 */
+	struct virtio_net_hdr_v1_hash_tunnel hdr = {
+		.hash_hdr.hdr.gso_type = VIRTIO_NET_HDR_GSO_NONE,
 	};
 	size_t total_len = 0;
 	int err, mergeable;
@@ -1260,17 +1266,26 @@ static void handle_rx(struct vhost_net *net)
 		}
 		/* Supply virtio_net_hdr if VHOST_NET_F_VIRTIO_NET_HDR */
 		if (unlikely(vhost_hlen)) {
-			if (copy_to_iter(&hdr, sizeof(hdr),
-					 &fixup) != sizeof(hdr)) {
+			/*
+			 * vhost_hlen is never larger than the header above.
+			 * Copy all of it: the fields vhost does not provide
+			 * (hash and UDP tunnel information) have to be zeroed
+			 * instead of being left with the previous content of
+			 * the guest buffer.
+			 */
+			if (copy_to_iter(&hdr, vhost_hlen, &fixup) != vhost_hlen) {
 				vq_err(vq, "Unable to write vnet_hdr "
 				       "at addr %p\n", vq->iov->iov_base);
 				goto out;
 			}
+			/* num_buffers follows the legacy header */
+			iov_iter_revert(&fixup,
+					vhost_hlen - sizeof(struct virtio_net_hdr));
 		} else {
 			/* Header came from socket; we'll need to patch
 			 * ->num_buffers over if VIRTIO_NET_F_MRG_RXBUF
 			 */
-			iov_iter_advance(&fixup, sizeof(hdr));
+			iov_iter_advance(&fixup, sizeof(struct virtio_net_hdr));
 		}
 		/* TODO: Should check and handle checksum. */
 
--
2.39.5


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] vhost/net: initialize the whole negotiated RX vnet header
  2026-10-23 11:37 [PATCH] vhost/net: initialize the whole negotiated RX vnet header Guikun Yang
@ 2026-10-23 12:13 ` Michael S. Tsirkin
  0 siblings, 0 replies; 2+ messages in thread
From: Michael S. Tsirkin @ 2026-10-23 12:13 UTC (permalink / raw)
  To: Guikun Yang
  Cc: Jason Wang, Eugenio Perez, kvm, virtualization, netdev, linux-kernel

On Fri, Oct 23, 2026 at 07:37:20PM +0800, Guikun Yang wrote:
> When VHOST_NET_F_VIRTIO_NET_HDR is negotiated, handle_rx() builds the
> virtio-net header itself and writes it into the guest buffer.  Since
> commit bbca931fce26 ("vhost/net: enable gso over UDP tunnel support.")
> vhost_net_set_features() raises vhost_hlen to
> sizeof(struct virtio_net_hdr_v1_hash_tunnel) as soon as one of the UDP
> tunnel GSO features is enabled, but handle_rx() still copies only
> sizeof(struct virtio_net_hdr) (10 bytes) and then writes num_buffers at
> offset 10.
> 
> The remaining 12 bytes of the 24 byte header (hash_value_lo,
> hash_value_hi, hash_report, padding, outer_th_offset and
> inner_nh_offset) are therefore never initialized, and the guest sees
> whatever its RX buffer contained before in fields whose layout the
> negotiated feature set defines.
> 
> Reproduced on 7.3.0-rc7 with a userspace vhost-net "guest" that fills its
> RX buffer with 0xaa and negotiates VHOST_NET_F_VIRTIO_NET_HDR,
> VIRTIO_F_VERSION_1 and VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO: offsets 0..9
> hold the vnet header, num_buffers is at 10..11, the payload correctly
> starts at offset 24, and offsets 12..23 still contain 0xaa.  Without the
> tunnel features the payload starts at offset 12 and nothing is left
> uninitialized.
> 
> Copy the whole vhost_hlen out of a zero initialized header of the
> largest layout, and rewind the iterator so that num_buffers keeps its
> position right after the legacy header.
> 
> Fixes: bbca931fce26 ("vhost/net: enable gso over UDP tunnel support.")
> Signed-off-by: Guikun Yang <yangguikun19@qq.com>


Which backend did you use? Sounds like you did not set
TUN_VNET_TNL_SIZE or did not set TUNSETOFFLOAD?
That's the issue then.


> ---
>  drivers/vhost/net.c | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -1161,9 +1161,15 @@ static void handle_rx(struct vhost_net *net)
>  		.msg_controllen = 0,
>  		.msg_flags = MSG_DONTWAIT,
>  	};
> -	struct virtio_net_hdr hdr = {
> -		.flags = 0,
> -		.gso_type = VIRTIO_NET_HDR_GSO_NONE
> +	/*
> +	 * The negotiated header is struct virtio_net_hdr when only the legacy
> +	 * layout is used, but struct virtio_net_hdr_v1_hash_tunnel (or the
> +	 * v1/v1_hash layouts) when the corresponding features are enabled:
> +	 * size it for the largest layout and let vhost_hlen select how much
> +	 * of it is sent.
> +	 */
> +	struct virtio_net_hdr_v1_hash_tunnel hdr = {
> +		.hash_hdr.hdr.gso_type = VIRTIO_NET_HDR_GSO_NONE,
>  	};
>  	size_t total_len = 0;
>  	int err, mergeable;
> @@ -1260,17 +1266,26 @@ static void handle_rx(struct vhost_net *net)
>  		}
>  		/* Supply virtio_net_hdr if VHOST_NET_F_VIRTIO_NET_HDR */
>  		if (unlikely(vhost_hlen)) {
> -			if (copy_to_iter(&hdr, sizeof(hdr),
> -					 &fixup) != sizeof(hdr)) {
> +			/*
> +			 * vhost_hlen is never larger than the header above.
> +			 * Copy all of it: the fields vhost does not provide
> +			 * (hash and UDP tunnel information) have to be zeroed
> +			 * instead of being left with the previous content of
> +			 * the guest buffer.
> +			 */
> +			if (copy_to_iter(&hdr, vhost_hlen, &fixup) != vhost_hlen) {
>  				vq_err(vq, "Unable to write vnet_hdr "
>  				       "at addr %p\n", vq->iov->iov_base);
>  				goto out;
>  			}
> +			/* num_buffers follows the legacy header */
> +			iov_iter_revert(&fixup,
> +					vhost_hlen - sizeof(struct virtio_net_hdr));
>  		} else {
>  			/* Header came from socket; we'll need to patch
>  			 * ->num_buffers over if VIRTIO_NET_F_MRG_RXBUF
>  			 */
> -			iov_iter_advance(&fixup, sizeof(hdr));
> +			iov_iter_advance(&fixup, sizeof(struct virtio_net_hdr));
>  		}
>  		/* TODO: Should check and handle checksum. */
>  
> --
> 2.39.5


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-10-23 12:13 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-23 11:37 [PATCH] vhost/net: initialize the whole negotiated RX vnet header Guikun Yang
2026-10-23 12:13 ` Michael S. Tsirkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®