[pve-devel] [PATCH qemu 1/2] pick up fix for VirtIO PCI regressions

[Fiona Ebner <f.ebner@proxmox.com>]

Commit f06b222 ("fixes for QEMU 9.0") included a revert for the QEMU
commit 2ce6cff94d ("virtio-pci: fix use of a released vector"). That
commit caused some regressions which sounded just as bad as the fix.
Those regressions have now been addressed upstream, so pick up the fix
and drop the revert. Dropping the revert fixes the original issue that
commit 2ce6cff94d ("virtio-pci: fix use of a released vector")
addressed.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 ...tio-pci-fix-use-of-a-released-vector.patch | 87 -------------------
 ...ck-copy-before-write-fix-permission.patch} |  0
 ...-write-support-unligned-snapshot-di.patch} |  0
 ...-write-create-block_copy-bitmap-in-.patch} |  0
 ...backup-add-discard-source-parameter.patch} |  0
 ...-de-initialization-of-vhost-user-de.patch} |  0
 ...se-float_status-copy-in-sme_fmopa_s.patch} |  0
 ...Use-FPST_F16-for-SME-FMOPA-widening.patch} |  0
 ...on-and-honor-bootindex-again-for-le.patch} |  0
 ...a-bump-instruction-limit-in-scripts.patch} |  0
 ...5-block-copy-Fix-missing-graph-lock.patch} |  0
 ...do-not-operate-on-sources-from-fina.patch} |  0
 ...ix-the-use-of-an-uninitialized-irqfd.patch | 77 ++++++++++++++++
 debian/patches/series                         | 24 ++---
 14 files changed, 89 insertions(+), 99 deletions(-)
 delete mode 100644 debian/patches/extra/0006-Revert-virtio-pci-fix-use-of-a-released-vector.patch
 rename debian/patches/extra/{0007-block-copy-before-write-fix-permission.patch => 0006-block-copy-before-write-fix-permission.patch} (100%)
 rename debian/patches/extra/{0008-block-copy-before-write-support-unligned-snapshot-di.patch => 0007-block-copy-before-write-support-unligned-snapshot-di.patch} (100%)
 rename debian/patches/extra/{0009-block-copy-before-write-create-block_copy-bitmap-in-.patch => 0008-block-copy-before-write-create-block_copy-bitmap-in-.patch} (100%)
 rename debian/patches/extra/{0010-qapi-blockdev-backup-add-discard-source-parameter.patch => 0009-qapi-blockdev-backup-add-discard-source-parameter.patch} (100%)
 rename debian/patches/extra/{0011-hw-virtio-Fix-the-de-initialization-of-vhost-user-de.patch => 0010-hw-virtio-Fix-the-de-initialization-of-vhost-user-de.patch} (100%)
 rename debian/patches/extra/{0012-target-arm-Use-float_status-copy-in-sme_fmopa_s.patch => 0011-target-arm-Use-float_status-copy-in-sme_fmopa_s.patch} (100%)
 rename debian/patches/extra/{0013-target-arm-Use-FPST_F16-for-SME-FMOPA-widening.patch => 0012-target-arm-Use-FPST_F16-for-SME-FMOPA-widening.patch} (100%)
 rename debian/patches/extra/{0014-scsi-fix-regression-and-honor-bootindex-again-for-le.patch => 0013-scsi-fix-regression-and-honor-bootindex-again-for-le.patch} (100%)
 rename debian/patches/extra/{0015-hw-scsi-lsi53c895a-bump-instruction-limit-in-scripts.patch => 0014-hw-scsi-lsi53c895a-bump-instruction-limit-in-scripts.patch} (100%)
 rename debian/patches/extra/{0016-block-copy-Fix-missing-graph-lock.patch => 0015-block-copy-Fix-missing-graph-lock.patch} (100%)
 rename debian/patches/extra/{0017-Revert-qemu-char-do-not-operate-on-sources-from-fina.patch => 0016-Revert-qemu-char-do-not-operate-on-sources-from-fina.patch} (100%)
 create mode 100644 debian/patches/extra/0017-virtio-pci-Fix-the-use-of-an-uninitialized-irqfd.patch

diff --git a/debian/patches/extra/0006-Revert-virtio-pci-fix-use-of-a-released-vector.patch b/debian/patches/extra/0006-Revert-virtio-pci-fix-use-of-a-released-vector.patch
deleted file mode 100644
index d2de6d1..0000000
--- a/debian/patches/extra/0006-Revert-virtio-pci-fix-use-of-a-released-vector.patch
+++ /dev/null
@@ -1,87 +0,0 @@
-From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
-From: Fiona Ebner <f.ebner@proxmox.com>
-Date: Thu, 16 May 2024 12:59:52 +0200
-Subject: [PATCH] Revert "virtio-pci: fix use of a released vector"
-
-This reverts commit 2ce6cff94df2650c460f809e5ad263f1d22507c0.
-
-The fix causes some issues:
-https://gitlab.com/qemu-project/qemu/-/issues/2321
-https://gitlab.com/qemu-project/qemu/-/issues/2334
-
-The CVE fixed by commit 2ce6cff94d ("virtio-pci: fix use of a released
-vector") is CVE-2024-4693 [0] and allows a malicious guest that
-controls the boot process in the guest to crash its QEMU process.
-
-The issues sound worse than the CVE, so revert until there is a proper
-fix.
-
-[0]: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2024-4693
-
-Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
----
- hw/virtio/virtio-pci.c | 37 ++-----------------------------------
- 1 file changed, 2 insertions(+), 35 deletions(-)
-
-diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
-index e04218a9fb..fd66713848 100644
---- a/hw/virtio/virtio-pci.c
-+++ b/hw/virtio/virtio-pci.c
-@@ -1410,38 +1410,6 @@ static int virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy,
-     return offset;
- }
- 
--static void virtio_pci_set_vector(VirtIODevice *vdev,
--                                  VirtIOPCIProxy *proxy,
--                                  int queue_no, uint16_t old_vector,
--                                  uint16_t new_vector)
--{
--    bool kvm_irqfd = (vdev->status & VIRTIO_CONFIG_S_DRIVER_OK) &&
--        msix_enabled(&proxy->pci_dev) && kvm_msi_via_irqfd_enabled();
--
--    if (new_vector == old_vector) {
--        return;
--    }
--
--    /*
--     * If the device uses irqfd and the vector changes after DRIVER_OK is
--     * set, we need to release the old vector and set up the new one.
--     * Otherwise just need to set the new vector on the device.
--     */
--    if (kvm_irqfd && old_vector != VIRTIO_NO_VECTOR) {
--        kvm_virtio_pci_vector_release_one(proxy, queue_no);
--    }
--    /* Set the new vector on the device. */
--    if (queue_no == VIRTIO_CONFIG_IRQ_IDX) {
--        vdev->config_vector = new_vector;
--    } else {
--        virtio_queue_set_vector(vdev, queue_no, new_vector);
--    }
--    /* If the new vector changed need to set it up. */
--    if (kvm_irqfd && new_vector != VIRTIO_NO_VECTOR) {
--        kvm_virtio_pci_vector_use_one(proxy, queue_no);
--    }
--}
--
- int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,
-                            uint8_t bar, uint64_t offset, uint64_t length,
-                            uint8_t id)
-@@ -1588,8 +1556,7 @@ static void virtio_pci_common_write(void *opaque, hwaddr addr,
-         } else {
-             val = VIRTIO_NO_VECTOR;
-         }
--        virtio_pci_set_vector(vdev, proxy, VIRTIO_CONFIG_IRQ_IDX,
--                              vdev->config_vector, val);
-+        vdev->config_vector = val;
-         break;
-     case VIRTIO_PCI_COMMON_STATUS:
-         if (!(val & VIRTIO_CONFIG_S_DRIVER_OK)) {
-@@ -1629,7 +1596,7 @@ static void virtio_pci_common_write(void *opaque, hwaddr addr,
-         } else {
-             val = VIRTIO_NO_VECTOR;
-         }
--        virtio_pci_set_vector(vdev, proxy, vdev->queue_sel, vector, val);
-+        virtio_queue_set_vector(vdev, vdev->queue_sel, val);
-         break;
-     case VIRTIO_PCI_COMMON_Q_ENABLE:
-         if (val == 1) {
diff --git a/debian/patches/extra/0007-block-copy-before-write-fix-permission.patch b/debian/patches/extra/0006-block-copy-before-write-fix-permission.patch
similarity index 100%
rename from debian/patches/extra/0007-block-copy-before-write-fix-permission.patch
rename to debian/patches/extra/0006-block-copy-before-write-fix-permission.patch
diff --git a/debian/patches/extra/0008-block-copy-before-write-support-unligned-snapshot-di.patch b/debian/patches/extra/0007-block-copy-before-write-support-unligned-snapshot-di.patch
similarity index 100%
rename from debian/patches/extra/0008-block-copy-before-write-support-unligned-snapshot-di.patch
rename to debian/patches/extra/0007-block-copy-before-write-support-unligned-snapshot-di.patch
diff --git a/debian/patches/extra/0009-block-copy-before-write-create-block_copy-bitmap-in-.patch b/debian/patches/extra/0008-block-copy-before-write-create-block_copy-bitmap-in-.patch
similarity index 100%
rename from debian/patches/extra/0009-block-copy-before-write-create-block_copy-bitmap-in-.patch
rename to debian/patches/extra/0008-block-copy-before-write-create-block_copy-bitmap-in-.patch
diff --git a/debian/patches/extra/0010-qapi-blockdev-backup-add-discard-source-parameter.patch b/debian/patches/extra/0009-qapi-blockdev-backup-add-discard-source-parameter.patch
similarity index 100%
rename from debian/patches/extra/0010-qapi-blockdev-backup-add-discard-source-parameter.patch
rename to debian/patches/extra/0009-qapi-blockdev-backup-add-discard-source-parameter.patch
diff --git a/debian/patches/extra/0011-hw-virtio-Fix-the-de-initialization-of-vhost-user-de.patch b/debian/patches/extra/0010-hw-virtio-Fix-the-de-initialization-of-vhost-user-de.patch
similarity index 100%
rename from debian/patches/extra/0011-hw-virtio-Fix-the-de-initialization-of-vhost-user-de.patch
rename to debian/patches/extra/0010-hw-virtio-Fix-the-de-initialization-of-vhost-user-de.patch
diff --git a/debian/patches/extra/0012-target-arm-Use-float_status-copy-in-sme_fmopa_s.patch b/debian/patches/extra/0011-target-arm-Use-float_status-copy-in-sme_fmopa_s.patch
similarity index 100%
rename from debian/patches/extra/0012-target-arm-Use-float_status-copy-in-sme_fmopa_s.patch
rename to debian/patches/extra/0011-target-arm-Use-float_status-copy-in-sme_fmopa_s.patch
diff --git a/debian/patches/extra/0013-target-arm-Use-FPST_F16-for-SME-FMOPA-widening.patch b/debian/patches/extra/0012-target-arm-Use-FPST_F16-for-SME-FMOPA-widening.patch
similarity index 100%
rename from debian/patches/extra/0013-target-arm-Use-FPST_F16-for-SME-FMOPA-widening.patch
rename to debian/patches/extra/0012-target-arm-Use-FPST_F16-for-SME-FMOPA-widening.patch
diff --git a/debian/patches/extra/0014-scsi-fix-regression-and-honor-bootindex-again-for-le.patch b/debian/patches/extra/0013-scsi-fix-regression-and-honor-bootindex-again-for-le.patch
similarity index 100%
rename from debian/patches/extra/0014-scsi-fix-regression-and-honor-bootindex-again-for-le.patch
rename to debian/patches/extra/0013-scsi-fix-regression-and-honor-bootindex-again-for-le.patch
diff --git a/debian/patches/extra/0015-hw-scsi-lsi53c895a-bump-instruction-limit-in-scripts.patch b/debian/patches/extra/0014-hw-scsi-lsi53c895a-bump-instruction-limit-in-scripts.patch
similarity index 100%
rename from debian/patches/extra/0015-hw-scsi-lsi53c895a-bump-instruction-limit-in-scripts.patch
rename to debian/patches/extra/0014-hw-scsi-lsi53c895a-bump-instruction-limit-in-scripts.patch
diff --git a/debian/patches/extra/0016-block-copy-Fix-missing-graph-lock.patch b/debian/patches/extra/0015-block-copy-Fix-missing-graph-lock.patch
similarity index 100%
rename from debian/patches/extra/0016-block-copy-Fix-missing-graph-lock.patch
rename to debian/patches/extra/0015-block-copy-Fix-missing-graph-lock.patch
diff --git a/debian/patches/extra/0017-Revert-qemu-char-do-not-operate-on-sources-from-fina.patch b/debian/patches/extra/0016-Revert-qemu-char-do-not-operate-on-sources-from-fina.patch
similarity index 100%
rename from debian/patches/extra/0017-Revert-qemu-char-do-not-operate-on-sources-from-fina.patch
rename to debian/patches/extra/0016-Revert-qemu-char-do-not-operate-on-sources-from-fina.patch
diff --git a/debian/patches/extra/0017-virtio-pci-Fix-the-use-of-an-uninitialized-irqfd.patch b/debian/patches/extra/0017-virtio-pci-Fix-the-use-of-an-uninitialized-irqfd.patch
new file mode 100644
index 0000000..055d7c0
--- /dev/null
+++ b/debian/patches/extra/0017-virtio-pci-Fix-the-use-of-an-uninitialized-irqfd.patch
@@ -0,0 +1,77 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Cindy Lu <lulu@redhat.com>
+Date: Tue, 6 Aug 2024 17:37:12 +0800
+Subject: [PATCH] virtio-pci: Fix the use of an uninitialized irqfd
+
+The crash was reported in MAC OS and NixOS, here is the link for this bug
+https://gitlab.com/qemu-project/qemu/-/issues/2334
+https://gitlab.com/qemu-project/qemu/-/issues/2321
+
+In this bug, they are using the virtio_input device. The guest notifier was
+not supported for this device, The function virtio_pci_set_guest_notifiers()
+was not called, and the vector_irqfd was not initialized.
+
+So the fix is adding the check for vector_irqfd in virtio_pci_get_notifier()
+
+The function virtio_pci_get_notifier() can be used in various devices.
+It could also be called when VIRTIO_CONFIG_S_DRIVER_OK is not set. In this situation,
+the vector_irqfd being NULL is acceptable. We can allow the device continue to boot
+
+If the vector_irqfd still hasn't been initialized after VIRTIO_CONFIG_S_DRIVER_OK
+is set, it means that the function set_guest_notifiers was not called before the
+driver started. This indicates that the device is not using the notifier.
+At this point, we will let the check fail.
+
+This fix is verified in vyatta,MacOS,NixOS,fedora system.
+
+The bt tree for this bug is:
+Thread 6 "CPU 0/KVM" received signal SIGSEGV, Segmentation fault.
+[Switching to Thread 0x7c817be006c0 (LWP 1269146)]
+kvm_virtio_pci_vq_vector_use () at ../qemu-9.0.0/hw/virtio/virtio-pci.c:817
+817         if (irqfd->users == 0) {
+(gdb) thread apply all bt
+...
+Thread 6 (Thread 0x7c817be006c0 (LWP 1269146) "CPU 0/KVM"):
+0  kvm_virtio_pci_vq_vector_use () at ../qemu-9.0.0/hw/virtio/virtio-pci.c:817
+1  kvm_virtio_pci_vector_use_one () at ../qemu-9.0.0/hw/virtio/virtio-pci.c:893
+2  0x00005983657045e2 in memory_region_write_accessor () at ../qemu-9.0.0/system/memory.c:497
+3  0x0000598365704ba6 in access_with_adjusted_size () at ../qemu-9.0.0/system/memory.c:573
+4  0x0000598365705059 in memory_region_dispatch_write () at ../qemu-9.0.0/system/memory.c:1528
+5  0x00005983659b8e1f in flatview_write_continue_step.isra.0 () at ../qemu-9.0.0/system/physmem.c:2713
+6  0x000059836570ba7d in flatview_write_continue () at ../qemu-9.0.0/system/physmem.c:2743
+7  flatview_write () at ../qemu-9.0.0/system/physmem.c:2774
+8  0x000059836570bb76 in address_space_write () at ../qemu-9.0.0/system/physmem.c:2894
+9  0x0000598365763afe in address_space_rw () at ../qemu-9.0.0/system/physmem.c:2904
+10 kvm_cpu_exec () at ../qemu-9.0.0/accel/kvm/kvm-all.c:2917
+11 0x000059836576656e in kvm_vcpu_thread_fn () at ../qemu-9.0.0/accel/kvm/kvm-accel-ops.c:50
+12 0x0000598365926ca8 in qemu_thread_start () at ../qemu-9.0.0/util/qemu-thread-posix.c:541
+13 0x00007c8185bcd1cf in ??? () at /usr/lib/libc.so.6
+14 0x00007c8185c4e504 in clone () at /usr/lib/libc.so.6
+
+Fixes: 2ce6cff94d ("virtio-pci: fix use of a released vector")
+Cc: qemu-stable@nongnu.org
+Signed-off-by: Cindy Lu <lulu@redhat.com>
+Message-Id: <20240806093715.65105-1-lulu@redhat.com>
+Acked-by: Jason Wang <jasowang@redhat.com>
+Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
+Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
+(cherry picked from commit a8e63ff289d137197ad7a701a587cc432872d798)
+Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
+---
+ hw/virtio/virtio-pci.c | 3 +++
+ 1 file changed, 3 insertions(+)
+
+diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
+index e04218a9fb..389bab003f 100644
+--- a/hw/virtio/virtio-pci.c
++++ b/hw/virtio/virtio-pci.c
+@@ -860,6 +860,9 @@ static int virtio_pci_get_notifier(VirtIOPCIProxy *proxy, int queue_no,
+     VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+     VirtQueue *vq;
+ 
++    if (!proxy->vector_irqfd && vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)
++        return -1;
++
+     if (queue_no == VIRTIO_CONFIG_IRQ_IDX) {
+         *n = virtio_config_get_guest_notifier(vdev);
+         *vector = vdev->config_vector;
diff --git a/debian/patches/series b/debian/patches/series
index c3b3117..f022897 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -3,18 +3,18 @@ extra/0002-scsi-megasas-Internal-cdbs-have-16-byte-length.patch
 extra/0003-ide-avoid-potential-deadlock-when-draining-during-tr.patch
 extra/0004-Revert-x86-acpi-workaround-Windows-not-handling-name.patch
 extra/0005-block-copy-before-write-use-uint64_t-for-timeout-in-.patch
-extra/0006-Revert-virtio-pci-fix-use-of-a-released-vector.patch
-extra/0007-block-copy-before-write-fix-permission.patch
-extra/0008-block-copy-before-write-support-unligned-snapshot-di.patch
-extra/0009-block-copy-before-write-create-block_copy-bitmap-in-.patch
-extra/0010-qapi-blockdev-backup-add-discard-source-parameter.patch
-extra/0011-hw-virtio-Fix-the-de-initialization-of-vhost-user-de.patch
-extra/0012-target-arm-Use-float_status-copy-in-sme_fmopa_s.patch
-extra/0013-target-arm-Use-FPST_F16-for-SME-FMOPA-widening.patch
-extra/0014-scsi-fix-regression-and-honor-bootindex-again-for-le.patch
-extra/0015-hw-scsi-lsi53c895a-bump-instruction-limit-in-scripts.patch
-extra/0016-block-copy-Fix-missing-graph-lock.patch
-extra/0017-Revert-qemu-char-do-not-operate-on-sources-from-fina.patch
+extra/0006-block-copy-before-write-fix-permission.patch
+extra/0007-block-copy-before-write-support-unligned-snapshot-di.patch
+extra/0008-block-copy-before-write-create-block_copy-bitmap-in-.patch
+extra/0009-qapi-blockdev-backup-add-discard-source-parameter.patch
+extra/0010-hw-virtio-Fix-the-de-initialization-of-vhost-user-de.patch
+extra/0011-target-arm-Use-float_status-copy-in-sme_fmopa_s.patch
+extra/0012-target-arm-Use-FPST_F16-for-SME-FMOPA-widening.patch
+extra/0013-scsi-fix-regression-and-honor-bootindex-again-for-le.patch
+extra/0014-hw-scsi-lsi53c895a-bump-instruction-limit-in-scripts.patch
+extra/0015-block-copy-Fix-missing-graph-lock.patch
+extra/0016-Revert-qemu-char-do-not-operate-on-sources-from-fina.patch
+extra/0017-virtio-pci-Fix-the-use-of-an-uninitialized-irqfd.patch
 bitmap-mirror/0001-drive-mirror-add-support-for-sync-bitmap-mode-never.patch
 bitmap-mirror/0002-drive-mirror-add-support-for-conditional-and-always-.patch
 bitmap-mirror/0003-mirror-add-check-for-bitmap-mode-without-bitmap.patch
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel