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 <[email protected]> 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/[email protected] > 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/[email protected]?part=1
