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

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3065 bytes --]

On Thu, Jul 30, 2026 at 12:09:38PM +0800, Jia Jia wrote:
> 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;
> +	}

This prevents one problem but there are still other issues with how
feature bit negotiation and the IOTLB are implemented:

1. VIRTIO_F_ACCESS_PLATFORM is defined by the VIRTIO spec and must not
   be change after feature bit negotiation. Please reject all feature
   bit updates except VHOST_F_LOG_ALL to comply with the VIRTIO spec and
   eliminate potential bugs in drivers/vhost/vsock.c.

2. When the device is reset, the iotlb cannot be left initialized
   because there is no guarantee that VIRTIO_F_ACCESS_PLATFORM will be
   negotiated again.

> +	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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]