diff mbox series

[PULL,04/22] hw/virtio: Add a protection against duplicate vu_scmi_stop calls

Message ID 63a3520e29a1a68d8610315b049ccb5840fe22e9.1691101215.git.mst@redhat.com
State New
Headers show
Series [PULL,01/22] hw/virtio-iommu: Fix potential OOB access in virtio_iommu_handle_command() | expand

Commit Message

Michael S. Tsirkin Aug. 3, 2023, 10:20 p.m. UTC 
From: Milan Zamazal <mzamazal@redhat.com>

The QEMU CI fails in virtio-scmi test occasionally.  As reported by
Thomas Huth, this happens most likely when the system is loaded and it
fails with the following error:

  qemu-system-aarch64: ../../devel/qemu/hw/pci/msix.c:659:
  msix_unset_vector_notifiers: Assertion `dev->msix_vector_use_notifier && dev->msix_vector_release_notifier' failed.
  ../../devel/qemu/tests/qtest/libqtest.c:200: kill_qemu() detected QEMU death from signal 6 (Aborted) (core dumped)

As discovered by Fabiano Rosas, the cause is a duplicate invocation of
msix_unset_vector_notifiers via duplicate vu_scmi_stop calls:

  msix_unset_vector_notifiers
  virtio_pci_set_guest_notifiers
  vu_scmi_stop
  vu_scmi_disconnect
  ...
  qemu_chr_write_buffer

  msix_unset_vector_notifiers
  virtio_pci_set_guest_notifiers
  vu_scmi_stop
  vu_scmi_set_status
  ...
  qemu_cleanup

While vu_scmi_stop calls are protected by vhost_dev_is_started()
check, it's apparently not enough.  vhost-user-blk and vhost-user-gpio
use an extra protection, see f5b22d06fb (vhost: recheck dev state in
the vhost_migration_log routine) for the motivation.  Let's use the
same in vhost-user-scmi, which fixes the failure above.

Fixes: a5dab090e142 ("hw/virtio: Add boilerplate for vhost-user-scmi device")
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Message-Id: <20230720101037.2161450-1-mzamazal@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Tested-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
---
 include/hw/virtio/vhost-user-scmi.h | 1 +
 hw/virtio/vhost-user-scmi.c         | 7 +++++++
 2 files changed, 8 insertions(+)
diff mbox series

Patch

diff --git a/include/hw/virtio/vhost-user-scmi.h b/include/hw/virtio/vhost-user-scmi.h
index 6175a74ebd..c90db77dd5 100644
--- a/include/hw/virtio/vhost-user-scmi.h
+++ b/include/hw/virtio/vhost-user-scmi.h
@@ -25,6 +25,7 @@  struct VHostUserSCMI {
     VirtQueue *cmd_vq;
     VirtQueue *event_vq;
     bool connected;
+    bool started_vu;
 };
 
 #endif /* _QEMU_VHOST_USER_SCMI_H */
diff --git a/hw/virtio/vhost-user-scmi.c b/hw/virtio/vhost-user-scmi.c
index d386fb2df9..918bb7dcf7 100644
--- a/hw/virtio/vhost-user-scmi.c
+++ b/hw/virtio/vhost-user-scmi.c
@@ -63,6 +63,7 @@  static int vu_scmi_start(VirtIODevice *vdev)
         error_report("Error starting vhost-user-scmi: %d", ret);
         goto err_guest_notifiers;
     }
+    scmi->started_vu = true;
 
     /*
      * guest_notifier_mask/pending not used yet, so just unmask
@@ -90,6 +91,12 @@  static void vu_scmi_stop(VirtIODevice *vdev)
     struct vhost_dev *vhost_dev = &scmi->vhost_dev;
     int ret;
 
+    /* vhost_dev_is_started() check in the callers is not fully reliable. */
+    if (!scmi->started_vu) {
+        return;
+    }
+    scmi->started_vu = false;
+
     if (!k->set_guest_notifiers) {
         return;
     }