Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Polling I/O (via `virtblk_poll`) can dereference a freed or NULL `vblk->vqs` because the queue is unfrozen before `vblk->vqs` is freed, and polling paths do not check if the queue is quiesced. -- commit f53b3954fde7a5b53ac6d49c8006ad87ad21054b Author: Cong Zhang <[email protected]> virtio_blk: use synchronous quiesce in virtblk_freeze_priv() This commit replaces blk_mq_quiesce_queue_nowait() with blk_mq_quiesce_queue() in virtblk_freeze_priv(). The queue is explicitly quiesced synchronously to prevent dispatch code from continuing to run and accessing deleted virtqueues. Link: https://lore.kernel.org/[email protected] > diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c > index 32bf3ba07a9dc..7a570591e040a 100644 > --- a/drivers/block/virtio_blk.c > +++ b/drivers/block/virtio_blk.c > @@ -1595,7 +1595,7 @@ static int virtblk_freeze_priv(struct virtio_device > *vdev) > > /* Ensure no requests in virtqueues before deleting vqs. */ > memflags = blk_mq_freeze_queue(q); > - blk_mq_quiesce_queue_nowait(q); > + blk_mq_quiesce_queue(q); > blk_mq_unfreeze_queue(q, memflags); [Severity: High] This isn't a bug introduced by this patch, but does unfreezing the queue here leave polling I/O exposed to a use-after-free or NULL pointer dereference? If virtio_blk is configured with poll_queues > 0, polling is enabled. Because the queue is explicitly unfrozen before vblk->vqs is freed later in virtblk_freeze_priv(), a userspace process can submit new polled requests (e.g., via preadv2(RWF_HIPRI)). These requests can successfully enter the queue and increment q_usage_counter. The process can then call blk_rq_poll(), which bypasses the standard dispatch path and directly calls the driver's poll callback (virtblk_poll()). Since polling paths do not check if the queue is quiesced, virtblk_poll() can calculate a pointer using the now-freed vblk->vqs and attempt to acquire a spinlock, resulting in an immediate use-after-free or NULL pointer dereference: virtblk_freeze_priv() { ... blk_mq_unfreeze_queue(q, memflags); // Queue unfrozen virtio_reset_device(vdev); flush_work(&vblk->config_work); vdev->config->del_vqs(vdev); kfree(vblk->vqs); // vqs freed vblk->vqs = NULL; ... } virtblk_poll() { ... struct virtio_blk_vq *vq = get_virtio_blk_vq(hctx); ... spin_lock_irqsave(&vq->lock, flags); // UAF / NULL dereference ... } This patch aims to fix a structurally identical race in the dispatch path by using a synchronous quiesce, but it appears to leave this polling race unresolved. > > /* Ensure we don't receive any more interrupts */ -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
