[PATCH] vhost/vsock: prevent stale IOTLB after ACCESS_PLATFORM changes

[PATCH] vhost/vsock: prevent stale IOTLB after ACCESS_PLATFORM changes

[Jia Jia]

vhost_vsock_set_features() initializes dev->iotlb when
VIRTIO_F_ACCESS_PLATFORM is enabled. It does not remove that IOTLB
when the feature is later cleared. The virtqueue still points at the
old IOTLB, and vhost_vsock_handle_tx_kick() passes descriptors to
vhost_get_vq_desc(), which translates them through that mapping.

A userspace backend can enable ACCESS_PLATFORM, install an IOTLB entry
for a payload GPA, start the device, clear ACCESS_PLATFORM, replace the
memory table, and reuse the old HVA before submitting the same GPA
again. The feature state then says direct memory access is in use
while the TX path still uses the old IOTLB HVA.

Reject clearing ACCESS_PLATFORM while the device IOTLB exists. Also
keep the existing IOTLB when a feature update leaves ACCESS_PLATFORM
enabled; VHOST_SET_FEATURES is used for runtime log updates and must
not discard the current translations by allocating an empty IOTLB.

Fixes: e13a6915a03f ("vhost/vsock: add IOTLB API support")
Signed-off-by: Jia Jia <physicalmtea@gmail.com>
---
 drivers/vhost/vsock.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index ae01457ea2cd..57e8fd1eb670 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -798,6 +798,7 @@ static int vhost_vsock_set_cid(struct vhost_vsock *vsock, u64 guest_cid)
 static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64 features)
 {
 	struct vhost_virtqueue *vq;
+	int ret = -EFAULT;
 	int i;
 
 	if (features & ~VHOST_VSOCK_FEATURES)
@@ -809,7 +810,14 @@ static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64 features)
 		goto err;
 	}
 
-	if ((features & (1ULL << VIRTIO_F_ACCESS_PLATFORM))) {
+	if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)) &&
+	    vsock->dev.iotlb) {
+		ret = -EBUSY;
+		goto err;
+	}
+
+	if ((features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)) &&
+	    !vsock->dev.iotlb) {
 		if (vhost_init_device_iotlb(&vsock->dev))
 			goto err;
 	}
@@ -827,7 +835,7 @@ static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64 features)
 
 err:
 	mutex_unlock(&vsock->dev.mutex);
-	return -EFAULT;
+	return ret;
 }
 
 static long vhost_vsock_dev_ioctl(struct file *f, unsigned int ioctl,
-- 
2.34.1

Re: [PATCH] vhost/vsock: prevent stale IOTLB after ACCESS_PLATFORM changes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The identical IOTLB-discarding bug and stale IOTLB UAF vulnerability fixed in vhost_vsock remain unpatched in the sibling vhost_net subsystem.
--

--- Patch [1]: [PATCH] vhost/vsock: prevent stale IOTLB after ACCESS_PLATFORM changes ---
commit 6249693cea8dab5e61026ab282a77f329039d592
Author: Jia Jia <physicalmtea@gmail.com>

vhost/vsock: prevent stale IOTLB after ACCESS_PLATFORM changes

This commit fixes a vulnerability in vhost_vsock where clearing the
VIRTIO_F_ACCESS_PLATFORM feature leaves a stale IOTLB. It also prevents
discarding the current translations during runtime log updates by avoiding
empty IOTLB allocations.

> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -865,7 +866,14 @@ static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64 features)
>  		goto err;
>  	}
>  
> -	if ((features & (1ULL << VIRTIO_F_ACCESS_PLATFORM))) {
> +	if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)) &&
> +	    vsock->dev.iotlb) {
> +		ret = -EBUSY;
> +		goto err;
> +	}
> +
> +	if ((features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)) &&
> +	    !vsock->dev.iotlb) {
>  		if (vhost_init_device_iotlb(&vsock->dev))
>  			goto err;
>  	}

[Severity: High]
This is a pre-existing issue, but does the identical bug still exist in the
sibling vhost_net subsystem?

Looking at drivers/vhost/net.c:vhost_net_set_features(), it appears to
still use the unconditional allocation logic:

drivers/vhost/net.c:vhost_net_set_features() {
    ...
    if (virtio_features_test_bit(features, VIRTIO_F_ACCESS_PLATFORM)) {
        if (vhost_init_device_iotlb(&n->dev))
            goto out_unlock;
    }
    ...
}

When a virtual machine uses vIOMMU, could a feature update (such as
userspace enabling log access during live migration) cause
vhost_net_set_features() to unconditionally call vhost_init_device_iotlb(),
destroying all current network DMA translations and breaking connectivity?

Additionally, does this function also fail to reject clearing the feature
while an IOTLB exists, leaving the same stale IOTLB vulnerability open for
vhost_net?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730040938.1725757-1-physicalmtea@gmail.com?part=1