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 <[email protected]>
---
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
