[PATCH] vhost-scsi: flush backend after device ioctls

[PATCH] vhost-scsi: flush backend after device ioctls

[Jia Jia]

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.

Signed-off-by: Jia Jia <physicalmtea@gmail.com>
---
 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
+			vhost_scsi_flush(vs);
 		mutex_unlock(&vs->dev.mutex);
 		return r;
 	}
 }
-- 
2.43.0

Re: [PATCH] vhost-scsi: flush backend after device ioctls

[Mike Christie]

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 <physicalmtea@gmail.com>
> ---
>  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;

Re: [PATCH] vhost-scsi: flush backend after device ioctls

[Jia Jia]

The changelog is too long. I wrote it incrementally while investigating
the stale response-HVA case. English is not my first language, so some parts
came out wrong. In particular, saying that the ioctl waits for pre-update
commands may have suggested a full stop-new -> flush -> swap -> start-new
sequence. That was not what I meant; sorry about the confusion. If the patch
is otherwise acceptable, I will shorten the changelog in v2.

I wrote the patch myself. I used AI assistance only for notes and wording
help; it did not author or submit the code.

For this specific stale-response-HVA issue, I believe the patch is correct.
The patch is a return barrier: vhost_dev_ioctl() publishes the new memory
table, and vhost_scsi_flush() switches each vhost virtqueue to a new
inflight generation, flushes the vhost work, and waits for the old
generation's references. The completion path copies the response through
cmd->tvc_resp_iovs before releasing the old-generation reference. There is
no separate stop-new phase or full quiesce.

A command can arrive after vhost_dev_ioctl() returns and before
vhost_scsi_flush() switches that virtqueue's generation. It is assigned to the
old generation and is included in the flush, but vhost_set_memory() has
already updated that virtqueue's memory table, so its response iov uses the
new table. It therefore does not introduce another stale-HVA case. This is
why stopping new commands is not needed here.

An old-generation command may complete while this ioctl is still running in
the kernel, including while it is blocked in vhost_scsi_flush(); that is
expected. The same userspace thread cannot perform the remap and follow-up
TUR before this ioctl returns, because the thread is still blocked inside the
ioctl. The owner must keep the old mappings valid until the ioctl returns.
Remapping or dropping them from another userspace thread before then is
outside the lifetime assumption of this transition barrier.