netdev - Re: [PATCH net v6 1/4] vsock/virtio: fix potential underflow in virtio_transport_get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aXDBKc0HyN8f-8l7@leonardi-redhat>
Date: Wed, 21 Jan 2026 13:07:17 +0100
From: Luigi Leonardi <leonardi@...hat.com>
To: Stefano Garzarella <sgarzare@...hat.com>
Cc: netdev@...r.kernel.org, Simon Horman <horms@...nel.org>, 
	Eric Dumazet <edumazet@...gle.com>, "Michael S. Tsirkin" <mst@...hat.com>, 
	Arseniy Krasnov <AVKrasnov@...rdevices.ru>, "David S. Miller" <davem@...emloft.net>, 
	virtualization@...ts.linux.dev, Paolo Abeni <pabeni@...hat.com>, 
	Jakub Kicinski <kuba@...nel.org>, Stefan Hajnoczi <stefanha@...hat.com>, kvm@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Claudio Imbrenda <imbrenda@...ux.vnet.ibm.com>, 
	Jason Wang <jasowang@...hat.com>, Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, 
	Eugenio Pérez <eperezma@...hat.com>, Asias He <asias@...hat.com>, Melbin K Mathew <mlbnkm1@...il.com>
Subject: Re: [PATCH net v6 1/4] vsock/virtio: fix potential underflow in
 virtio_transport_get_credit()

On Wed, Jan 21, 2026 at 10:36:25AM +0100, Stefano Garzarella wrote:
>From: Melbin K Mathew <mlbnkm1@...il.com>
>
>The credit calculation in virtio_transport_get_credit() uses unsigned
>arithmetic:
>
>  ret = vvs->peer_buf_alloc - (vvs->tx_cnt - vvs->peer_fwd_cnt);
>
>If the peer shrinks its advertised buffer (peer_buf_alloc) while bytes
>are in flight, the subtraction can underflow and produce a large
>positive value, potentially allowing more data to be queued than the
>peer can handle.
>
>Reuse virtio_transport_has_space() which already handles this case and
>add a comment to make it clear why we are doing that.
>
>Fixes: 06a8fc78367d ("VSOCK: Introduce virtio_vsock_common.ko")
>Suggested-by: Stefano Garzarella <sgarzare@...hat.com>
>Signed-off-by: Melbin K Mathew <mlbnkm1@...il.com>
>[Stefano: use virtio_transport_has_space() instead of duplicating the code]
>[Stefano: tweak the commit message]
>Signed-off-by: Stefano Garzarella <sgarzare@...hat.com>
>---
> net/vmw_vsock/virtio_transport_common.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index 26b979ad71f0..6175124d63d3 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -28,6 +28,7 @@
>
> static void virtio_transport_cancel_close_work(struct vsock_sock *vsk,
> 					       bool cancel_timeout);
>+static s64 virtio_transport_has_space(struct virtio_vsock_sock *vvs);
>
> static const struct virtio_transport *
> virtio_transport_get_ops(struct vsock_sock *vsk)
>@@ -499,9 +500,7 @@ u32 virtio_transport_get_credit(struct virtio_vsock_sock *vvs, u32 credit)
> 		return 0;
>
> 	spin_lock_bh(&vvs->tx_lock);
>-	ret = vvs->peer_buf_alloc - (vvs->tx_cnt - vvs->peer_fwd_cnt);
>-	if (ret > credit)
>-		ret = credit;
>+	ret = min_t(u32, credit, virtio_transport_has_space(vvs));
> 	vvs->tx_cnt += ret;
> 	vvs->bytes_unsent += ret;
> 	spin_unlock_bh(&vvs->tx_lock);
>@@ -877,11 +876,14 @@ u32 virtio_transport_seqpacket_has_data(struct vsock_sock *vsk)
> }
> EXPORT_SYMBOL_GPL(virtio_transport_seqpacket_has_data);
>
>-static s64 virtio_transport_has_space(struct vsock_sock *vsk)
>+static s64 virtio_transport_has_space(struct virtio_vsock_sock *vvs)
> {
>-	struct virtio_vsock_sock *vvs = vsk->trans;
> 	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)vvs->peer_buf_alloc - (vvs->tx_cnt - vvs->peer_fwd_cnt);
> 	if (bytes < 0)
> 		bytes = 0;
>@@ -895,7 +897,7 @@ s64 virtio_transport_stream_has_space(struct vsock_sock *vsk)
> 	s64 bytes;
>
> 	spin_lock_bh(&vvs->tx_lock);
>-	bytes = virtio_transport_has_space(vsk);
>+	bytes = virtio_transport_has_space(vvs);
> 	spin_unlock_bh(&vvs->tx_lock);
>
> 	return bytes;
>@@ -1492,7 +1494,7 @@ static bool virtio_transport_space_update(struct sock *sk,
> 	spin_lock_bh(&vvs->tx_lock);
> 	vvs->peer_buf_alloc = le32_to_cpu(hdr->buf_alloc);
> 	vvs->peer_fwd_cnt = le32_to_cpu(hdr->fwd_cnt);
>-	space_available = virtio_transport_has_space(vsk);
>+	space_available = virtio_transport_has_space(vvs);
> 	spin_unlock_bh(&vvs->tx_lock);
> 	return space_available;
> }
>-- 2.52.0
>

LGTM!

Reviewed-by: Luigi Leonardi <leonardi@...hat.com>