Re: [PATCH v2 4/4] vhost/vsock: add VHOST_RESET_OWNER ioctl

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing backend check in `vhost_transport_cancel_pkt()` leads to a Use-After-Free of `vq->worker` and/or a permanently stuck `VHOST_WORK_QUEUED` flag during `VHOST_RESET_OWNER`.
--

commit 1246364b7bca9df01b3f2315a384db215bb18763
Author: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>

vhost/vsock: add VHOST_RESET_OWNER ioctl

This patch adds the VHOST_RESET_OWNER ioctl for QEMU CPR migration of a
guest with a vhost-vsock device. It fences the send path around the teardown
by adding a backend check in vhost_transport_send_pkt() to prevent using
the worker after it is freed.

> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index 81d4f72097191..7d0146cd38d26 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c

[ ... ]

> @@ -903,6 +910,36 @@ static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64 features)
>  	return -EFAULT;
>  }
>  
> +static int vhost_vsock_reset_owner(struct vhost_vsock *vsock)
> +{
> +	struct vhost_iotlb *umem;
> +	long err;
> +
> +	mutex_lock(&vsock->dev.mutex);
> +	err = vhost_dev_check_owner(&vsock->dev);
> +	if (err)
> +		goto done;
> +	umem = vhost_dev_reset_owner_prepare();
> +	if (!umem) {
> +		err = -ENOMEM;
> +		goto done;
> +	}
> +	vhost_vsock_drop_backends(vsock);
> +
> +	/* Let in-flight send_pkt() callers stop touching the worker before the
> +	 * flush + free below. Pairs with the backend check in
> +	 * vhost_transport_send_pkt().
> +	 */
> +	synchronize_rcu();

[Severity: High]
Does vhost_transport_cancel_pkt() also need a backend check similar to the
one added in vhost_transport_send_pkt()?

Since VHOST_RESET_OWNER leaves the vsock hashed so connections survive,
a concurrent socket close can still look up the vsock and call
vhost_transport_cancel_pkt().

If this happens after the synchronize_rcu() call finishes,
vhost_transport_cancel_pkt() can conditionally call
vhost_poll_queue(&tx_vq->poll). This unconditionally queues work without
checking if the backend is alive, which dereferences vq->worker.

Because vhost_workers_free() frees the worker synchronously via kfree()
during teardown, could this lead to a use-after-free of the worker
or a permanently stuck VHOST_WORK_QUEUED flag?

> +
> +	vhost_vsock_flush(vsock);
> +	vhost_dev_stop(&vsock->dev);
> +	vhost_dev_reset_owner(&vsock->dev, umem);
> +done:
> +	mutex_unlock(&vsock->dev.mutex);
> +	return err;
> +}

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