vp_modern_avq_cleanup() detaches unused buffers from the admin
virtqueue. 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, from del_vqs.

Reported-by: Sashiko <[email protected]>
Link: 
https://lore.kernel.org/virtualization/[email protected]/
Fixes: 4c3b54af907e ("virtio_pci_modern: use completion instead of busy loop to 
wait on admin cmd result")
Cc: Jiri Pirko <[email protected]>
Assisted-by: LLM
Signed-off-by: Michael S. Tsirkin <[email protected]>
---

Notes (changelog):
    v3->v4:
        split patch 3: avq cleanup move is now a separate
        patch from callback sync removal. Callback sync
        removal (previously modern-only in patch 3) merged
        with legacy removal into a single patch.
    
    v2->v3:
        new in v3

 drivers/virtio/virtio_pci_modern.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/virtio/virtio_pci_modern.c 
b/drivers/virtio/virtio_pci_modern.c
index 6d8ae2a6a8ca..b4249afd7f58 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -364,6 +364,12 @@ static void vp_modern_avq_cleanup(struct virtio_device 
*vdev)
        }
 }
 
+static void vp_modern_del_vqs(struct virtio_device *vdev)
+{
+       vp_modern_avq_cleanup(vdev);
+       vp_del_vqs(vdev);
+}
+
 static void vp_transport_features(struct virtio_device *vdev, u64 features)
 {
        struct virtio_pci_device *vp_dev = to_vp_device(vdev);
@@ -558,8 +564,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);
 }
@@ -1232,7 +1236,7 @@ static const struct virtio_config_ops 
virtio_pci_config_nodev_ops = {
        .set_status     = vp_set_status,
        .reset          = vp_reset,
        .find_vqs       = vp_modern_find_vqs,
-       .del_vqs        = vp_del_vqs,
+       .del_vqs        = vp_modern_del_vqs,
        .synchronize_cbs = vp_synchronize_vectors,
        .get_extended_features = vp_get_features,
        .finalize_features = vp_finalize_features,
@@ -1252,7 +1256,7 @@ static const struct virtio_config_ops 
virtio_pci_config_ops = {
        .set_status     = vp_set_status,
        .reset          = vp_reset,
        .find_vqs       = vp_modern_find_vqs,
-       .del_vqs        = vp_del_vqs,
+       .del_vqs        = vp_modern_del_vqs,
        .synchronize_cbs = vp_synchronize_vectors,
        .get_extended_features = vp_get_features,
        .finalize_features = vp_finalize_features,
-- 
MST


Reply via email to