[PATCH v3 3/3] virtio_pci_modern: move avq cleanup from reset to del_vqs

[Michael S. Tsirkin]

vp_modern_avq_cleanup() detaches unused buffers from the admin
virtqueue and completes pending commands with -EIO. Calling it
from vp_reset() is incorrect: virtqueue_get_buf in the avq
interrupt handler can race with virtqueue_detach_unused_buf in
cleanup, and get_buf after detach is not documented as valid.

The root cause is that detaching buffers does not belong in
reset at all - reset quiesces the device, while cleanup belongs
where the virtqueue is about to be destroyed, in del_vqs.

Move the call to vp_del_vqs(), which runs after
virtio_synchronize_cbs() has already guaranteed that no
interrupt handler is in progress, eliminating the race.

Reported-by: sashiko-bot@xxxxxxxxxx
Closes: https://lore.kernel.org/virtualization/20260911125745.E0A2F1F00899@xxxxxxxxxxxxxxx/
Fixes: 4c3b54af907e ("virtio_pci_modern: use completion instead of busy loop to wait on admin cmd result")
Cc: Jiri Pirko <jiri@xxxxxxxxxxx>
Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
Assisted-by: LLM
---
New in v3. v2 dropped sync from modern vp_reset in one combined
patch, leaving avq_cleanup in vp_reset. v3 moves avq_cleanup out
of vp_reset entirely into vp_del_vqs, fixing the race. Adds NULL
check for admin_vq.info for find_vqs error paths.

 drivers/virtio/virtio_pci_common.c |  2 ++
 drivers/virtio/virtio_pci_common.h |  1 +
 drivers/virtio/virtio_pci_modern.c | 10 ++++------
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index b90c174450b2..28b254ee4726 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -270,6 +270,8 @@ void vp_del_vqs(struct virtio_device *vdev)
 	struct virtqueue *vq, *n;
 	int i;
 
+	vp_modern_avq_cleanup(vdev);
+
 	list_for_each_entry_safe(vq, n, &vdev->vqs, list) {
 		info = vp_is_avq(vdev, vq->index) ? vp_dev->admin_vq.info :
 						    vp_dev->vqs[vq->index];
diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
index 8cd01de27baf..a4ff6ec903a3 100644
--- a/drivers/virtio/virtio_pci_common.h
+++ b/drivers/virtio/virtio_pci_common.h
@@ -194,6 +194,7 @@ struct virtio_device *virtio_pci_vf_get_pf_dev(struct pci_dev *pdev);
 #endif
 
 bool vp_is_avq(struct virtio_device *vdev, unsigned int index);
+void vp_modern_avq_cleanup(struct virtio_device *vdev);
 void vp_modern_avq_done(struct virtqueue *vq);
 int vp_modern_admin_cmd_exec(struct virtio_device *vdev,
 			     struct virtio_admin_cmd *cmd);
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 6d8ae2a6a8ca..ef76f35c6b2c 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -345,7 +345,7 @@ static void vp_modern_avq_activate(struct virtio_device *vdev)
 	virtio_pci_admin_cmd_cap_init(vdev);
 }
 
-static void vp_modern_avq_cleanup(struct virtio_device *vdev)
+void vp_modern_avq_cleanup(struct virtio_device *vdev)
 {
 	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
 	struct virtio_admin_cmd *cmd;
@@ -354,6 +354,9 @@ static void vp_modern_avq_cleanup(struct virtio_device *vdev)
 	if (!virtio_has_feature(vdev, VIRTIO_F_ADMIN_VQ))
 		return;
 
+	if (!vp_dev->admin_vq.info)
+		return;
+
 	vq = vp_dev->admin_vq.info->vq;
 	if (!vq)
 		return;
@@ -557,11 +560,6 @@ static void vp_reset(struct virtio_device *vdev)
 	 */
 	while (vp_modern_get_status(mdev))
 		msleep(1);
-
-	vp_modern_avq_cleanup(vdev);
-
-	/* Flush pending VQ/configuration callbacks. */
-	vp_synchronize_vectors(vdev);
 }
 
 static int vp_active_vq(struct virtqueue *vq, u16 msix_vec)
-- 
MST