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

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Jul 30, 2026 at 09:55:08AM -0400, Stefan Hajnoczi wrote:
> 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.

Unfortunately, vhost does not expose the feature negotiation to the
kernel.


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