On 7/21/26 2:36 AM, Jia Jia wrote: > vhost-scsi translates guest response descriptors into userspace iovecs > at command submission time and later completes those commands > asynchronously through target-core. Device-wide control operations such > as VHOST_SET_MEM_TABLE replace the memory table under the device and > virtqueue mutexes, but historically returned without waiting for > outstanding SCSI commands that still hold the pre-update response > iovecs. > > After such a replacement, completion may write virtio_scsi_cmd_resp > through the old host virtual addresses. If the owner has already > remapped those addresses, the write lands on the wrong userspace object. > The kernel tree has carried a TODO for this since the 2012 split of > vhost_dev_ioctl() and vhost_vring_ioctl(): > > /* TODO: flush backend after dev ioctl. */ > > A userspace test kept a READ(10) pending, replaced the memory table so > the response GPA mapped to a new HVA, remapped the old response address > as a victim page, and then let the command complete. The completion > wrote the victim page (victim_changed=yes) and left the replacement > page unchanged; a later TUR updated the new mapping instead. So the > pending command retained the pre-update response address across > VHOST_SET_MEM_TABLE. > > That same 2012 change deliberately avoided a second backend flush on the > vring-ioctl path: vring updates already flush where appropriate, and an > extra heavy flush would hurt when kick or call fds are reconfigured on > the data path. This fix does not reintroduce that. The default branch > still routes unknown commands through vhost_dev_ioctl() first; only a > non-ENOIOCTLCMD result flushes. Vring ops such as SET_VRING_KICK/CALL, > num, addr, and base return -ENOIOCTLCMD there and fall through to > vhost_vring_ioctl() without this backend flush. > > What vhost_dev_ioctl() actually handles on this path is small: > VHOST_SET_OWNER, VHOST_SET_MEM_TABLE, VHOST_SET_LOG_BASE, > VHOST_SET_LOG_FD, and the optional fork-owner ioctls when enabled. > Flushing after those is fine: they are rare device-wide control ops, > and SET_OWNER normally runs before any inflight SCSI work. Call > vhost_scsi_flush() so pre-update worker work and target-core inflight > commands finish before the ioctl returns. As with the existing net > pattern, any non-ENOIOCTLCMD result flushes, including failures that > may have applied a partial update such as VHOST_SET_LOG_BASE. > > I later noticed vhost-net and vhost-vsock already use the same device > versus vring split. > > This is a control-plane barrier only. Ordinary submission, completion, > kick, and call paths are unchanged. The owner is expected to keep > pre-update mappings valid until the device ioctl returns. Completion > copies the response and signals from the vhost worker without needing > further userspace progress, so waiting in this ioctl does not leave the > owner process stuck on itself.
It's such a long and verbose description. Was it written by AI? Was the patch also done with AI? > > Signed-off-by: Jia Jia <[email protected]> > --- > drivers/vhost/scsi.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c > index 9a1253b9d8c5..c3e8f1a0b2d4 100644 > --- a/drivers/vhost/scsi.c > +++ b/drivers/vhost/scsi.c > @@ -2424,10 +2424,11 @@ vhost_scsi_ioctl(struct file *f, unsigned int ioctl, > unsigned long arg) > default: > mutex_lock(&vs->dev.mutex); > r = vhost_dev_ioctl(&vs->dev, ioctl, argp); > - /* TODO: flush backend after dev ioctl. */ > if (r == -ENOIOCTLCMD) > r = vhost_vring_ioctl(&vs->dev, ioctl, argp); > + else Is it still possible for userspace to execute the READ/TUR sequence describe above at this time before the flush? >From the description it sounded like we needed to stop new cmds, flush running cmds, swap the new mem pointer, then start new commands? > + vhost_scsi_flush(vs); > mutex_unlock(&vs->dev.mutex); > return r;
