Re: [PATCH v4 1/3] vhost: add helper to clear device IOTLB

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Setting `d->iotlb = NULL` before acquiring `vq->mutex` in `vhost_clear_device_iotlb` creates a race condition with active vhost worker threads. This exposes the host kernel to a NULL pointer dereference or an IOMMU bypass (TOCTOU vulnerability) leading to arbitrary host memory corruption.
--

--- Patch [1]: [PATCH v4 1/3] vhost: add helper to clear device IOTLB ---
commit 721258d5c0d5f3c1aeaefcc8dc997b5bbedcd9c1
Author: Jia Jia <[email protected]>

vhost: add helper to clear device IOTLB

This patch adds a common helper for tearing down the device IOTLB shared
by vhost backends. It drops each virtqueue's IOTLB pointer and metadata
cache before freeing the old table.

> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index a0c1d54019aac..0861406f3246f 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
[ ... ]
> @@ -2298,6 +2298,30 @@ long vhost_vring_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *arg
>  }
>  EXPORT_SYMBOL_GPL(vhost_vring_ioctl);
>  
> +/* Caller must hold the device mutex. */
> +void vhost_clear_device_iotlb(struct vhost_dev *d)
> +{
> +	struct vhost_iotlb *iotlb;
> +	int i;
> +
> +	iotlb = d->iotlb;
> +	d->iotlb = NULL;
> +
> +	for (i = 0; i < d->nvqs; ++i) {

[Severity: High]
Does setting d->iotlb = NULL before acquiring vq->mutex create a race
condition with active vhost worker threads?

If a worker thread is concurrently executing vhost_copy_to_user() while
holding vq->mutex, it could call translate_desc(), which locklessly
reads dev->iotlb:

    struct vhost_iotlb *umem = dev->iotlb ? dev->iotlb : dev->umem;

Since d->iotlb was already set to NULL by vhost_clear_device_iotlb(),
the worker thread might mistakenly fall back to a potentially NULL
dev->umem. 

This could bypass the IOTLB translation and expose the host to memory
corruption via forged GIOVAs or result in a NULL pointer dereference.

> +		struct vhost_virtqueue *vq = d->vqs[i];
> +
> +		mutex_lock(&vq->mutex);
> +		vq->iotlb = NULL;
> +		__vhost_vq_meta_reset(vq);
> +		mutex_unlock(&vq->mutex);
> +	}
> +
> +	vhost_clear_msg(d);
> +	vhost_iotlb_free(iotlb);
> +	wake_up_interruptible_poll(&d->wait, EPOLLIN | EPOLLRDNORM);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1