Re: [PATCH v7 3/3] vhost/net: discard IOTLB when ACCESS_PLATFORM is cleared

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Concurrent execution of vhost_clear_device_iotlb() and translate_desc() causes the vhost worker thread to mistakenly translate IOVAs using the guest physical memory map (GPA map), leading to arbitrary guest memory corruption.
--

--- Patch [3]: [PATCH v7 3/3] vhost/net: discard IOTLB when ACCESS_PLATFORM is cleared ---
commit 4247ba53ba530a29217ab642534cab65e8a1846d
Author: Jia Jia <[email protected]>
vhost/net: discard IOTLB when ACCESS_PLATFORM is cleared

This commit applies common device IOTLB teardown when userspace clears
VIRTIO_F_ACCESS_PLATFORM. This avoids rebuilding an existing IOTLB during
feature updates that keep it enabled, while requiring userspace to
reconfigure vring addresses for the new address mode.

Link: https://lore.kernel.org/all/[email protected]/

> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 38d9c184082d0..4d9d7c2216ed5 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -1696,6 +1696,8 @@ static int vhost_net_set_features(struct vhost_net *n, const u64 *features)
>  	if (virtio_features_test_bit(features, VIRTIO_F_ACCESS_PLATFORM)) {
>  		if (vhost_init_device_iotlb(&n->dev))
>  			goto out_unlock;
> +	} else {
> +		vhost_clear_device_iotlb(&n->dev);

[Severity: Critical]
Does this introduce a race condition with the vhost worker thread that
could lead to guest memory corruption?

When userspace clears VIRTIO_F_ACCESS_PLATFORM on a running device, this
code calls vhost_clear_device_iotlb() while the backend remains attached.
Looking at vhost_clear_device_iotlb() in drivers/vhost/vhost.c, it drops
the global device IOTLB pointer before taking the per-VQ mutex:

drivers/vhost/vhost.c:vhost_clear_device_iotlb() {
	...
	d->iotlb = NULL;

	for (i = 0; i < d->nvqs; ++i) {
		struct vhost_virtqueue *vq = d->vqs[i];

		mutex_lock(&vq->mutex);
		vq->iotlb = NULL;
	...
}

Concurrently, if the vhost worker thread is active, it will have acquired
vq->mutex and proceed to translate addresses. However, translate_desc()
evaluates the global dev->iotlb pointer rather than the protected
vq->iotlb pointer:

drivers/vhost/vhost.c:translate_desc() {
	...
	struct vhost_iotlb *umem = dev->iotlb ? dev->iotlb : dev->umem;
	...
}

Since d->iotlb was cleared without the per-VQ mutex, the active worker
thread might observe dev->iotlb as NULL and incorrectly fall back to
dev->umem (the guest physical memory map).

Could this cause IOVAs to be mistakenly translated as GPAs, leading the
kernel to read or write network payloads to the wrong Host Virtual Addresses?

>  	}
>  
>  	for (i = 0; i < VHOST_NET_VQ_MAX; ++i) {

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