[PATCH net 0/2] vsock/virtio: fix vsockmon tap skb construction

[PATCH net 0/2] vsock/virtio: fix vsockmon tap skb construction

[Stefano Garzarella]

While reviewing the patch posted by Yiqi Sun [1] to fix an issue in
virtio_transport_build_skb(), I discovered another issue related to
the offset and length of the payload to be copied in the new skb.
This was introduced when we did the skb conversion, and fixed by
patch 1.

Patch 2 fixes the issue found by Yiqi Sun in a different way: using
iov_iter_kvec() to properly initialize all the iov_iter fields and
removing the linear vs non-linear split like we alredy do in
vhost-vsock.

It could have been a single patch, but since there were two affected
commits, I decided to keep the fixes separate.

[1] https://lore.kernel.org/netdev/20260430071110.380509-1-sunyiqixm@gmail.com/

Stefano Garzarella (2):
  vsock/virtio: fix length and offset in tap skb for split packets
  vsock/virtio: fix empty payload in tap skb for non-linear buffers

 net/vmw_vsock/virtio_transport_common.c | 47 +++++++++----------------
 1 file changed, 16 insertions(+), 31 deletions(-)

-- 
2.54.0

[PATCH net 1/2] vsock/virtio: fix length and offset in tap skb for split packets

[Stefano Garzarella]

From: Stefano Garzarella <sgarzare@redhat.com>

virtio_transport_build_skb() builds a new skb to be delivered to the
vsockmon tap device. To build the new skb, it uses the original skb
data length as payload length, but as the comment notes, the original
packet stored in the skb may have been split in multiple packets, so we
need to use the length in the header, which is correctly updated before
the packet is delivered to the tap, and the offset for the data.

This was also similar to what we did before commit 71dc9ec9ac7d
("virtio/vsock: replace virtio_vsock_pkt with sk_buff") where we probably
missed something during the skb conversion.

Also update the comment above, which was left stale by the skb
conversion and still mentioned a buffer pointer that no longer exists.

Fixes: 71dc9ec9ac7d ("virtio/vsock: replace virtio_vsock_pkt with sk_buff")
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 net/vmw_vsock/virtio_transport_common.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 9b8014516f4f..a678d5d75704 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -166,12 +166,12 @@ static struct sk_buff *virtio_transport_build_skb(void *opaque)
 	struct sk_buff *skb;
 	size_t payload_len;
 
-	/* A packet could be split to fit the RX buffer, so we can retrieve
-	 * the payload length from the header and the buffer pointer taking
-	 * care of the offset in the original packet.
+	/* A packet could be split to fit the RX buffer, so we use
+	 * the payload length from the header, which has been updated
+	 * by the sender to reflect the fragment size.
 	 */
 	pkt_hdr = virtio_vsock_hdr(pkt);
-	payload_len = pkt->len;
+	payload_len = le32_to_cpu(pkt_hdr->len);
 
 	skb = alloc_skb(sizeof(*hdr) + sizeof(*pkt_hdr) + payload_len,
 			GFP_ATOMIC);
@@ -219,7 +219,8 @@ static struct sk_buff *virtio_transport_build_skb(void *opaque)
 
 			virtio_transport_copy_nonlinear_skb(pkt, data, payload_len);
 		} else {
-			skb_put_data(skb, pkt->data, payload_len);
+			skb_put_data(skb, pkt->data + VIRTIO_VSOCK_SKB_CB(pkt)->offset,
+				     payload_len);
 		}
 	}
 
-- 
2.54.0

[PATCH net 2/2] vsock/virtio: fix empty payload in tap skb for non-linear buffers

[Stefano Garzarella]

From: Stefano Garzarella <sgarzare@redhat.com>

For non-linear skbs, virtio_transport_build_skb() goes through
virtio_transport_copy_nonlinear_skb() to copy the original payload
in the new skb to be delivered to the vsockmon tap device.
This manually initializes an iov_iter but does not set iov_iter.count.
Since the iov_iter is zero-initialized, the copy length is zero and no
payload is actually copied to the monitor interface, leaving data
un-initialized.

Fix this by removing the linear vs non-linear split and using
skb_copy_datagram_iter() with iov_iter_kvec() for all cases, as
vhost-vsock already does. This handles both linear and non-linear skbs,
properly initializes the iov_iter, and removes the now unused
virtio_transport_copy_nonlinear_skb().

While touching this code, let's also check the return value of
skb_copy_datagram_iter(), even though it's unlikely to fail.

Fixes: 4b0bf10eb077 ("vsock/virtio: non-linear skb handling for tap")
Reported-by: Yiqi Sun <sunyiqixm@gmail.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 net/vmw_vsock/virtio_transport_common.c | 40 ++++++++-----------------
 1 file changed, 12 insertions(+), 28 deletions(-)

diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index a678d5d75704..989cc252d3d3 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -136,27 +136,6 @@ static void virtio_transport_init_hdr(struct sk_buff *skb,
 	hdr->fwd_cnt	= cpu_to_le32(0);
 }
 
-static void virtio_transport_copy_nonlinear_skb(const struct sk_buff *skb,
-						void *dst,
-						size_t len)
-{
-	struct iov_iter iov_iter = { 0 };
-	struct kvec kvec;
-	size_t to_copy;
-
-	kvec.iov_base = dst;
-	kvec.iov_len = len;
-
-	iov_iter.iter_type = ITER_KVEC;
-	iov_iter.kvec = &kvec;
-	iov_iter.nr_segs = 1;
-
-	to_copy = min_t(size_t, len, skb->len);
-
-	skb_copy_datagram_iter(skb, VIRTIO_VSOCK_SKB_CB(skb)->offset,
-			       &iov_iter, to_copy);
-}
-
 /* Packet capture */
 static struct sk_buff *virtio_transport_build_skb(void *opaque)
 {
@@ -214,13 +193,18 @@ static struct sk_buff *virtio_transport_build_skb(void *opaque)
 	skb_put_data(skb, pkt_hdr, sizeof(*pkt_hdr));
 
 	if (payload_len) {
-		if (skb_is_nonlinear(pkt)) {
-			void *data = skb_put(skb, payload_len);
-
-			virtio_transport_copy_nonlinear_skb(pkt, data, payload_len);
-		} else {
-			skb_put_data(skb, pkt->data + VIRTIO_VSOCK_SKB_CB(pkt)->offset,
-				     payload_len);
+		struct iov_iter iov_iter;
+		struct kvec kvec;
+		void *data = skb_put(skb, payload_len);
+
+		kvec.iov_base = data;
+		kvec.iov_len = payload_len;
+		iov_iter_kvec(&iov_iter, ITER_DEST, &kvec, 1, payload_len);
+
+		if (skb_copy_datagram_iter(pkt, VIRTIO_VSOCK_SKB_CB(pkt)->offset,
+					   &iov_iter, payload_len)) {
+			kfree_skb(skb);
+			return NULL;
 		}
 	}
 
-- 
2.54.0

Re: [PATCH net 1/2] vsock/virtio: fix length and offset in tap skb for split packets

[Bobby Eshleman]

On Fri, May 08, 2026 at 06:44:10PM +0200, Stefano Garzarella wrote:
> From: Stefano Garzarella <sgarzare@redhat.com>
> 
> virtio_transport_build_skb() builds a new skb to be delivered to the
> vsockmon tap device. To build the new skb, it uses the original skb
> data length as payload length, but as the comment notes, the original
> packet stored in the skb may have been split in multiple packets, so we
> need to use the length in the header, which is correctly updated before
> the packet is delivered to the tap, and the offset for the data.
> 
> This was also similar to what we did before commit 71dc9ec9ac7d
> ("virtio/vsock: replace virtio_vsock_pkt with sk_buff") where we probably
> missed something during the skb conversion.
> 
> Also update the comment above, which was left stale by the skb
> conversion and still mentioned a buffer pointer that no longer exists.
> 
> Fixes: 71dc9ec9ac7d ("virtio/vsock: replace virtio_vsock_pkt with sk_buff")
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>  net/vmw_vsock/virtio_transport_common.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 9b8014516f4f..a678d5d75704 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -166,12 +166,12 @@ static struct sk_buff *virtio_transport_build_skb(void *opaque)
>  	struct sk_buff *skb;
>  	size_t payload_len;
>  
> -	/* A packet could be split to fit the RX buffer, so we can retrieve
> -	 * the payload length from the header and the buffer pointer taking
> -	 * care of the offset in the original packet.
> +	/* A packet could be split to fit the RX buffer, so we use
> +	 * the payload length from the header, which has been updated
> +	 * by the sender to reflect the fragment size.
>  	 */
>  	pkt_hdr = virtio_vsock_hdr(pkt);
> -	payload_len = pkt->len;
> +	payload_len = le32_to_cpu(pkt_hdr->len);
>  
>  	skb = alloc_skb(sizeof(*hdr) + sizeof(*pkt_hdr) + payload_len,
>  			GFP_ATOMIC);
> @@ -219,7 +219,8 @@ static struct sk_buff *virtio_transport_build_skb(void *opaque)
>  
>  			virtio_transport_copy_nonlinear_skb(pkt, data, payload_len);
>  		} else {
> -			skb_put_data(skb, pkt->data, payload_len);
> +			skb_put_data(skb, pkt->data + VIRTIO_VSOCK_SKB_CB(pkt)->offset,
> +				     payload_len);
>  		}
>  	}
>  
> -- 
> 2.54.0
> 

Reviewed-by: Bobby Eshleman <bobbyeshleman@meta.com>

Re: [PATCH net 2/2] vsock/virtio: fix empty payload in tap skb for non-linear buffers

[Bobby Eshleman]

On Fri, May 08, 2026 at 06:44:11PM +0200, Stefano Garzarella wrote:
> From: Stefano Garzarella <sgarzare@redhat.com>
> 
> For non-linear skbs, virtio_transport_build_skb() goes through
> virtio_transport_copy_nonlinear_skb() to copy the original payload
> in the new skb to be delivered to the vsockmon tap device.
> This manually initializes an iov_iter but does not set iov_iter.count.
> Since the iov_iter is zero-initialized, the copy length is zero and no
> payload is actually copied to the monitor interface, leaving data
> un-initialized.
> 
> Fix this by removing the linear vs non-linear split and using
> skb_copy_datagram_iter() with iov_iter_kvec() for all cases, as
> vhost-vsock already does. This handles both linear and non-linear skbs,
> properly initializes the iov_iter, and removes the now unused
> virtio_transport_copy_nonlinear_skb().
> 
> While touching this code, let's also check the return value of
> skb_copy_datagram_iter(), even though it's unlikely to fail.
> 
> Fixes: 4b0bf10eb077 ("vsock/virtio: non-linear skb handling for tap")
> Reported-by: Yiqi Sun <sunyiqixm@gmail.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>  net/vmw_vsock/virtio_transport_common.c | 40 ++++++++-----------------
>  1 file changed, 12 insertions(+), 28 deletions(-)
> 
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index a678d5d75704..989cc252d3d3 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -136,27 +136,6 @@ static void virtio_transport_init_hdr(struct sk_buff *skb,
>  	hdr->fwd_cnt	= cpu_to_le32(0);
>  }
>  
> -static void virtio_transport_copy_nonlinear_skb(const struct sk_buff *skb,
> -						void *dst,
> -						size_t len)
> -{
> -	struct iov_iter iov_iter = { 0 };
> -	struct kvec kvec;
> -	size_t to_copy;
> -
> -	kvec.iov_base = dst;
> -	kvec.iov_len = len;
> -
> -	iov_iter.iter_type = ITER_KVEC;
> -	iov_iter.kvec = &kvec;
> -	iov_iter.nr_segs = 1;
> -
> -	to_copy = min_t(size_t, len, skb->len);
> -
> -	skb_copy_datagram_iter(skb, VIRTIO_VSOCK_SKB_CB(skb)->offset,
> -			       &iov_iter, to_copy);
> -}
> -
>  /* Packet capture */
>  static struct sk_buff *virtio_transport_build_skb(void *opaque)
>  {
> @@ -214,13 +193,18 @@ static struct sk_buff *virtio_transport_build_skb(void *opaque)
>  	skb_put_data(skb, pkt_hdr, sizeof(*pkt_hdr));
>  
>  	if (payload_len) {
> -		if (skb_is_nonlinear(pkt)) {
> -			void *data = skb_put(skb, payload_len);
> -
> -			virtio_transport_copy_nonlinear_skb(pkt, data, payload_len);
> -		} else {
> -			skb_put_data(skb, pkt->data + VIRTIO_VSOCK_SKB_CB(pkt)->offset,
> -				     payload_len);
> +		struct iov_iter iov_iter;
> +		struct kvec kvec;
> +		void *data = skb_put(skb, payload_len);
> +
> +		kvec.iov_base = data;
> +		kvec.iov_len = payload_len;
> +		iov_iter_kvec(&iov_iter, ITER_DEST, &kvec, 1, payload_len);
> +
> +		if (skb_copy_datagram_iter(pkt, VIRTIO_VSOCK_SKB_CB(pkt)->offset,
> +					   &iov_iter, payload_len)) {
> +			kfree_skb(skb);
> +			return NULL;
>  		}
>  	}
>  
> -- 
> 2.54.0
> 

Reviewed-by: Bobby Eshleman <bobbyeshleman@meta.com>

Re: [PATCH net 0/2] vsock/virtio: fix vsockmon tap skb construction

[Michael S. Tsirkin]

On Fri, May 08, 2026 at 06:44:09PM +0200, Stefano Garzarella wrote:
> While reviewing the patch posted by Yiqi Sun [1] to fix an issue in
> virtio_transport_build_skb(), I discovered another issue related to
> the offset and length of the payload to be copied in the new skb.
> This was introduced when we did the skb conversion, and fixed by
> patch 1.
> 
> Patch 2 fixes the issue found by Yiqi Sun in a different way: using
> iov_iter_kvec() to properly initialize all the iov_iter fields and
> removing the linear vs non-linear split like we alredy do in
> vhost-vsock.
> 
> It could have been a single patch, but since there were two affected
> commits, I decided to keep the fixes separate.
> 
> [1] https://lore.kernel.org/netdev/20260430071110.380509-1-sunyiqixm@gmail.com/


Acked-by: Michael S. Tsirkin <mst@redhat.com>

> Stefano Garzarella (2):
>   vsock/virtio: fix length and offset in tap skb for split packets
>   vsock/virtio: fix empty payload in tap skb for non-linear buffers
> 
>  net/vmw_vsock/virtio_transport_common.c | 47 +++++++++----------------
>  1 file changed, 16 insertions(+), 31 deletions(-)
> 
> -- 
> 2.54.0

Arseniy Krasnov

[Arseniy Krasnov]

> From: Stefano Garzarella <sgarzare@redhat.com>
>
> virtio_transport_build_skb() builds a new skb to be delivered to the
> vsockmon tap device. To build the new skb, it uses the original skb
> data length as payload length, but as the comment notes, the original
> packet stored in the skb may have been split in multiple packets, so we
> need to use the length in the header, which is correctly updated before
> the packet is delivered to the tap, and the offset for the data.
>
> This was also similar to what we did before commit 71dc9ec9ac7d
> ("virtio/vsock: replace virtio_vsock_pkt with sk_buff") where we probably
> missed something during the skb conversion.
>
> Also update the comment above, which was left stale by the skb
> conversion and still mentioned a buffer pointer that no longer exists.
>
> Fixes: 71dc9ec9ac7d ("virtio/vsock: replace virtio_vsock_pkt with sk_buff")
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>  net/vmw_vsock/virtio_transport_common.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 9b8014516f4f..a678d5d75704 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -166,12 +166,12 @@ static struct sk_buff *virtio_transport_build_skb(void *opaque)
>  	struct sk_buff *skb;
>  	size_t payload_len;
>  
> -	/* A packet could be split to fit the RX buffer, so we can retrieve
> -	 * the payload length from the header and the buffer pointer taking
> -	 * care of the offset in the original packet.
> +	/* A packet could be split to fit the RX buffer, so we use
> +	 * the payload length from the header, which has been updated
> +	 * by the sender to reflect the fragment size.
>  	 */
>  	pkt_hdr = virtio_vsock_hdr(pkt);
> -	payload_len = pkt->len;
> +	payload_len = le32_to_cpu(pkt_hdr->len);
>  
>  	skb = alloc_skb(sizeof(*hdr) + sizeof(*pkt_hdr) + payload_len,
>  			GFP_ATOMIC);
> @@ -219,7 +219,8 @@ static struct sk_buff *virtio_transport_build_skb(void *opaque)
>  
>  			virtio_transport_copy_nonlinear_skb(pkt, data, payload_len);
>  		} else {
> -			skb_put_data(skb, pkt->data, payload_len);
> +			skb_put_data(skb, pkt->data + VIRTIO_VSOCK_SKB_CB(pkt)->offset,
> +				     payload_len);
>  		}
>  	}
>  
> -- 2.54.0

Reviewed-by: Arseniy Krasnov <avkrasnov@rulkc.org>

Arseniy Krasnov

[Arseniy Krasnov]

> From: Stefano Garzarella <sgarzare@redhat.com>
>
> For non-linear skbs, virtio_transport_build_skb() goes through
> virtio_transport_copy_nonlinear_skb() to copy the original payload
> in the new skb to be delivered to the vsockmon tap device.
> This manually initializes an iov_iter but does not set iov_iter.count.
> Since the iov_iter is zero-initialized, the copy length is zero and no
> payload is actually copied to the monitor interface, leaving data
> un-initialized.
>
> Fix this by removing the linear vs non-linear split and using
> skb_copy_datagram_iter() with iov_iter_kvec() for all cases, as
> vhost-vsock already does. This handles both linear and non-linear skbs,
> properly initializes the iov_iter, and removes the now unused
> virtio_transport_copy_nonlinear_skb().
>
> While touching this code, let's also check the return value of
> skb_copy_datagram_iter(), even though it's unlikely to fail.
>
> Fixes: 4b0bf10eb077 ("vsock/virtio: non-linear skb handling for tap")
> Reported-by: Yiqi Sun <sunyiqixm@gmail.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>  net/vmw_vsock/virtio_transport_common.c | 40 ++++++++-----------------
>  1 file changed, 12 insertions(+), 28 deletions(-)
>
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index a678d5d75704..989cc252d3d3 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -136,27 +136,6 @@ static void virtio_transport_init_hdr(struct sk_buff *skb,
>  	hdr->fwd_cnt	= cpu_to_le32(0);
>  }
>  
> -static void virtio_transport_copy_nonlinear_skb(const struct sk_buff *skb,
> -						void *dst,
> -						size_t len)
> -{
> -	struct iov_iter iov_iter = { 0 };
> -	struct kvec kvec;
> -	size_t to_copy;
> -
> -	kvec.iov_base = dst;
> -	kvec.iov_len = len;
> -
> -	iov_iter.iter_type = ITER_KVEC;
> -	iov_iter.kvec = &kvec;
> -	iov_iter.nr_segs = 1;
> -
> -	to_copy = min_t(size_t, len, skb->len);
> -
> -	skb_copy_datagram_iter(skb, VIRTIO_VSOCK_SKB_CB(skb)->offset,
> -			       &iov_iter, to_copy);
> -}
> -
>  /* Packet capture */
>  static struct sk_buff *virtio_transport_build_skb(void *opaque)
>  {
> @@ -214,13 +193,18 @@ static struct sk_buff *virtio_transport_build_skb(void *opaque)
>  	skb_put_data(skb, pkt_hdr, sizeof(*pkt_hdr));
>  
>  	if (payload_len) {
> -		if (skb_is_nonlinear(pkt)) {
> -			void *data = skb_put(skb, payload_len);
> -
> -			virtio_transport_copy_nonlinear_skb(pkt, data, payload_len);
> -		} else {
> -			skb_put_data(skb, pkt->data + VIRTIO_VSOCK_SKB_CB(pkt)->offset,
> -				     payload_len);
> +		struct iov_iter iov_iter;
> +		struct kvec kvec;
> +		void *data = skb_put(skb, payload_len);
> +
> +		kvec.iov_base = data;
> +		kvec.iov_len = payload_len;
> +		iov_iter_kvec(&iov_iter, ITER_DEST, &kvec, 1, payload_len);
> +
> +		if (skb_copy_datagram_iter(pkt, VIRTIO_VSOCK_SKB_CB(pkt)->offset,
> +					   &iov_iter, payload_len)) {
> +			kfree_skb(skb);
> +			return NULL;
>  		}
>  	}
>  
> -- 
> 2.54.0
>
Reviewed-by: Arseniy Krasnov <avkrasnov@rulkc.org>