Re: [PATCH v2] vhost-scsi: initialize command pools for late I/O virtqueues

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] vhost-scsi: double fetch of vq_index in SET_VRING_ADDR
--

commit 9fca211b81e6c842310a6804151d7dcf766af409
Author: Linfeng Sun <linfeng.sun.dev@gmail.com>

vhost-scsi: initialize command pools for late I/O virtqueues

This commit delays the allocation of command pools for skipped I/O
virtqueues during VHOST_SCSI_SET_ENDPOINT. When VHOST_SET_VRING_ADDR
subsequently completes their setup, it allocates the pools and activates
only the newly configured queue.

Link: https://patch.msgid.link/20260908-fix-warning-in-sbitmap_get-v1-1-767c2c211a2b@gmail.com

> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index 4f8c0260bc9e7..efca343626664 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c

[ ... ]

> @@ -2464,12 +2503,19 @@ vhost_scsi_ioctl(struct file *f,
>  		mutex_unlock(&vs->dev.mutex);
>  		return r;
>  	default:
> +		if (ioctl == VHOST_SET_VRING_ADDR &&
> +		    get_user(vq_index, (u32 __user *)argp))
> +			return -EFAULT;
> +
>  		mutex_lock(&vs->dev.mutex);
>  		r = vhost_dev_ioctl(&vs->dev, ioctl, argp);
> -		if (r == -ENOIOCTLCMD)
> +		if (r == -ENOIOCTLCMD) {
>  			r = vhost_vring_ioctl(&vs->dev, ioctl, argp);
> -		else
> +			if (!r && ioctl == VHOST_SET_VRING_ADDR)
> +				r = vhost_scsi_activate_vq(vs, vq_index);

[Severity: Medium]
Could this introduce a double fetch vulnerability?

The index is fetched first here in vhost_scsi_ioctl():
    get_user(vq_index, (u32 __user *)argp)

And then later in the call chain it fetches the index again from the
same user pointer:

vhost_vring_ioctl() -> vhost_get_vq_from_user():
    get_user(idx, idxp);

If a malicious userspace thread (such as a VMM) modifies the index in the
payload between these two fetches, could it decouple virtqueue configuration
from activation? This might lead to state confusion where one virtqueue gets
its addresses configured by vhost_vring_set_num_addr(), while a completely
different virtqueue index (vq_index from the first fetch) is unexpectedly
activated by vhost_scsi_activate_vq().

While array_index_nospec() in vhost_scsi_activate_vq() prevents out-of-bounds
accesses, could activating a queue that wasn't actually targeted for address
configuration lead to unintended behavior?

> +		} else {
>  			vhost_scsi_flush(vs);
> +		}
>  		mutex_unlock(&vs->dev.mutex);
>  		return r;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916-fix-warning-in-sbitmap_get-v2-1-e61a09047089@gmail.com?part=1