[PATCH] vhost/net: initialize the whole negotiated RX vnet header

[PATCH] vhost/net: initialize the whole negotiated RX vnet header

[Guikun Yang]

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

Re: [PATCH] vhost/net: initialize the whole negotiated RX vnet header

[Michael S. Tsirkin]

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

Re: [PATCH] vhost/net: initialize the whole negotiated RX vnet header

[Guikun Yang]

On Fri, Oct 23, 2026 at 08:13:22AM -0400, Michael S. Tsirkin wrote:
> 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.

The backend is a tap device attached via the normal tun path:
TUNSETIFF (IFF_TAP|IFF_NO_PI, no IFF_VNET_HDR), then
VHOST_NET_SET_BACKEND on the RX vq.

I re-ran the repro with TUNSETOFFLOAD declared before attaching the
backend, listing every offload bit the interface understands:

	TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
	TUN_F_USO4 | TUN_F_USO6 |
	TUN_F_UDP_TUNNEL_GSO | TUN_F_UDP_TUNNEL_GSO_CSUM

TUNSETOFFLOAD succeeds (0 is returned, so tun accepted all of them,
including the UDP tunnel GSO bits).  The result is unchanged:

	features[0] = 0x108000000, features[1] = 0xa   (tunnel bits on)
	VHOST_SET_FEATURES_ARRAY ok
	TUNSETOFFLOAD ok
	--- RX buffer first 32 bytes (initially all 0xAA) ---
	hdr: 00 00 00 00 00 00 00 00 00 00 01 00
	     aa aa aa aa aa aa aa aa aa aa aa aa     <-- offsets 12..23
	     33 33 00 00 ...

Offsets 12..23 still contain the stale 0xAA fill.

The reason is that vhost_net_set_features() computes vhost_hlen purely
from the feature bits passed via VHOST_SET_FEATURES(_ARRAY):

	hdr_len = ... sizeof(struct virtio_net_hdr_v1_hash_tunnel) when
	           HOST/GUEST_UDP_TUNNEL_GSO is set ...
	if (test_bit(VHOST_NET_F_VIRTIO_NET_HDR))
		vhost_hlen = hdr_len;

TUNSETOFFLOAD only updates tun->set_features and the tap device's
wanted_features; it does not feed back into the vhost feature bits or
into vhost_hlen.  handle_rx() then writes vhost_hlen bytes of the
virtio-net header into the guest buffer while handle_tx() / the GSO
path never touches those 12 bytes for a header vhost itself built.

I could not find TUN_VNET_TNL_SIZE anywhere in 7.3.0-rc7 or in
linux-next (drivers/net/tun.c, drivers/vhost/net.c, include/uapi/linux/
if_tun.h, include/uapi/linux/vhost.h) — is that an interface from a
tree/series I'm missing?  If it is meant to gate vhost_hlen I'd be
happy to re-test with it.

Meanwhile the stale 12 bytes are observable with a plain tun backend
that has declared full tunnel offload support, so I believe the patch
(Zero-initialize the largest header layout and copy vhost_hlen bytes)
is still needed.  Happy to re-test under whatever backend setup you
had in mind.

Best,
Guikun Yang

Re: [PATCH] vhost/net: initialize the whole negotiated RX vnet header

[Michael S. Tsirkin]

On Sat, Oct 24, 2026 at 10:11:01AM +0800, Guikun Yang wrote:
> On Fri, Oct 23, 2026 at 08:13:22AM -0400, Michael S. Tsirkin wrote:
> > 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.
> 
> The backend is a tap device attached via the normal tun path:
> TUNSETIFF (IFF_TAP|IFF_NO_PI, no IFF_VNET_HDR),

Ah, no IFF_VNET_HDR *with* vhost. That's a weird combination.

> then
> VHOST_NET_SET_BACKEND on the RX vq.
> 
> I re-ran the repro with TUNSETOFFLOAD declared before attaching the
> backend, listing every offload bit the interface understands:
> 
> 	TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
> 	TUN_F_USO4 | TUN_F_USO6 |
> 	TUN_F_UDP_TUNNEL_GSO | TUN_F_UDP_TUNNEL_GSO_CSUM
> 
> TUNSETOFFLOAD succeeds (0 is returned, so tun accepted all of them,
> including the UDP tunnel GSO bits).  The result is unchanged:
> 
> 	features[0] = 0x108000000, features[1] = 0xa   (tunnel bits on)
> 	VHOST_SET_FEATURES_ARRAY ok
> 	TUNSETOFFLOAD ok
> 	--- RX buffer first 32 bytes (initially all 0xAA) ---
> 	hdr: 00 00 00 00 00 00 00 00 00 00 01 00
> 	     aa aa aa aa aa aa aa aa aa aa aa aa     <-- offsets 12..23
> 	     33 33 00 00 ...
> 
> Offsets 12..23 still contain the stale 0xAA fill.
> 
> The reason is that vhost_net_set_features() computes vhost_hlen purely
> from the feature bits passed via VHOST_SET_FEATURES(_ARRAY):
> 
> 	hdr_len = ... sizeof(struct virtio_net_hdr_v1_hash_tunnel) when
> 	           HOST/GUEST_UDP_TUNNEL_GSO is set ...
> 	if (test_bit(VHOST_NET_F_VIRTIO_NET_HDR))
> 		vhost_hlen = hdr_len;
> 
> TUNSETOFFLOAD only updates tun->set_features and the tap device's
> wanted_features; it does not feed back into the vhost feature bits or
> into vhost_hlen.  handle_rx() then writes vhost_hlen bytes of the
> virtio-net header into the guest buffer while handle_tx() / the GSO
> path never touches those 12 bytes for a header vhost itself built.
> 
> I could not find TUN_VNET_TNL_SIZE anywhere in 7.3.0-rc7 or in

Apologies. TUN_VNET_TNL_SIZE is an internal macro that header size
is set it. TUNSETVNETHDRSZ is what I meant.

> linux-next (drivers/net/tun.c, drivers/vhost/net.c, include/uapi/linux/
> if_tun.h, include/uapi/linux/vhost.h) — is that an interface from a
> tree/series I'm missing?  If it is meant to gate vhost_hlen I'd be
> happy to re-test with it.
> 
> Meanwhile the stale 12 bytes are observable with a plain tun backend
> that has declared full tunnel offload support, so I believe the patch
> (Zero-initialize the largest header layout and copy vhost_hlen bytes)
> is still needed.  Happy to re-test under whatever backend setup you
> had in mind.
> 
> Best,
> Guikun Yang


I have in mind simply disabling this combo: vhost + !IFF_VNET_HDR)

I'll research a bit whether anything anywhere could reasonably
use it.

Can you reproduce any problem *with* IFF_VNET_HDR?

-- 
MST

Re: [PATCH] vhost/net: initialize the whole negotiated RX vnet header

[Guikun Yang]

On Sat, Oct 24, 2026 at 02:45:39AM -0400, Michael S. Tsirkin wrote:
> Ah, no IFF_VNET_HDR *with* vhost. That's a weird combination.
>
> Apologies. TUN_VNET_TNL_SIZE is an internal macro that header size
> is set it. TUNSETVNETHDRSZ is what I meant.
>
> I have in mind simply disabling this combo: vhost + !IFF_VNET_HDR)
>
> I'll research a bit whether anything anywhere could reasonably
> use it.
>
> Can you reproduce any problem *with* IFF_VNET_HDR?

Yes.  I added IFF_VNET_HDR + TUNSETVNETHDRSZ(24) to the repro
(in addition to VHOST_SET_FEATURES_ARRAY with the tunnel GSO bits
and VHOST_NET_F_VIRTIO_NET_HDR):

	tap ifindex = 20482
	TUNSETVNETHDRSZ(24) ok
	tap attached to RX vq

	--- RX buffer first 32 bytes (initially all 0xAA) ---
	hdr: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
	     aa aa aa aa aa aa aa aa 00 00 00 00 00 00 00 00

The entire 24-byte vnet header area is stale 0xAA fill — including
offsets 0..9 where vhost is supposed to write the virtio_net_hdr.
The payload (from the host) starts at offset 24.

So the uninitialized vnet header is observable in both configurations:
with and without IFF_VNET_HDR on the tun device.

Note that with IFF_VNET_HDR enabled, the RX buffer is also corrupted
in a different way: the tun device prepends its own 24-byte vnet
header to the data it sends to vhost, and handle_rx() with
sock_hlen = 0 does not skip it.  So even if the virtio_net_hdr
were properly written, the payload delivered to the guest would
be shifted by 24 bytes.

Best,
Guikun Yang

Re: [PATCH] vhost/net: initialize the whole negotiated RX vnet header

[Guikun Yang]

First, a correction to my previous message: the AF_PACKET injection in
that run never actually worked.  I took the "ifindex" from the ifreq
returned by TUNSETIFF, but TUNSETIFF fills in ifr_flags there (the
values 4098/20482 I printed were BROADCAST|MULTICAST|VNET_HDR flag
bits), so every sendto failed with ENODEV and no packet was ever
injected.  The all-0xAA buffer in that run only shows that no packet
was processed at all, and the "payload starts at offset 24" claim was
wrong.  Sorry for the noise.

I fixed the injection (SIOCGIFINDEX by name) and re-ran both
configurations on 7.3.0-rc7 with a 64-byte broadcast frame injected on
the tap.  With IFF_VNET_HDR enabled (plus TUNSETVNETHDRSZ(24)) the
result is worse than stale bytes: the packet is consumed from the tun
queue and then silently discarded, so the guest sees nothing at all:

  tunnel mode (no IFF_VNET_HDR), sendto 64 bytes ok:
	hdr: 00 00 00 00 00 00 00 00 00 00 01 00
	     aa aa aa aa aa aa aa aa aa aa aa aa   <- stale 12..23
	     ff ff ff ff ff ff 02 02 ...           <- our frame at 24

  tunnel-vnet-hdr mode (IFF_VNET_HDR | TUNSETVNETHDRSZ(24)),
  same 64-byte sendto ok:
	hdr: aa aa aa aa ... (all 32 bytes untouched)

The guest buffer is never touched in the second run.  The packet was
read out of the tun queue and then dropped.

The reason is a peek/read size disagreement:

  - peek_head_len() returns head->len, i.e. the raw skb->len.  The tun
    vnet header is not in the skb; tun_put_user() adds vnet_hdr_sz
    bytes at read time.
  - With VHOST_NET_F_VIRTIO_NET_HDR negotiated, sock_hlen = 0, so
    handle_rx() computes sock_len = peek = skb->len (64) and calls
    recvmsg(sock, &msg, 64, ...) with a msg_iter sized for
    vhost_len = 64 + vhost_hlen(24) = 88.
  - tun_do_read() delivers skb->len + vnet_hdr_sz = 64 + 24 = 88 bytes
    and returns 88.
  - handle_rx() then sees err (88) != sock_len (64) and takes the
    "Discarded rx packet" path: vhost_discard_vq_desc() rolls back the
    descriptor, but the skb has already been dequeued from the tun
    receive queue.  The frame is gone.

So for a tun backend, the three combinations are:

  VHOST_NET_F_VIRTIO_NET_HDR | IFF_VNET_HDR | result
  ---------------------------+--------------+--------------------------
  off                        | on           | ok (classic: tun header
                             |              | passed through)
  on                         | off          | header written by vhost,
                             |              | bytes 12..23 stale (the
                             |              | patch under discussion)
  on                         | on           | every packet consumed and
                             |              | dropped, guest RX blackout

VHOST_NET_F_VIRTIO_NET_HDR means "vhost supplies the header, the socket
data has none", so it is only consistent with a backend whose
vnet_hdr_sz is 0.  When the backend has vnet_hdr_sz > 0 the peek and
the read disagree and handle_rx() eats every frame.

Given that, I don't think disabling "vhost + !IFF_VNET_HDR" is the
right fix: that is exactly the combination VHOST_NET_F_VIRTIO_NET_HDR
is for, and it works apart from the 12 stale bytes my patch addresses.
The combination that is plainly broken is "vhost supplies the header"
together with a backend that supplies one (IFF_VNET_HDR set), which
today results in a silent total RX blackout.

So I'd suggest one of:

  1. Reject VHOST_NET_F_VIRTIO_NET_HDR (or refuse the backend attach)
     when the backend's vnet_hdr_sz is nonzero, so both broken
     combinations become impossible; keep my patch for the stale bytes
     in the remaining valid combination.  The backend header size could
     be queried at attach time the same way TUNSETIFF-derived sockets
     are handled today.

  2. Keep all combinations and fix both symptoms: my patch for the
     stale bytes, plus making the peek path account for the backend
     header so peek and read agree.

Happy to test whichever you prefer.

Best,
Guikun Yang