Re: [PATCH net v2 2/3] vsock/virtio: bound the send credit by bytes not yet consumed by the transport

[Stefano Garzarella <sgarzare@redhat.com>]

On Fri, Nov 06, 2026 at 12:02:43AM -0500, Oleg Drokin wrote:
>The send credit is
>
>    min(peer_buf_alloc, buf_alloc) - (tx_cnt - peer_fwd_cnt)
>
>where peer_fwd_cnt is taken unchecked from every header the peer sends.
>A peer that never consumes what it is sent but keeps reporting
>fwd_cnt == tx_cnt gets unlimited credit: every send() is accepted and
>the skbs pile up in the transport's queue (on a host, vhost's
>send_pkt_queue, for a guest that posts no rx buffers). The skbs are
>GFP_KERNEL allocations charged to no memory cgroup. The peer learns
>tx_cnt easily when the host side relays a stream it also controls.
>
>Commit 8ee784fdf006 ("vsock/virtio: cap TX credit to local buffer size")
>bounded peer_buf_alloc, but not peer_fwd_cnt.
>
>bytes_unsent (maintained for SIOCOUTQ) counts what was handed to the
>transport and not yet consumed by the device. It is local knowledge, so
>bounding it by the same window limits a lying peer to one window of
>queued data per socket.
>
>For an honest peer, bytes_unsent can exceed tx_cnt - peer_fwd_cnt only
>by data the peer already has but the transport has not yet released:
>
> - on a host, the delivered part of an skb that vhost splits across rx
>   buffers smaller than the skb: the peer acknowledges the pieces, but
>   bytes_unsent drops only once the whole skb is delivered, so at most
>   one skb (64 KiB);
> - in a guest, tx buffers the host has consumed but
>   virtio_transport_tx_work() has not reaped yet: vhost passes the
>   packet to the host socket before it marks the buffer used, so the
>   host's credit update can arrive first.
>
>Either way the window shrinks only until the transport catches up;
>nothing stalls and nothing changes on the wire. The writer is woken by
>sock_wfree() once enough of the queued skbs are freed, as it already is
>when credit comes back.
>
>With a user-space vhost-vsock "guest" that posts no rx buffers and
>acknowledges everything, one host socket queued 256 MiB in under 2 s
>(SIOCOUTQ 268304384, where the test stops); with this patch it stays at
>one window (SIOCOUTQ 262144).
>
>Found by an LLM-assisted security review of vhost-vsock, done while
>porting a project from Xen to KVM, and reproduced as above.
>
>Fixes: 06a8fc78367d ("VSOCK: Introduce virtio_vsock_common.ko")
>Signed-off-by: Oleg Drokin <green@linuxhacker.ru>
>---
> net/vmw_vsock/virtio_transport_common.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index 1e762a480df4..74a8fdf825d5 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -1031,14 +1031,17 @@ EXPORT_SYMBOL_GPL(virtio_transport_seqpacket_has_data);
>
> static s64 virtio_transport_has_space(struct virtio_vsock_sock *vvs)
> {
>+	s64 tx_buf_size = virtio_transport_tx_buf_size(vvs);
> 	s64 bytes;
>
> 	/* Use s64 arithmetic so if the peer shrinks peer_buf_alloc while
> 	 * we have bytes in flight (tx_cnt - peer_fwd_cnt), the subtraction
> 	 * does not underflow.
> 	 */
>-	bytes = (s64)virtio_transport_tx_buf_size(vvs) -
>-		(vvs->tx_cnt - vvs->peer_fwd_cnt);
>+	bytes = tx_buf_size - (vvs->tx_cnt - vvs->peer_fwd_cnt);
>+
>+	/* peer_fwd_cnt is peer-controlled, bytes_unsent is not. */

Would be nice to explain a bit better what we are preventing here.
Not a long description, just few words.

>+	bytes = min_t(s64, bytes, tx_buf_size - (s64)vvs->bytes_unsent);

TBH I found this code a bit unreadable, what about this:

         in_flight = max_t(s64, vvs->tx_cnt - vvs->peer_fwd_cnt,
                           vvs->bytes_unsent);
	bytes = (s64)virtio_transport_tx_buf_size(vvs) - in_flight;


Thanks,
Stefano


> 	if (bytes < 0)
> 		bytes = 0;
>
>-- 
>2.56.0
>