[PATCH net v2] vsock/virtio: fix skb overhead accounting to preserve full buf_alloc

[PATCH net v2] vsock/virtio: fix skb overhead accounting to preserve full buf_alloc

[Stefano Garzarella]

From: Stefano Garzarella <sgarzare@redhat.com>

After commit 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb
queue"), virtio_transport_inc_rx_pkt() subtracts per-skb overhead from
buf_alloc when checking whether a new packet fits. This reduces the
effective receive buffer below what the user configured via
SO_VM_SOCKETS_BUFFER_SIZE, causing legitimate data packets to be
silently dropped and applications that rely on the full buffer size
to deadlock.

Also, the reduced space is not communicated to the remote peer, so
its credit calculation accounts more credit than the receiver will
actually accept, causing data loss (there is no retransmission).

With this approach we currently have failures in
tools/testing/vsock/vsock_test.c. Test 18 sometimes fails, while
test 22 always fails in this way:
    18 - SOCK_STREAM MSG_ZEROCOPY...hash mismatch

    22 - SOCK_STREAM virtio credit update + SO_RCVLOWAT...send failed:
    Resource temporarily unavailable

Fix this by using `buf_alloc * 2` as the total budget for payload plus
skb overhead in virtio_transport_inc_rx_pkt(), similar to how SO_RCVBUF
is doubled to reserve space for sk_buff metadata. This preserves the
full buf_alloc for payload under normal operation, while still bounding
the skb queue growth.

When the total budget (buf_alloc * 2) is exceeded (e.g. under small-packet
flooding where overhead dominates), the connection is reset and local
socket error set to ENOBUFS, so both peers are explicitly notified of
the failure rather than silently losing data.

With this patch, all tests in tools/testing/vsock/vsock_test.c are
now passing again.

A solution to handle small-packet overhead efficiently also for
SEQPACKET (we already do that for STREAM) is planned as follow-up work.
This patch is needed in any case to prevent silent data loss, because
even if we reduce the overhead, we can't eliminate it entirely.

Fixes: 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue")
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
v2:
- Close the connection when we can no longer queue new packets instead
  of losing data.
- No longer announce the reduced buf_alloc to avoid violating the
  spec. [MST]

v1: https://lore.kernel.org/netdev/20260508092330.69690-1-sgarzare@redhat.com/
---
 net/vmw_vsock/virtio_transport_common.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 9b8014516f4f..f23bf8a11319 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -449,7 +449,10 @@ static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
 {
 	u64 skb_overhead = (skb_queue_len(&vvs->rx_queue) + 1) * SKB_TRUESIZE(0);
 
-	if (skb_overhead + vvs->buf_used + len > vvs->buf_alloc)
+	/* Use buf_alloc * 2 as total budget (payload + overhead), similar to
+	 * how SO_RCVBUF is doubled to reserve space for sk_buff metadata.
+	 */
+	if (skb_overhead + vvs->buf_used + len > (u64)vvs->buf_alloc * 2)
 		return false;
 
 	vvs->rx_bytes += len;
@@ -1365,7 +1368,7 @@ virtio_transport_recv_connecting(struct sock *sk,
 	return err;
 }
 
-static void
+static bool
 virtio_transport_recv_enqueue(struct vsock_sock *vsk,
 			      struct sk_buff *skb)
 {
@@ -1380,10 +1383,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
 	spin_lock_bh(&vvs->rx_lock);
 
 	can_enqueue = virtio_transport_inc_rx_pkt(vvs, len);
-	if (!can_enqueue) {
-		free_pkt = true;
+	if (!can_enqueue)
 		goto out;
-	}
 
 	if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM)
 		vvs->msg_count++;
@@ -1423,6 +1424,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
 	spin_unlock_bh(&vvs->rx_lock);
 	if (free_pkt)
 		kfree_skb(skb);
+
+	return can_enqueue;
 }
 
 static int
@@ -1435,7 +1438,16 @@ virtio_transport_recv_connected(struct sock *sk,
 
 	switch (le16_to_cpu(hdr->op)) {
 	case VIRTIO_VSOCK_OP_RW:
-		virtio_transport_recv_enqueue(vsk, skb);
+		if (!virtio_transport_recv_enqueue(vsk, skb)) {
+			/* There is no more space to queue the packet, so let's
+			 * close the connection; otherwise, we'll lose data.
+			 */
+			(void)virtio_transport_reset(vsk, skb);
+			sk->sk_state = TCP_CLOSE;
+			sk->sk_err = ENOBUFS;
+			sk_error_report(sk);
+			break;
+		}
 		vsock_data_ready(sk);
 		return err;
 	case VIRTIO_VSOCK_OP_CREDIT_REQUEST:
-- 
2.54.0

Re: [PATCH net v2] vsock/virtio: fix skb overhead accounting to preserve full buf_alloc

[Michael S. Tsirkin]

On Tue, May 12, 2026 at 10:07:37AM +0200, Stefano Garzarella wrote:
> From: Stefano Garzarella <sgarzare@redhat.com>
> 
> After commit 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb
> queue"), virtio_transport_inc_rx_pkt() subtracts per-skb overhead from
> buf_alloc when checking whether a new packet fits. This reduces the
> effective receive buffer below what the user configured via
> SO_VM_SOCKETS_BUFFER_SIZE, causing legitimate data packets to be
> silently dropped and applications that rely on the full buffer size
> to deadlock.
> 
> Also, the reduced space is not communicated to the remote peer, so
> its credit calculation accounts more credit than the receiver will
> actually accept, causing data loss (there is no retransmission).
> 
> With this approach we currently have failures in
> tools/testing/vsock/vsock_test.c. Test 18 sometimes fails, while
> test 22 always fails in this way:
>     18 - SOCK_STREAM MSG_ZEROCOPY...hash mismatch
> 
>     22 - SOCK_STREAM virtio credit update + SO_RCVLOWAT...send failed:
>     Resource temporarily unavailable
> 
> Fix this by using `buf_alloc * 2` as the total budget for payload plus
> skb overhead in virtio_transport_inc_rx_pkt(), similar to how SO_RCVBUF
> is doubled to reserve space for sk_buff metadata. This preserves the
> full buf_alloc for payload under normal operation, while still bounding
> the skb queue growth.
> 
> When the total budget (buf_alloc * 2) is exceeded (e.g. under small-packet
> flooding where overhead dominates), the connection is reset and local
> socket error set to ENOBUFS, so both peers are explicitly notified of
> the failure rather than silently losing data.
> 
> With this patch, all tests in tools/testing/vsock/vsock_test.c are
> now passing again.
> 
> A solution to handle small-packet overhead efficiently also for
> SEQPACKET (we already do that for STREAM) is planned as follow-up work.
> This patch is needed in any case to prevent silent data loss, because
> even if we reduce the overhead, we can't eliminate it entirely.
> 
> Fixes: 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue")
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>

Thanks for the patch!  I'd like to split this: 
1. buf alloc boost
2. reset when out of credits

this way we can revert 2 easier later.


> ---
> v2:
> - Close the connection when we can no longer queue new packets instead
>   of losing data.
> - No longer announce the reduced buf_alloc to avoid violating the
>   spec. [MST]
> 
> v1: https://lore.kernel.org/netdev/20260508092330.69690-1-sgarzare@redhat.com/
> ---
>  net/vmw_vsock/virtio_transport_common.c | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 9b8014516f4f..f23bf8a11319 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -449,7 +449,10 @@ static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
>  {
>  	u64 skb_overhead = (skb_queue_len(&vvs->rx_queue) + 1) * SKB_TRUESIZE(0);
>  
> -	if (skb_overhead + vvs->buf_used + len > vvs->buf_alloc)
> +	/* Use buf_alloc * 2 as total budget (payload + overhead), similar to
> +	 * how SO_RCVBUF is doubled to reserve space for sk_buff metadata.
> +	 */
> +	if (skb_overhead + vvs->buf_used + len > (u64)vvs->buf_alloc * 2)
>  		return false;
>  
>  	vvs->rx_bytes += len;
> @@ -1365,7 +1368,7 @@ virtio_transport_recv_connecting(struct sock *sk,
>  	return err;
>  }
>  
> -static void
> +static bool
>  virtio_transport_recv_enqueue(struct vsock_sock *vsk,
>  			      struct sk_buff *skb)
>  {
> @@ -1380,10 +1383,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
>  	spin_lock_bh(&vvs->rx_lock);
>  
>  	can_enqueue = virtio_transport_inc_rx_pkt(vvs, len);
> -	if (!can_enqueue) {
> -		free_pkt = true;
> +	if (!can_enqueue)
>  		goto out;
> -	}
>  
>  	if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM)
>  		vvs->msg_count++;
> @@ -1423,6 +1424,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
>  	spin_unlock_bh(&vvs->rx_lock);
>  	if (free_pkt)
>  		kfree_skb(skb);
> +
> +	return can_enqueue;
>  }
>  
>  static int
> @@ -1435,7 +1438,16 @@ virtio_transport_recv_connected(struct sock *sk,
>  
>  	switch (le16_to_cpu(hdr->op)) {
>  	case VIRTIO_VSOCK_OP_RW:
> -		virtio_transport_recv_enqueue(vsk, skb);
> +		if (!virtio_transport_recv_enqueue(vsk, skb)) {
> +			/* There is no more space to queue the packet, so let's
> +			 * close the connection; otherwise, we'll lose data.
> +			 */
> +			(void)virtio_transport_reset(vsk, skb);
> +			sk->sk_state = TCP_CLOSE;
> +			sk->sk_err = ENOBUFS;
> +			sk_error_report(sk);
> +			break;
> +		}
>  		vsock_data_ready(sk);
>  		return err;
>  	case VIRTIO_VSOCK_OP_CREDIT_REQUEST:
> -- 
> 2.54.0

Re: [PATCH net v2] vsock/virtio: fix skb overhead accounting to preserve full buf_alloc

[Stefano Garzarella]

On Tue, May 12, 2026 at 04:54:34AM -0400, Michael S. Tsirkin wrote:
>On Tue, May 12, 2026 at 10:07:37AM +0200, Stefano Garzarella wrote:
>> From: Stefano Garzarella <sgarzare@redhat.com>
>>
>> After commit 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb
>> queue"), virtio_transport_inc_rx_pkt() subtracts per-skb overhead from
>> buf_alloc when checking whether a new packet fits. This reduces the
>> effective receive buffer below what the user configured via
>> SO_VM_SOCKETS_BUFFER_SIZE, causing legitimate data packets to be
>> silently dropped and applications that rely on the full buffer size
>> to deadlock.
>>
>> Also, the reduced space is not communicated to the remote peer, so
>> its credit calculation accounts more credit than the receiver will
>> actually accept, causing data loss (there is no retransmission).
>>
>> With this approach we currently have failures in
>> tools/testing/vsock/vsock_test.c. Test 18 sometimes fails, while
>> test 22 always fails in this way:
>>     18 - SOCK_STREAM MSG_ZEROCOPY...hash mismatch
>>
>>     22 - SOCK_STREAM virtio credit update + SO_RCVLOWAT...send failed:
>>     Resource temporarily unavailable
>>
>> Fix this by using `buf_alloc * 2` as the total budget for payload plus
>> skb overhead in virtio_transport_inc_rx_pkt(), similar to how SO_RCVBUF
>> is doubled to reserve space for sk_buff metadata. This preserves the
>> full buf_alloc for payload under normal operation, while still bounding
>> the skb queue growth.
>>
>> When the total budget (buf_alloc * 2) is exceeded (e.g. under small-packet
>> flooding where overhead dominates), the connection is reset and local
>> socket error set to ENOBUFS, so both peers are explicitly notified of
>> the failure rather than silently losing data.
>>
>> With this patch, all tests in tools/testing/vsock/vsock_test.c are
>> now passing again.
>>
>> A solution to handle small-packet overhead efficiently also for
>> SEQPACKET (we already do that for STREAM) is planned as follow-up work.
>> This patch is needed in any case to prevent silent data loss, because
>> even if we reduce the overhead, we can't eliminate it entirely.
>>
>> Fixes: 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue")
>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>
>Thanks for the patch!  I'd like to split this:
>1. buf alloc boost
>2. reset when out of credits

Good point, also the reset maybe should have an other fixes tag (i.e.  
when we introduced that check)

>
>this way we can revert 2 easier later.

I'm not sure if we should revert them at some point, even though we'll 
be able to handle the overhead better, but I agree that we should split 
them.

I'll wait for a few more comments and then send v3 with the split.

Thanks,
Stefano

Re: [PATCH net v2] vsock/virtio: fix skb overhead accounting to preserve full buf_alloc

[Michael S. Tsirkin]

On Tue, May 12, 2026 at 12:03:38PM +0200, Stefano Garzarella wrote:
> On Tue, May 12, 2026 at 04:54:34AM -0400, Michael S. Tsirkin wrote:
> > On Tue, May 12, 2026 at 10:07:37AM +0200, Stefano Garzarella wrote:
> > > From: Stefano Garzarella <sgarzare@redhat.com>
> > > 
> > > After commit 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb
> > > queue"), virtio_transport_inc_rx_pkt() subtracts per-skb overhead from
> > > buf_alloc when checking whether a new packet fits. This reduces the
> > > effective receive buffer below what the user configured via
> > > SO_VM_SOCKETS_BUFFER_SIZE, causing legitimate data packets to be
> > > silently dropped and applications that rely on the full buffer size
> > > to deadlock.
> > > 
> > > Also, the reduced space is not communicated to the remote peer, so
> > > its credit calculation accounts more credit than the receiver will
> > > actually accept, causing data loss (there is no retransmission).
> > > 
> > > With this approach we currently have failures in
> > > tools/testing/vsock/vsock_test.c. Test 18 sometimes fails, while
> > > test 22 always fails in this way:
> > >     18 - SOCK_STREAM MSG_ZEROCOPY...hash mismatch
> > > 
> > >     22 - SOCK_STREAM virtio credit update + SO_RCVLOWAT...send failed:
> > >     Resource temporarily unavailable
> > > 
> > > Fix this by using `buf_alloc * 2` as the total budget for payload plus
> > > skb overhead in virtio_transport_inc_rx_pkt(), similar to how SO_RCVBUF
> > > is doubled to reserve space for sk_buff metadata. This preserves the
> > > full buf_alloc for payload under normal operation, while still bounding
> > > the skb queue growth.
> > > 
> > > When the total budget (buf_alloc * 2) is exceeded (e.g. under small-packet
> > > flooding where overhead dominates), the connection is reset and local
> > > socket error set to ENOBUFS, so both peers are explicitly notified of
> > > the failure rather than silently losing data.
> > > 
> > > With this patch, all tests in tools/testing/vsock/vsock_test.c are
> > > now passing again.
> > > 
> > > A solution to handle small-packet overhead efficiently also for
> > > SEQPACKET (we already do that for STREAM) is planned as follow-up work.
> > > This patch is needed in any case to prevent silent data loss, because
> > > even if we reduce the overhead, we can't eliminate it entirely.
> > > 
> > > Fixes: 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue")
> > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > 
> > Thanks for the patch!  I'd like to split this:
> > 1. buf alloc boost
> > 2. reset when out of credits
> 
> Good point, also the reset maybe should have an other fixes tag (i.e.  when
> we introduced that check)
> 
> > 
> > this way we can revert 2 easier later.
> 
> I'm not sure if we should revert them at some point, even though we'll be
> able to handle the overhead better,

I mean we'll prevent the overflow, the condition will never be met.


> but I agree that we should split them.
> 
> I'll wait for a few more comments and then send v3 with the split.
> 
> Thanks,
> Stefano