Re: [PATCH v2] vhost: reset the vring metadata cache on vring reconfiguration

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

Pre-existing issues:
- [High] `vhost_put_user` falls back to `__vhost_get_user_slow` which hardcodes `VHOST_ACCESS_RO`, bypassing IOTLB write protections.
--

--- Patch [1]: [PATCH v2] vhost: reset the vring metadata cache on vring reconfiguration ---
commit 3ebd42f4cc1c99600e85fca94195264924a8cc97
Author: Jun Yang <[email protected]>
vhost: reset the vring metadata cache on vring reconfiguration

This commit ensures the vring metadata cache is reset whenever the vring is
reconfigured. This forces new addresses to be validated through the slower
access path instead of hitting stale cache entries.

> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 4c525b3e16ead..77c96993100b5 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
[ ... ]
> @@ -2126,6 +2126,14 @@ static long vhost_vring_set_num_addr(struct vhost_dev *d,
>  		BUG();
>  	}
>  
> +	/*
> +	 * The metadata cache holds the IOTLB mapping that backed the previous
> +	 * desc/avail/used addresses and vring size, both of which are being
> +	 * replaced here.  iotlb_access_ok() takes a cache hit as proof that the
> +	 * region was validated, so the stale entries have to go.
> +	 */
> +	__vhost_vq_meta_reset(vq);
> +

[Severity: High]
This is a pre-existing issue, but does resetting the metadata cache here
expose a vulnerability where IOTLB write protections can be bypassed?

When the cache is reset, a subsequent backend configuration can trigger
vhost_update_used_flags(), which calls vhost_put_user() to write to the
used ring. Because the cache is empty, vhost_put_user() falls back to
__vhost_get_user_slow().

Looking at the fallback in __vhost_get_user_slow():

drivers/vhost/vhost.c:__vhost_get_user_slow() {
    ...
	ret = translate_desc(vq, (u64)(uintptr_t)addr, size, vq->iotlb_iov,
			     ARRAY_SIZE(vq->iotlb_iov),
			     VHOST_ACCESS_RO);
    ...
}

Since translate_desc() unconditionally requests VHOST_ACCESS_RO even for
write operations via vhost_put_user(), could a guest bypass vIOMMU
restrictions by mapping target memory as read-only and letting
vhost_put_user() overwrite it?

>  	mutex_unlock(&vq->mutex);
>  
>  	return r;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1