Re: [PATCH 3/4] vhost/vsock: suppress EHOSTUNREACH fast-fail during CPR pause

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Memory ordering TOCTOU race allows incorrect fast-fail of packets during CPR resume.
--

commit ee9060520c490bf817a7d5354ea841c3834e4978
Author: Denis V. Lunev <den@openvz.org>

vhost/vsock: suppress EHOSTUNREACH fast-fail during CPR pause

This commit suppresses the fast-fail in vhost_transport_send_pkt() during CPR
pause by adding a cpr_paused flag. The flag is set when backends are dropped
and cleared when started, allowing packets to be queued instead of dropped
with -EHOSTUNREACH while the connection is paused.

> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index e629886e5cf84..bcaba36becd79 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
[ ... ]
> @@ -311,11 +312,17 @@ vhost_transport_send_pkt(struct sk_buff *skb, struct net *net)
>  	 * the mutex would be too expensive in this hot path, and we already have
>  	 * all the outcomes covered: if the backend becomes NULL right after the check,
>  	 * vhost_transport_do_send_pkt() will check it under the mutex anyway.
> +	 *
> +	 * Don't fast-fail if cpr_paused is set, keep queueing skbs instead.
> +	 * The kick in vhost_vsock_start() will drain them on resume.
>  	 */
>  	if (unlikely(!data_race(vhost_vq_get_backend(&vsock->vqs[VSOCK_VQ_RX])))) {
> -		rcu_read_unlock();
> -		kfree_skb(skb);
> -		return -EHOSTUNREACH;
> +		smp_rmb();	/* pairs with smp_wmb() in start/drop_backends */
> +		if (!READ_ONCE(vsock->cpr_paused)) {
> +			rcu_read_unlock();
> +			kfree_skb(skb);
> +			return -EHOSTUNREACH;
> +		}
>  	}
>  
>  	if (virtio_vsock_skb_reply(skb))
[ ... ]
> @@ -640,6 +647,9 @@ static int vhost_vsock_start(struct vhost_vsock *vsock)
>  		mutex_unlock(&vq->mutex);
>  	}
>  
> +	smp_wmb();	/* pairs with smp_rmb() in send_pkt */
> +	WRITE_ONCE(vsock->cpr_paused, false);

[Severity: High]
Does this memory ordering still allow a TOCTOU race where packets are
incorrectly fast-failed?

In vhost_vsock_drop_backends(), the writer updates variables in the order:
cpr_paused = true -> smp_wmb() -> backend = NULL.

The lockless reader in vhost_transport_send_pkt() reads in reverse order:
backend -> smp_rmb() -> cpr_paused. This correctly guarantees it won't see
backend == NULL without cpr_paused == true.

However, here in vhost_vsock_start(), the writer updates them in the same
order as the reader:
backend = vsock -> smp_wmb() -> cpr_paused = false.

Because both the reader and this writer process the backend pointer first and
cpr_paused second, the memory barriers do not prevent interleaving. Can a
concurrent reader execute its first read (backend) before the writer's
update, seeing NULL, and its second read (cpr_paused) after the writer's
update, seeing false?

This would cause the reader to erroneously drop the packet with
-EHOSTUNREACH on an active connection.

> +
>  	/* Some packets may have been queued before the device was started,
>  	 * let's kick the send worker to send them.
>  	 */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612165718.433546-1-andrey.drobyshev@virtuozzo.com?part=3