[RFC PATCH] vhost-scsi: Serialize completion notification with callfd updates

[RFC PATCH] vhost-scsi: Serialize completion notification with callfd updates

[Jia Jia]

During userspace stress testing on a KASAN-enabled host, a host-side
callfd replacement concurrent with ordinary SCSI completions exposed an
eventfd_ctx lifetime race. The test requires access to /dev/vhost-scsi and
a host process that can issue VHOST_SET_VRING_CALL; on the default device
permissions this normally means root or an explicitly delegated service.

The test used TUR completions while closing the old callfd and binding a
new one. The unbind form tends to produce a NULL pointer, while replacing
the callfd after closing the old file makes the old eventfd_ctx eligible
for release.

vhost_scsi_complete_cmd_work() drops vq->mutex before calling
vhost_signal(). VHOST_SET_VRING_CALL updates call_ctx under the same mutex
and puts the old eventfd_ctx before returning. vhost_signal() does not take
the mutex or hold a reference to call_ctx.ctx:

	if (vq->call_ctx.ctx && vhost_notify(dev, vq))
		eventfd_signal(vq->call_ctx.ctx);

	CPU 0 (ioctl / callfd replace)		CPU 1 (vhost worker)

						vhost_scsi_complete_cmd_work()
						mutex_unlock(&vq->mutex)
						vhost_signal()
						  /* load old ctx */
						  vhost_notify() ...

	VHOST_SET_VRING_CALL
	mutex_lock(&vq->mutex)
	swap(vq->call_ctx.ctx, new)
	eventfd_ctx_put(old)	/* free */
	mutex_unlock(&vq->mutex)

						eventfd_signal(old)
						  /* use-after-free */

On the reproducing run, the worker ran on CPU 1 and the callfd ioctl path
ran on CPU 0. KASAN reported:

	BUG: KASAN: slab-use-after-free in eventfd_signal_mask+0x6c/0x110

The use stack was:

	eventfd_signal_mask
	vhost_signal
	vhost_scsi_complete_cmd_work
	vhost_run_work_list
	vhost_task_fn

The freeing stack was:

	eventfd_ctx_put
	vhost_vring_ioctl
	vhost_scsi_ioctl
	__x64_sys_ioctl

Hold vq->mutex across vhost_signal() so SET_VRING_CALL cannot swap and
release the context while the completion worker is notifying the guest.
This gives the completion and callfd update a clear ordering without
changing the vhost API or adding a new lock. Other vhost-scsi response
paths already signal while holding the virtqueue mutex.

A private eventfd reference taken under the mutex would allow signalling
after unlock, but it needs additional reference-count plumbing. An RCU
design would require broader vhost-core changes for all call_ctx readers
and additional eventfd lifetime rules. Since callfd changes are a
control-plane operation, extending the existing per-virtqueue critical
section is the smaller complete fix.

This is an RFC because keeping vhost_notify() and eventfd_signal() inside
the virtqueue critical section extends the lock hold time slightly. I'd
appreciate feedback on whether this is acceptable or whether a
reference-pinning approach, taking the reference under the mutex and
releasing it after unlock, would be preferable.

Signed-off-by: Jia Jia <physicalmtea@gmail.com>
---
 drivers/vhost/scsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 9a1253b9d8c5..7f46bc0de3c2 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -735,10 +735,9 @@ static void vhost_scsi_complete_cmd_work(struct vhost_work *work)
 		vhost_scsi_release_cmd_res(se_cmd);
 	}
 
-	mutex_unlock(&svq->vq.mutex);
-
 	if (signal)
 		vhost_signal(&svq->vs->dev, &svq->vq);
+	mutex_unlock(&svq->vq.mutex);
 }
 
 static struct vhost_scsi_cmd *
-- 
2.34.1