Commit 3ef3d52a authored by Melbin K Mathew's avatar Melbin K Mathew Committed by Paolo Abeni
Browse files

vsock/virtio: fix potential underflow in virtio_transport_get_credit() 



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: 06a8fc78 ("VSOCK: Introduce virtio_vsock_common.ko")
Suggested-by: default avatarStefano Garzarella <sgarzare@redhat.com>
Signed-off-by: default avatarMelbin K Mathew <mlbnkm1@gmail.com>
[Stefano: use virtio_transport_has_space() instead of duplicating the code]
[Stefano: tweak the commit message]
Signed-off-by: default avatarStefano Garzarella <sgarzare@redhat.com>
Reviewed-by: default avatarLuigi Leonardi <leonardi@redhat.com>
Link: https://patch.msgid.link/20260121093628.9941-2-sgarzare@redhat.com


Acked-by: default avatarMichael S. Tsirkin <mst@redhat.com>
Signed-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
parent ca1bb3fe

net/vmw_vsock/virtio_transport_common.c

+9 −7
Original line number Diff line number Diff line
@@ -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;

}