Re: [PATCH] vhost/vdpa: reject VRING_NUM larger than device max

Re: [PATCH] vhost/vdpa: reject VRING_NUM larger than device max

[Michael S. Tsirkin]

On Sat, Aug 08, 2026 at 06:34:15PM +0800, Jia Jia wrote:
> vhost_vring_set_num() only requires a non-zero power-of-two that fits
> in 16 bits. vhost-vdpa then hands that value to the backend through
> set_vq_num() without comparing it to get_vq_num_max().
> 
> A local process with access to /dev/vhost-vdpa-* can therefore set a
> queue size larger than the device advertises. On the vdpa_sim backend,
> the worker then walks descriptors beyond the mapped descriptor ring.
> KASAN reports a 16-byte out-of-bounds read, corresponding to one
> vring_desc, in the vringh IOTLB path:
> 
>   BUG: KASAN: out-of-bounds in _copy_from_iter
>   Read of size 16
>   copy_from_iotlb
>   copydesc_iotlb
>   vringh_getdesc_iotlb
>   vdpasim_net_work
> 
> Check the requested size against get_vq_num_max() in
> vhost_vdpa_vring_ioctl() before calling vhost_vring_ioctl(), and return
> -EINVAL when the request is too large.
> 
> Fixes: 4c8cf31885f6 ("vhost: introduce vDPA-based backend")

sign off missing

> ---
>  drivers/vhost/vdpa.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index ac55275fa0d0..b2083e1ef150 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -716,6 +716,13 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd,
>  				vhost_vdpa_unsetup_vq_irq(v, idx);
>  		}
>  		break;
> +	case VHOST_SET_VRING_NUM:
> +		/* Reject sizes above the max reported by the device. */
> +		if (copy_from_user(&s, argp, sizeof(s)))
> +			return -EFAULT;
> +		if (s.num > ops->get_vq_num_max(vdpa))
> +			return -EINVAL;
> +		break;
>  	}
>  
>  	r = vhost_vring_ioctl(&v->vdev, cmd, argp);
> -- 
> 2.34.1

Re: [PATCH] vhost/vdpa: reject VRING_NUM larger than device max

[sashiko-bot]

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

New issues:
- [High] The newly added validation for VHOST_SET_VRING_NUM introduces a Time-of-Check to Time-of-Use (TOCTOU) vulnerability due to a double fetch from user memory, which completely bypasses the security fix.

Pre-existing issues:
- [High] The virtqueue index (`idx`) is fetched from userspace twice, allowing concurrent modification to desynchronize the vhost core and vDPA backend states.
--

--- Patch [1]: [PATCH] vhost/vdpa: reject VRING_NUM larger than device max ---
commit a17536b4ee293947d6ce500193f7ea4d0e4cae10
Author: Jia Jia <physicalmtea@gmail.com>

vhost/vdpa: reject VRING_NUM larger than device max

vhost_vring_set_num() only requires a non-zero power-of-two that fits
in 16 bits. This patch adds a check in vhost_vdpa_vring_ioctl() to
reject requested queue sizes larger than get_vq_num_max() before calling
vhost_vring_ioctl().

> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -723,6 +723,13 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd,
>  				vhost_vdpa_unsetup_vq_irq(v, idx);
>  		}
>  		break;
> +	case VHOST_SET_VRING_NUM:
> +		/* Reject sizes above the max reported by the device. */
> +		if (copy_from_user(&s, argp, sizeof(s)))
> +			return -EFAULT;
> +		if (s.num > ops->get_vq_num_max(vdpa))
> +			return -EINVAL;
> +		break;

[Severity: High]
Does this validation introduce a time-of-check to time-of-use vulnerability?

The requested size is fetched and validated here in vhost_vdpa_vring_ioctl(),
but the subsequent call to vhost_vring_ioctl() ultimately delegates to
vhost_vring_set_num(), which fetches the same struct from userspace a second
time:

drivers/vhost/vhost.c:vhost_vring_set_num() {
    ...
    if (copy_from_user(&s, argp, sizeof s))
    ...
    vq->num = s.num;
}

Since a concurrent thread could modify the userspace memory between the two
fetches, could this second fetch overwrite the validated size with an
excessively large value? If so, this unvalidated value would then be passed
to ops->set_vq_num() and completely bypass the security fix.

>  	}
>  
>  	r = vhost_vring_ioctl(&v->vdev, cmd, argp);

[Severity: High]
This is a pre-existing issue, but I noticed a potential state mismatch between
the vhost core and the vDPA backend.

The virtqueue index is initially fetched at the top of vhost_vdpa_vring_ioctl():

drivers/vhost/vdpa.c:vhost_vdpa_vring_ioctl() {
    ...
    r = get_user(idx, (u32 __user *)argp);
    ...
    vq = &v->vqs[idx];
    ...
}

However, the call to vhost_vring_ioctl() performs a second fetch for the index:

drivers/vhost/vhost.c:vhost_vring_ioctl() {
    ...
    r = vhost_get_vq_from_user(d, argp, &vq, &idx);
    ...
}

If a concurrent thread modifies the index in userspace between these two
fetches, could the vhost core end up updating a different queue than the one
vhost_vdpa_vring_ioctl() originally resolved? This might cause the vDPA
backend configuration (applied later in this function using the original
unverified vq and idx) to become desynchronized from the vhost core state.

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