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

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

[Stefano Garzarella]

Patch 1 resets the connection when we can no longer queue packets,
this prevents silent data loss, and both peers are notified.

Patch 2 increases the total budget to `buf_alloc * 2` for payload
plus skb overhead 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.

In the future, we plan to improve how we handle the merging of packets
to minimize overhead and avoid closing connections.

v4:
- Split the buf_alloc check to be sure the credit is still respected and
  to avoid overflow of buf_used [sashiko]
- call virtio_transport_do_close() and vsock_remove_sock() to properly
  close the connection and remove the socket from the connect table
  [sashiko]

v3: https://lore.kernel.org/netdev/20260513105417.56761-1-sgarzare@redhat.com/
- Split in 2 patches [MST]

v2: https://lore.kernel.org/netdev/20260512080737.36787-1-sgarzare@redhat.com/
- 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/

Stefano Garzarella (2):
  vsock/virtio: reset connection on receiving queue overflow
  vsock/virtio: fix skb overhead accounting to preserve full buf_alloc

 net/vmw_vsock/virtio_transport_common.c | 29 ++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 6 deletions(-)

-- 
2.54.0

[PATCH net v4 1/2] vsock/virtio: reset connection on receiving queue overflow

[Stefano Garzarella]

From: Stefano Garzarella <sgarzare@redhat.com>

When there is no more space to queue an incoming packet, the packet is
silently dropped. This causes data loss without any notification to
either peer, since there is no retransmission.

Under normal circumstances, this should never happen. However, it could
happen if the other peer doesn't respect the credit, or if the skb
overhead, which we recently began to take into account with commit
059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue"),
is too high.

Fix this by resetting the connection and setting the local socket error
to ENOBUFS when virtio_transport_recv_enqueue() can no longer queue a
packet, so both peers are explicitly notified of the failure rather than
silently losing data.

Fixes: ae6fcfbf5f03 ("vsock/virtio: discard packets if credit is not respected")
Cc: stable@vger.kernel.org
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 net/vmw_vsock/virtio_transport_common.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 1e3409d28164..5028ff534888 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -1335,7 +1335,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)
 {
@@ -1350,10 +1350,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++;
@@ -1393,6 +1391,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
@@ -1405,7 +1405,17 @@ 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);
+			virtio_transport_do_close(vsk, true);
+			sk->sk_err = ENOBUFS;
+			sk_error_report(sk);
+			vsock_remove_sock(vsk);
+			break;
+		}
 		vsock_data_ready(sk);
 		return err;
 	case VIRTIO_VSOCK_OP_CREDIT_REQUEST:
-- 
2.54.0

[PATCH net v4 2/2] 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 by allowing at most `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.

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

Fixes: 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue")
Cc: stable@vger.kernel.org
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 net/vmw_vsock/virtio_transport_common.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 5028ff534888..df3b418e0392 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -419,7 +419,14 @@ 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)
+	/* Allow at most buf_alloc * 2 total budget (payload + overhead),
+	 * similar to how SO_RCVBUF is doubled to reserve space for sk_buff
+	 * metadata. Check payload against buf_alloc to be sure the other
+	 * peer is respecting the credit, and sk_buff overhead to bound
+	 * queue growth.
+	 */
+	if ((u64)vvs->buf_used + len > vvs->buf_alloc ||
+	    skb_overhead > vvs->buf_alloc)
 		return false;
 
 	vvs->rx_bytes += len;
-- 
2.54.0

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

[Stefano Garzarella]

On Mon, May 18, 2026 at 11:06:54AM +0200, Stefano Garzarella wrote:
>Patch 1 resets the connection when we can no longer queue packets,
>this prevents silent data loss, and both peers are notified.
>
>Patch 2 increases the total budget to `buf_alloc * 2` for payload
>plus skb overhead 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.
>
>In the future, we plan to improve how we handle the merging of packets
>to minimize overhead and avoid closing connections.
>
>v4:
>- Split the buf_alloc check to be sure the credit is still respected and
>  to avoid overflow of buf_used [sashiko]
>- call virtio_transport_do_close() and vsock_remove_sock() to properly
>  close the connection and remove the socket from the connect table
>  [sashiko]

sashiko reports 2 issues on the second patch but both IMO are 
pre-existing: 
https://sashiko.dev/#/patchset/20260518090656.134588-1-sgarzare%40redhat.com

The first one related 32-bit architectures should be fixed, but can be a 
follow up since not introduced by this series.

The secong one related to a low value set by the user as buffer size, 
it's also pre-existing and I don't think we can do much.

Please let me know if you prefer to add another patch to this series to 
fix the first issue, or just send another patch.

Thanks,
Stefano


>
>v3: https://lore.kernel.org/netdev/20260513105417.56761-1-sgarzare@redhat.com/
>- Split in 2 patches [MST]
>
>v2: https://lore.kernel.org/netdev/20260512080737.36787-1-sgarzare@redhat.com/
>- 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/
>
>Stefano Garzarella (2):
>  vsock/virtio: reset connection on receiving queue overflow
>  vsock/virtio: fix skb overhead accounting to preserve full buf_alloc
>
> net/vmw_vsock/virtio_transport_common.c | 29 ++++++++++++++++++++-----
> 1 file changed, 23 insertions(+), 6 deletions(-)
>
>-- 
>2.54.0
>

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

[Jakub Kicinski]

On Mon, 18 May 2026 11:06:54 +0200 Stefano Garzarella wrote:
> Patch 1 resets the connection when we can no longer queue packets,
> this prevents silent data loss, and both peers are notified.
> 
> Patch 2 increases the total budget to `buf_alloc * 2` for payload
> plus skb overhead 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.

Hi Michael!

Are you okay with these fixes? A bandaid but not too outrageous..