[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

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

[Stefan Hajnoczi]

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

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

[Michael S. Tsirkin]

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

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

[Michael S. Tsirkin]

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>

Thanks for the patch! yet something to improve:

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


I don't know if this will break anything. Why not just blow
out the iotlb? kernel will rebuild it if it needs it afterwards
for some reason.



> +
> +	if ((features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)) &&
> +	    !vsock->dev.iotlb) {
>  		if (vhost_init_device_iotlb(&vsock->dev))
>  			goto err;
>  	}


This part you are fixing is harmless. Let's make it a separate patch
and document the motivation.

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