Re: [PATCH] vhost-scsi: do not relock event vq mutex on send_evt fallback
flat view
From: sashiko-bot@kernel.org
Date: 2026-09-13 09:04:53
Also in:
kvm
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [Critical] vhost_scsi_do_evt_work() accesses vq->iov[out] without verifying that the guest actually provided an input buffer, leading to arbitrary user-space memory corruption.
- [High] Events are processed in LIFO order instead of FIFO, which can cause state corruption for hotplug/hotunplug sequences.
--
commit 05830c30fc2e4dc682c270a9b487ae5abb66218b
Author: Jia Jia [off-list ref]
vhost-scsi: do not relock event vq mutex on send_evt fallback
This commit resolves a deadlock during fallback event handling when the vhost
worker has disconnected. It splits event completion into an internal unlocked
helper and a locked wrapper to safely reuse the completion path.
Link: https://lore.kernel.org/all/20260905005352.1E5B01F00A3D@smtp.kernel.org/ (local)
quoted hunk ↗ jump to hunk
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c index 4f8c0260bc9e7..42abf18291b75 100644 --- a/drivers/vhost/scsi.c +++ b/drivers/vhost/scsi.c@@ -660,19 +660,26 @@ vhost_scsi_do_evt_work(struct vhost_scsi *vs, struct vhost_scsi_evt *evt) vhost_scsi_log_write(vq, vq_log, log_num); } -static void vhost_scsi_complete_events(struct vhost_scsi *vs, bool drop) +/* Caller must hold the event virtqueue mutex. */ +static void __vhost_scsi_complete_events(struct vhost_scsi *vs, bool drop) { - struct vhost_virtqueue *vq = &vs->vqs[VHOST_SCSI_VQ_EVT].vq; struct vhost_scsi_evt *evt, *t; struct llist_node *llnode; - mutex_lock(&vq->mutex); llnode = llist_del_all(&vs->vs_event_list); llist_for_each_entry_safe(evt, t, llnode, list) {
[Severity: High] This is a pre-existing issue, but does this loop deliver events to the guest in reverse order? The events are added to vs_event_list using llist_add(), which constructs a last-in, first-out list. Because lnode is retrieved locklessly with llist_del_all(), and the iteration happens without first calling llist_reverse_order(), the events appear to be processed from newest to oldest. Could this cause state corruption for hotplug and hotunplug sequences? For example, if a device is plugged and then rapidly unplugged, the guest might receive the unplug event first and the plug event second, leaving it erroneously believing the device is still present.
if (!drop) vhost_scsi_do_evt_work(vs, evt);
[Severity: Critical]
This is a pre-existing issue, but can this function access out-of-bounds or
stale memory if the guest provides an empty input buffer?
Looking at vhost_scsi_do_evt_work() in drivers/vhost/scsi.c, the code uses
vhost_get_vq_desc() which populates vq->iov:
head = vhost_get_vq_desc(vq, vq->iov,
ARRAY_SIZE(vq->iov), &out, &in,
vq_log, &log_num);
...
if ((vq->iov[out].iov_len != sizeof(struct virtio_scsi_event))) {
vq_err(vq, "Expecting virtio_scsi_event, got %zu bytes\n",
vq->iov[out].iov_len);
If a guest submits a descriptor chain with only read-only descriptors,
vhost_get_vq_desc() sets in = 0. The out variable holds the count of
read-only descriptors, meaning the first writable descriptor expected by the
host would be at index out.
Does the length check above access vq->iov[out].iov_len without verifying
that in > 0?
If in is 0, vq->iov[out] was not populated by vhost_get_vq_desc() and
contains stale data from a previous request. Could a malicious guest use
this to force the kernel to write the event structure to a previously
legitimate but now hot-unplugged host virtual address via the
__copy_to_user() call later in the function?
quoted hunk ↗ jump to hunk
vhost_scsi_free_evt(vs, evt); } +}
[ ... ] -- Sashiko AI review · https://sashiko.dev/#/patchset/20260912090420.47439-1-physicalmtea@gmail.com?part=1