[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

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

[Jia Jia]

On Thu, Jul 30, 2026 at 10:51:15AM -0400, Michael S. Tsirkin wrote:
> 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.
>
> This part you are fixing is harmless. Let's make it a separate patch
> and document the motivation.

Thanks.

Yes, I considered discarding the IOTLB before choosing -EBUSY. My
hesitation was not about whether it could eventually be rebuilt, but
that doing so would no longer be only a control-plane validation
change. It would modify translation state directly used by running
virtqueues.

The minimum synchronization I had in mind was roughly:

  vhost_vsock_set_features(features):
      mutex_lock(dev.mutex)

      if (!(features & ACCESS_PLATFORM) && dev.iotlb):
          lock all vq.mutex in index order

          old = dev.iotlb
          dev.iotlb = NULL

          for each vq:
              vq.iotlb = NULL
              reset vq.meta_iotlb
              vq.acked_features = features

          unlock all vq.mutex
          vhost_iotlb_free(old)

      ...

      mutex_unlock(dev.mutex)

The switch has to be serialized against all active VQs. My hesitation
was that this turns VHOST_SET_FEATURES into a datapath synchronization
point: the ioctl may wait for in-flight kick handlers, and rebuilding
an empty IOTLB may stall queues on misses until the translations are
restored. If stopping or flushing the device is also required, the
disruption is larger than a normal feature update.

I was not sure whether VHOST_SET_FEATURES is expected to impose that
cost on a running device, or whether userspace should quiesce it first.
That is why I chose -EBUSY for v1. If synchronization with active VQs is
the intended behavior here, I can rework the patch accordingly.

I will also split the existing-IOTLB preservation change into a
separate patch and document its motivation.

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

[Michael S. Tsirkin]

On Fri, Jul 31, 2026 at 12:41:45PM +0800, Jia Jia wrote:
> On Thu, Jul 30, 2026 at 10:51:15AM -0400, Michael S. Tsirkin wrote:
> > 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.
> >
> > This part you are fixing is harmless. Let's make it a separate patch
> > and document the motivation.
> 
> Thanks.
> 
> Yes, I considered discarding the IOTLB before choosing -EBUSY. My
> hesitation was not about whether it could eventually be rebuilt, but
> that doing so would no longer be only a control-plane validation
> change. It would modify translation state directly used by running
> virtqueues.
> 
> The minimum synchronization I had in mind was roughly:
> 
>   vhost_vsock_set_features(features):
>       mutex_lock(dev.mutex)
> 
>       if (!(features & ACCESS_PLATFORM) && dev.iotlb):
>           lock all vq.mutex in index order
> 
>           old = dev.iotlb
>           dev.iotlb = NULL
> 
>           for each vq:
>               vq.iotlb = NULL
>               reset vq.meta_iotlb
>               vq.acked_features = features
> 
>           unlock all vq.mutex
>           vhost_iotlb_free(old)
> 
>       ...
> 
>       mutex_unlock(dev.mutex)
> 
> The switch has to be serialized against all active VQs. My hesitation
> was that this turns VHOST_SET_FEATURES into a datapath synchronization
> point: the ioctl may wait for in-flight kick handlers, and rebuilding
> an empty IOTLB may stall queues on misses until the translations are
> restored. If stopping or flushing the device is also required, the
> disruption is larger than a normal feature update.
> 
> I was not sure whether VHOST_SET_FEATURES is expected to impose that
> cost on a running device, or whether userspace should quiesce it first.
> That is why I chose -EBUSY for v1. If synchronization with active VQs is
> the intended behavior here, I can rework the patch accordingly.
> 
> I will also split the existing-IOTLB preservation change into a
> separate patch and document its motivation.

thanks

[PATCH v2 0/2] vhost/vsock: fix device IOTLB feature lifecycle

[Jia Jia]

As suggested, split the changes into two patches.

Patch 1 detaches and frees the device IOTLB when ACCESS_PLATFORM is
cleared. The device and virtqueue IOTLB pointers, metadata caches, and
acked features are updated while all virtqueue mutexes are held. It also
drops old IOTLB miss messages and wakes readers.

Patch 2 keeps the current IOTLB when ACCESS_PLATFORM remains enabled,
so a runtime logging update does not discard valid translations.

Changes since v1:
- discard the existing IOTLB instead of returning -EBUSY
- serialize the switch against all virtqueues
- clear old IOTLB messages and wake readers
- move the existing-IOTLB preservation change to a separate patch

Jia Jia (2):
  vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared
  vhost/vsock: keep IOTLB across feature updates

 drivers/vhost/vsock.c | 46 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 40 insertions(+), 6 deletions(-)

-- 
2.34.1

[PATCH v2 1/2] vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared

[Jia Jia]

vhost_vsock_set_features() leaves the device IOTLB attached when
userspace clears VIRTIO_F_ACCESS_PLATFORM. Descriptors can therefore
continue to use translations installed before the feature change,
including HVAs made stale by a later memory table update.

Detach the IOTLB before acknowledging a feature mask without
ACCESS_PLATFORM. Hold all virtqueue mutexes in index order while clearing
the device and virtqueue IOTLB pointers, resetting metadata caches, and
updating the acknowledged features. This prevents a kick handler from
observing a mixed translation state.

Free the old IOTLB after releasing the virtqueue mutexes. Also drop
the old IOTLB miss messages and wake readers now that the device no
longer accepts IOTLB updates.

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

diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 9aaab6bb8061..562b9e139a76 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -851,6 +851,34 @@ static int vhost_vsock_set_cid(struct vhost_vsock *vsock, u64 guest_cid)
 	return 0;
 }
 
+/* Caller must hold the device mutex. */
+static void vhost_vsock_clear_iotlb(struct vhost_vsock *vsock, u64 features)
+{
+	struct vhost_iotlb *iotlb;
+	struct vhost_virtqueue *vq;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(vsock->vqs); i++)
+		mutex_lock_nested(&vsock->vqs[i].mutex, i);
+
+	iotlb = vsock->dev.iotlb;
+	vsock->dev.iotlb = NULL;
+
+	for (i = 0; i < ARRAY_SIZE(vsock->vqs); i++) {
+		vq = &vsock->vqs[i];
+		vq->iotlb = NULL;
+		memset(vq->meta_iotlb, 0, sizeof(vq->meta_iotlb));
+		vq->acked_features = features;
+	}
+
+	for (i = ARRAY_SIZE(vsock->vqs); i-- > 0;)
+		mutex_unlock(&vsock->vqs[i].mutex);
+
+	vhost_clear_msg(&vsock->dev);
+	vhost_iotlb_free(iotlb);
+	wake_up_interruptible_poll(&vsock->dev.wait, EPOLLIN | EPOLLRDNORM);
+}
+
 static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64 features)
 {
 	struct vhost_virtqueue *vq;
@@ -872,11 +900,16 @@ static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64 features)
 
 	vsock->seqpacket_allow = features & (1ULL << VIRTIO_VSOCK_F_SEQPACKET);
 
-	for (i = 0; i < ARRAY_SIZE(vsock->vqs); i++) {
-		vq = &vsock->vqs[i];
-		mutex_lock(&vq->mutex);
-		vq->acked_features = features;
-		mutex_unlock(&vq->mutex);
+	if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)) &&
+	    vsock->dev.iotlb) {
+		vhost_vsock_clear_iotlb(vsock, features);
+	} else {
+		for (i = 0; i < ARRAY_SIZE(vsock->vqs); i++) {
+			vq = &vsock->vqs[i];
+			mutex_lock(&vq->mutex);
+			vq->acked_features = features;
+			mutex_unlock(&vq->mutex);
+		}
 	}
 	mutex_unlock(&vsock->dev.mutex);
 	return 0;
-- 
2.34.1

[PATCH v2 2/2] vhost/vsock: keep IOTLB across feature updates

[Jia Jia]

VHOST_SET_FEATURES is also used to update logging while a device is
running. When ACCESS_PLATFORM stays enabled, allocating a new empty
IOTLB on every call drops valid translations and forces avoidable
misses.

Initialize the device IOTLB only when one does not already exist.

Signed-off-by: Jia Jia <physicalmtea@gmail.com>
---
 drivers/vhost/vsock.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 562b9e139a76..3703fd219039 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -893,7 +893,8 @@ 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) {
 		if (vhost_init_device_iotlb(&vsock->dev))
 			goto err;
 	}
-- 
2.34.1

Re: [PATCH v2 1/2] vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared

[sashiko-bot]

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

Pre-existing issues:
- [Critical] A NULL pointer dereference occurs in translate_desc() if both dev->iotlb and dev->umem are NULL when a descriptor is processed.
--

--- Patch [1]: [PATCH v2 1/2] vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared ---
commit 711b531d8532907f465c4af9ee082250c352dc13
Author: Jia Jia <physicalmtea@gmail.com>

vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared

This patch detaches the IOTLB before acknowledging a feature mask without
ACCESS_PLATFORM to prevent stale translations. It clears the device and
virtqueue IOTLB pointers while holding virtqueue mutexes to ensure kick
handlers do not observe mixed translation states.

> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index 9aaab6bb8061c..562b9e139a76f 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -851,6 +851,34 @@ static int vhost_vsock_set_cid(struct vhost_vsock *vsock, u64 guest_cid)
>  	return 0;
>  }
>  
> +/* Caller must hold the device mutex. */
> +static void vhost_vsock_clear_iotlb(struct vhost_vsock *vsock, u64 features)
> +{
> +	struct vhost_iotlb *iotlb;
> +	struct vhost_virtqueue *vq;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(vsock->vqs); i++)
> +		mutex_lock_nested(&vsock->vqs[i].mutex, i);
> +
> +	iotlb = vsock->dev.iotlb;
> +	vsock->dev.iotlb = NULL;
> +
> +	for (i = 0; i < ARRAY_SIZE(vsock->vqs); i++) {
> +		vq = &vsock->vqs[i];
> +		vq->iotlb = NULL;

[Severity: Critical]
This is a pre-existing issue, but by clearing dev->iotlb and vq->iotlb to
NULL here, does this expose a NULL pointer dereference in translate_desc()
if dev->umem was never initialized?

If userspace configures the vrings and kicks the virtqueue without ever
calling VHOST_SET_MEM_TABLE, dev->umem remains NULL. When
vhost_vsock_clear_iotlb() clears dev->iotlb, the next descriptor
translation could trigger this path:

drivers/vhost/vhost.c:translate_desc() {
	...
	struct vhost_iotlb *umem = dev->iotlb ? dev->iotlb : dev->umem;
	...
	while ((u64)len > s) {
		...
		map = vhost_iotlb_itree_first(umem, addr, last);

drivers/vhost/iotlb.c:vhost_iotlb_itree_first() {
	return vhost_iotlb_itree_iter_first(&iotlb->root, start, last);
}

Because umem is NULL, taking the address of iotlb->root passes NULL to the
interval tree iterator, which would cause a page fault when dereferenced.

Can a malicious guest or unprivileged userspace process trigger this by
negotiating VIRTIO_F_ACCESS_PLATFORM, avoiding VHOST_SET_MEM_TABLE, and
then clearing the feature bit dynamically?

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

Re: [PATCH v2 2/2] vhost/vsock: keep IOTLB across feature updates

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Missing bounds checking in `vhost_vq_meta_fetch()` combined with a failure to clear the metadata cache during `VHOST_SET_VRING_ADDR` allows malicious userspace to achieve arbitrary kernel read/write via vhost worker threads.
--

--- Patch [2]: [PATCH v2 2/2] vhost/vsock: keep IOTLB across feature updates ---
commit f04e21a46c433140a2f8e01687d07519c6aa6866
Author: Jia Jia <physicalmtea@gmail.com>

vhost/vsock: keep IOTLB across feature updates

This commit modifies vhost_vsock_set_features() to only initialize the
device IOTLB when one does not already exist. This prevents dropping
valid translations and forcing avoidable misses when VHOST_SET_FEATURES
is used to update logging while a device is running.

> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index 562b9e139a76f..3703fd219039b 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -893,7 +893,8 @@ 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) {
>  		if (vhost_init_device_iotlb(&vsock->dev))
>  			goto err;
>  	}

[Severity: Critical]
This is a pre-existing issue, but I noticed a potential bounds checking
problem in vhost_vq_meta_fetch() and vhost_vring_set_addr() in
drivers/vhost/vhost.c.

Does vhost_vq_meta_fetch() lack bounds checking on the requested address?

drivers/vhost/vhost.c:vhost_vq_meta_fetch() {
	const struct vhost_iotlb_map *map = vq->meta_iotlb[type];

	if (!map)
		return NULL;

	return (void __user *)(uintptr_t)(map->addr + addr - map->start);
}

When VHOST_SET_VRING_ADDR is called, vhost_vring_set_addr() updates the
vring addresses but does not appear to invalidate the meta_iotlb cache
(for example, by calling __vhost_vq_meta_reset()):

drivers/vhost/vhost.c:vhost_vring_set_addr() {
	...
	vq->log_used = !!(a.flags & (0x1 << VHOST_VRING_F_LOG));
	vq->desc = (void __user *)(unsigned long)a.desc_user_addr;
	vq->avail = (void __user *)(unsigned long)a.avail_user_addr;
	vq->log_addr = a.log_guest_addr;
	vq->used = (void __user *)(unsigned long)a.used_user_addr;

	return 0;
}

Could this allow a crafted, out-of-bounds IOVA for the vring to bypass
access_ok() checks and calculate an arbitrary kernel address if a valid
IOTLB mapping was previously established and cached?

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