all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH qemu 1/2] fixup patch "ide: avoid potential deadlock when draining during trim"
@ 2023-03-09 13:37 Fiona Ebner
  2023-03-09 13:37 ` [pve-devel] [PATCH qemu 2/2] add more stable fixes Fiona Ebner
  2023-03-13 16:53 ` [pve-devel] applied-series: [PATCH qemu 1/2] fixup patch "ide: avoid potential deadlock when draining during trim" Thomas Lamprecht
  0 siblings, 2 replies; 3+ messages in thread
From: Fiona Ebner @ 2023-03-09 13:37 UTC (permalink / raw)
  To: pve-devel

The patch was incomplete and (re-)introduced an issue with a potential
failing assertion upon cancelation of the DMA request.

There is a patch on qemu-devel now[0], and it's the same as this one
code-wise (except for comments). But the discussion is still ongoing.
While there shouldn't be a real issue with the patch, there might be
better approaches. The plan is to use this as a stop-gap for now and
pick up the proper solution once it's ready.

[0]: https://lists.nongnu.org/archive/html/qemu-devel/2023-03/msg03325.html

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 ...ial-deadlock-when-draining-during-tr.patch | 28 +++++++++++++++++--
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/debian/patches/extra/0011-ide-avoid-potential-deadlock-when-draining-during-tr.patch b/debian/patches/extra/0011-ide-avoid-potential-deadlock-when-draining-during-tr.patch
index 77d7eee..8ce9c79 100644
--- a/debian/patches/extra/0011-ide-avoid-potential-deadlock-when-draining-during-tr.patch
+++ b/debian/patches/extra/0011-ide-avoid-potential-deadlock-when-draining-during-tr.patch
@@ -37,15 +37,25 @@ Thus, even after moving the blk_inc_in_flight to above the
 replay_bh_schedule_event call, the invariant "ide_issue_trim_cb
 returns with an accompanying in-flight count" is still satisfied.
 
+However, the issue 7e5cdb345f fixed for canceling resurfaces, because
+ide_cancel_dma_sync assumes that it just needs to drain once. But now
+the in_flight count is not consistently > 0 during the trim operation.
+So, change it to drain until !s->bus->dma->aiocb, which means that the
+operation finished (s->bus->dma->aiocb is cleared by ide_set_inactive
+via the ide_dma_cb when the end of the transfer is reached).
+
+Discussion here:
+https://lists.nongnu.org/archive/html/qemu-devel/2023-03/msg02506.html
+
 Fixes: 7e5cdb345f ("ide: Increment BB in-flight counter for TRIM BH")
 Suggested-by: Hanna Czenczek <hreitz@redhat.com>
 Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
 ---
- hw/ide/core.c | 7 +++----
- 1 file changed, 3 insertions(+), 4 deletions(-)
+ hw/ide/core.c | 12 ++++++------
+ 1 file changed, 6 insertions(+), 6 deletions(-)
 
 diff --git a/hw/ide/core.c b/hw/ide/core.c
-index 39afdc0006..6474522bc9 100644
+index 39afdc0006..b67c1885a8 100644
 --- a/hw/ide/core.c
 +++ b/hw/ide/core.c
 @@ -443,7 +443,7 @@ static void ide_trim_bh_cb(void *opaque)
@@ -76,3 +86,15 @@ index 39afdc0006..6474522bc9 100644
      iocb = blk_aio_get(&trim_aiocb_info, s->blk, cb, cb_opaque);
      iocb->s = s;
      iocb->bh = qemu_bh_new(ide_trim_bh_cb, iocb);
+@@ -739,8 +738,9 @@ void ide_cancel_dma_sync(IDEState *s)
+      */
+     if (s->bus->dma->aiocb) {
+         trace_ide_cancel_dma_sync_remaining();
+-        blk_drain(s->blk);
+-        assert(s->bus->dma->aiocb == NULL);
++        while (s->bus->dma->aiocb) {
++            blk_drain(s->blk);
++        }
+     }
+ }
+ 
-- 
2.30.2





^ permalink raw reply	[flat|nested] 3+ messages in thread

* [pve-devel] [PATCH qemu 2/2] add more stable fixes
  2023-03-09 13:37 [pve-devel] [PATCH qemu 1/2] fixup patch "ide: avoid potential deadlock when draining during trim" Fiona Ebner
@ 2023-03-09 13:37 ` Fiona Ebner
  2023-03-13 16:53 ` [pve-devel] applied-series: [PATCH qemu 1/2] fixup patch "ide: avoid potential deadlock when draining during trim" Thomas Lamprecht
  1 sibling, 0 replies; 3+ messages in thread
From: Fiona Ebner @ 2023-03-09 13:37 UTC (permalink / raw)
  To: pve-devel

The patches were selected from the recent "Patch Round-up for stable
7.2.1" [0]. Those that should be relevant for our supported use-cases
(and the upcoming nvme use-case) were picked. Most of the patches
added now have not been submitted to qemu-stable before.

The follow-up for the virtio-rng-pci migration fix will break
migration between versions with the fix and without the fix when a
virtio-pci-rng(-non)-transitional device is used. Luckily Proxmox VE
only uses the virtio-pci-rng device, and this was fixed by
0006-virtio-rng-pci-fix-migration-compat-for-vectors.patch which was
applied before any public version of Proxmox VE's QEMU 7.2 package was
released.

[0]: https://lists.nongnu.org/archive/html/qemu-stable/2023-03/msg00010.html
[1]: https://bugzilla.redhat.com/show_bug.cgi?id=2162569

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 ...ing-endian-conversions-for-doorbell-.patch |  67 +++++++++
 ...fix-field-corruption-in-type-4-table.patch |  50 +++++++
 ...ix-transitional-migration-compat-for.patch |  35 +++++
 ...er-hpet-Fix-expiration-time-overflow.patch |  80 +++++++++++
 ...vdpa-stop-all-svq-on-device-deletion.patch |  71 ++++++++++
 ...tential-use-of-an-uninitialized-vari.patch | 132 ++++++++++++++++++
 ...ket-set-s-listener-NULL-in-char_sock.patch |  70 ++++++++++
 ...il-MAP-notifier-without-caching-mode.patch |  41 ++++++
 ...-fail-DEVIOTLB_UNMAP-without-dt-mode.patch |  50 +++++++
 debian/patches/series                         |   9 ++
 10 files changed, 605 insertions(+)
 create mode 100644 debian/patches/extra/0012-hw-nvme-fix-missing-endian-conversions-for-doorbell-.patch
 create mode 100644 debian/patches/extra/0013-hw-smbios-fix-field-corruption-in-type-4-table.patch
 create mode 100644 debian/patches/extra/0014-virtio-rng-pci-fix-transitional-migration-compat-for.patch
 create mode 100644 debian/patches/extra/0015-hw-timer-hpet-Fix-expiration-time-overflow.patch
 create mode 100644 debian/patches/extra/0016-vdpa-stop-all-svq-on-device-deletion.patch
 create mode 100644 debian/patches/extra/0017-vhost-avoid-a-potential-use-of-an-uninitialized-vari.patch
 create mode 100644 debian/patches/extra/0018-chardev-char-socket-set-s-listener-NULL-in-char_sock.patch
 create mode 100644 debian/patches/extra/0019-intel-iommu-fail-MAP-notifier-without-caching-mode.patch
 create mode 100644 debian/patches/extra/0020-intel-iommu-fail-DEVIOTLB_UNMAP-without-dt-mode.patch

diff --git a/debian/patches/extra/0012-hw-nvme-fix-missing-endian-conversions-for-doorbell-.patch b/debian/patches/extra/0012-hw-nvme-fix-missing-endian-conversions-for-doorbell-.patch
new file mode 100644
index 0000000..aa9d0b0
--- /dev/null
+++ b/debian/patches/extra/0012-hw-nvme-fix-missing-endian-conversions-for-doorbell-.patch
@@ -0,0 +1,67 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Klaus Jensen <k.jensen@samsung.com>
+Date: Wed, 8 Mar 2023 19:57:12 +0300
+Subject: [PATCH] hw/nvme: fix missing endian conversions for doorbell buffers
+
+The eventidx and doorbell value are not handling endianness correctly.
+Fix this.
+
+Fixes: 3f7fe8de3d49 ("hw/nvme: Implement shadow doorbell buffer support")
+Cc: qemu-stable@nongnu.org
+Reported-by: Guenter Roeck <linux@roeck-us.net>
+Reviewed-by: Keith Busch <kbusch@kernel.org>
+Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
+(cherry picked from commit 2fda0726e5149e032acfa5fe442db56cd6433c4c)
+Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
+Conflicts: hw/nvme/ctrl.c
+(picked up from qemu-stable mailing list)
+Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
+---
+ hw/nvme/ctrl.c | 22 ++++++++++++++++------
+ 1 file changed, 16 insertions(+), 6 deletions(-)
+
+diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
+index e54276dc1d..98d8e34109 100644
+--- a/hw/nvme/ctrl.c
++++ b/hw/nvme/ctrl.c
+@@ -1333,8 +1333,12 @@ static inline void nvme_blk_write(BlockBackend *blk, int64_t offset,
+ 
+ static void nvme_update_cq_head(NvmeCQueue *cq)
+ {
+-    pci_dma_read(&cq->ctrl->parent_obj, cq->db_addr, &cq->head,
+-            sizeof(cq->head));
++    uint32_t v;
++
++    pci_dma_read(&cq->ctrl->parent_obj, cq->db_addr, &v, sizeof(v));
++
++    cq->head = le32_to_cpu(v);
++
+     trace_pci_nvme_shadow_doorbell_cq(cq->cqid, cq->head);
+ }
+ 
+@@ -6141,15 +6145,21 @@ static uint16_t nvme_admin_cmd(NvmeCtrl *n, NvmeRequest *req)
+ 
+ static void nvme_update_sq_eventidx(const NvmeSQueue *sq)
+ {
+-    pci_dma_write(&sq->ctrl->parent_obj, sq->ei_addr, &sq->tail,
+-                  sizeof(sq->tail));
++    uint32_t v = cpu_to_le32(sq->tail);
++
++    pci_dma_write(&sq->ctrl->parent_obj, sq->ei_addr, &v, sizeof(v));
++
+     trace_pci_nvme_eventidx_sq(sq->sqid, sq->tail);
+ }
+ 
+ static void nvme_update_sq_tail(NvmeSQueue *sq)
+ {
+-    pci_dma_read(&sq->ctrl->parent_obj, sq->db_addr, &sq->tail,
+-                 sizeof(sq->tail));
++    uint32_t v;
++
++    pci_dma_read(&sq->ctrl->parent_obj, sq->db_addr, &v, sizeof(v));
++
++    sq->tail = le32_to_cpu(v);
++
+     trace_pci_nvme_shadow_doorbell_sq(sq->sqid, sq->tail);
+ }
+ 
diff --git a/debian/patches/extra/0013-hw-smbios-fix-field-corruption-in-type-4-table.patch b/debian/patches/extra/0013-hw-smbios-fix-field-corruption-in-type-4-table.patch
new file mode 100644
index 0000000..901dbfe
--- /dev/null
+++ b/debian/patches/extra/0013-hw-smbios-fix-field-corruption-in-type-4-table.patch
@@ -0,0 +1,50 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Julia Suvorova <jusual@redhat.com>
+Date: Thu, 23 Feb 2023 13:57:47 +0100
+Subject: [PATCH] hw/smbios: fix field corruption in type 4 table
+
+Since table type 4 of SMBIOS version 2.6 is shorter than 3.0, the
+strings which follow immediately after the struct fields have been
+overwritten by unconditional filling of later fields such as core_count2.
+Make these fields dependent on the SMBIOS version.
+
+Fixes: 05e27d74c7 ("hw/smbios: add core_count2 to smbios table type 4")
+Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2169904
+
+Signed-off-by: Julia Suvorova <jusual@redhat.com>
+Message-Id: <20230223125747.254914-1-jusual@redhat.com>
+Reviewed-by: Igor Mammedov <imammedo@redhat.com>
+Reviewed-by: Ani Sinha <ani@anisinha.ca>
+Reviewed-by: Igor Mammedov <imammedo@redhat.com>
+Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
+Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
+(cherry-picked from commit 60d09b8dc7dd4256d664ad680795cb1327805b2b)
+Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
+---
+ hw/smbios/smbios.c | 8 +++++---
+ 1 file changed, 5 insertions(+), 3 deletions(-)
+
+diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
+index b4243de735..66a020999b 100644
+--- a/hw/smbios/smbios.c
++++ b/hw/smbios/smbios.c
+@@ -749,14 +749,16 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
+     t->core_count = (ms->smp.cores > 255) ? 0xFF : ms->smp.cores;
+     t->core_enabled = t->core_count;
+ 
+-    t->core_count2 = t->core_enabled2 = cpu_to_le16(ms->smp.cores);
+-
+     t->thread_count = (ms->smp.threads > 255) ? 0xFF : ms->smp.threads;
+-    t->thread_count2 = cpu_to_le16(ms->smp.threads);
+ 
+     t->processor_characteristics = cpu_to_le16(0x02); /* Unknown */
+     t->processor_family2 = cpu_to_le16(0x01); /* Other */
+ 
++    if (tbl_len == SMBIOS_TYPE_4_LEN_V30) {
++        t->core_count2 = t->core_enabled2 = cpu_to_le16(ms->smp.cores);
++        t->thread_count2 = cpu_to_le16(ms->smp.threads);
++    }
++
+     SMBIOS_BUILD_TABLE_POST;
+     smbios_type4_count++;
+ }
diff --git a/debian/patches/extra/0014-virtio-rng-pci-fix-transitional-migration-compat-for.patch b/debian/patches/extra/0014-virtio-rng-pci-fix-transitional-migration-compat-for.patch
new file mode 100644
index 0000000..d44da6b
--- /dev/null
+++ b/debian/patches/extra/0014-virtio-rng-pci-fix-transitional-migration-compat-for.patch
@@ -0,0 +1,35 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
+Date: Tue, 7 Feb 2023 17:49:44 +0000
+Subject: [PATCH] virtio-rng-pci: fix transitional migration compat for vectors
+
+In bad9c5a516 ("virtio-rng-pci: fix migration compat for vectors") I
+fixed the virtio-rng-pci migration compatibility, but it was discovered
+that we also need to fix the other aliases of the device for the
+transitional cases.
+
+Fixes: 9ea02e8f1 ('virtio-rng-pci: Allow setting nvectors, so we can use MSI-X')
+bz: https://bugzilla.redhat.com/show_bug.cgi?id=2162569
+Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
+Message-Id: <20230207174944.138255-1-dgilbert@redhat.com>
+Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
+Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
+(cherry-picked from commit 62bdb8871512076841f4464f7e26efdc7783f78d)
+Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
+---
+ hw/core/machine.c | 2 ++
+ 1 file changed, 2 insertions(+)
+
+diff --git a/hw/core/machine.c b/hw/core/machine.c
+index cd84579591..4297315984 100644
+--- a/hw/core/machine.c
++++ b/hw/core/machine.c
+@@ -43,6 +43,8 @@
+ GlobalProperty hw_compat_7_1[] = {
+     { "virtio-device", "queue_reset", "false" },
+     { "virtio-rng-pci", "vectors", "0" },
++    { "virtio-rng-pci-transitional", "vectors", "0" },
++    { "virtio-rng-pci-non-transitional", "vectors", "0" },
+ };
+ const size_t hw_compat_7_1_len = G_N_ELEMENTS(hw_compat_7_1);
+ 
diff --git a/debian/patches/extra/0015-hw-timer-hpet-Fix-expiration-time-overflow.patch b/debian/patches/extra/0015-hw-timer-hpet-Fix-expiration-time-overflow.patch
new file mode 100644
index 0000000..3c30764
--- /dev/null
+++ b/debian/patches/extra/0015-hw-timer-hpet-Fix-expiration-time-overflow.patch
@@ -0,0 +1,80 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Akihiko Odaki <akihiko.odaki@daynix.com>
+Date: Tue, 31 Jan 2023 12:00:37 +0900
+Subject: [PATCH] hw/timer/hpet: Fix expiration time overflow
+
+The expiration time provided for timer_mod() can overflow if a
+ridiculously large value is set to the comparator register. The
+resulting value can represent a past time after rounded, forcing the
+timer to fire immediately. If the timer is configured as periodic, it
+will rearm the timer again, and form an endless loop.
+
+Check if the expiration value will overflow, and if it will, stop the
+timer instead of rearming the timer with the overflowed time.
+
+This bug was found by Alexander Bulekov when fuzzing igb, a new
+network device emulation:
+https://patchew.org/QEMU/20230129053316.1071513-1-alxndr@bu.edu/
+
+The fixed test case is:
+fuzz/crash_2d7036941dcda1ad4380bb8a9174ed0c949bcefd
+
+Fixes: 16b29ae180 ("Add HPET emulation to qemu (Beth Kon)")
+Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
+Acked-by: Michael S. Tsirkin <mst@redhat.com>
+Message-Id: <20230131030037.18856-1-akihiko.odaki@daynix.com>
+Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
+Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
+(cherry-picked from commit 37d2bcbc2a4e9c2e9061bec72a32c7e49b9f81ec)
+Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
+---
+ hw/timer/hpet.c | 19 +++++++++++++------
+ 1 file changed, 13 insertions(+), 6 deletions(-)
+
+diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
+index 9520471be2..5f88ffdef8 100644
+--- a/hw/timer/hpet.c
++++ b/hw/timer/hpet.c
+@@ -352,6 +352,16 @@ static const VMStateDescription vmstate_hpet = {
+     }
+ };
+ 
++static void hpet_arm(HPETTimer *t, uint64_t ticks)
++{
++    if (ticks < ns_to_ticks(INT64_MAX / 2)) {
++        timer_mod(t->qemu_timer,
++                  qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + ticks_to_ns(ticks));
++    } else {
++        timer_del(t->qemu_timer);
++    }
++}
++
+ /*
+  * timer expiration callback
+  */
+@@ -374,13 +384,11 @@ static void hpet_timer(void *opaque)
+             }
+         }
+         diff = hpet_calculate_diff(t, cur_tick);
+-        timer_mod(t->qemu_timer,
+-                       qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + (int64_t)ticks_to_ns(diff));
++        hpet_arm(t, diff);
+     } else if (t->config & HPET_TN_32BIT && !timer_is_periodic(t)) {
+         if (t->wrap_flag) {
+             diff = hpet_calculate_diff(t, cur_tick);
+-            timer_mod(t->qemu_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
+-                           (int64_t)ticks_to_ns(diff));
++            hpet_arm(t, diff);
+             t->wrap_flag = 0;
+         }
+     }
+@@ -407,8 +415,7 @@ static void hpet_set_timer(HPETTimer *t)
+             t->wrap_flag = 1;
+         }
+     }
+-    timer_mod(t->qemu_timer,
+-                   qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + (int64_t)ticks_to_ns(diff));
++    hpet_arm(t, diff);
+ }
+ 
+ static void hpet_del_timer(HPETTimer *t)
diff --git a/debian/patches/extra/0016-vdpa-stop-all-svq-on-device-deletion.patch b/debian/patches/extra/0016-vdpa-stop-all-svq-on-device-deletion.patch
new file mode 100644
index 0000000..07166db
--- /dev/null
+++ b/debian/patches/extra/0016-vdpa-stop-all-svq-on-device-deletion.patch
@@ -0,0 +1,71 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Eugenio=20P=C3=A9rez?= <eperezma@redhat.com>
+Date: Thu, 9 Feb 2023 18:00:04 +0100
+Subject: [PATCH] vdpa: stop all svq on device deletion
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+Not stopping them leave the device in a bad state when virtio-net
+fronted device is unplugged with device_del monitor command.
+
+This is not triggable in regular poweroff or qemu forces shutdown
+because cleanup is called right after vhost_vdpa_dev_start(false).  But
+devices hot unplug does not call vdpa device cleanups.  This lead to all
+the vhost_vdpa devices without stop the SVQ but the last.
+
+Fix it and clean the code, making it symmetric with
+vhost_vdpa_svqs_start.
+
+Fixes: dff4426fa656 ("vhost: Add Shadow VirtQueue kick forwarding capabilities")
+Reported-by: Lei Yang <leiyang@redhat.com>
+Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
+Message-Id: <20230209170004.899472-1-eperezma@redhat.com>
+Tested-by: Laurent Vivier <lvivier@redhat.com>
+Acked-by: Jason Wang <jasowang@redhat.com>
+(cherry-picked from commit 2e1a9de96b487cf818a22d681cad8d3f5d18dcca)
+Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
+---
+ hw/virtio/vhost-vdpa.c | 17 ++---------------
+ 1 file changed, 2 insertions(+), 15 deletions(-)
+
+diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
+index 7468e44b87..03c78d25d8 100644
+--- a/hw/virtio/vhost-vdpa.c
++++ b/hw/virtio/vhost-vdpa.c
+@@ -707,26 +707,11 @@ static int vhost_vdpa_get_device_id(struct vhost_dev *dev,
+     return ret;
+ }
+ 
+-static void vhost_vdpa_reset_svq(struct vhost_vdpa *v)
+-{
+-    if (!v->shadow_vqs_enabled) {
+-        return;
+-    }
+-
+-    for (unsigned i = 0; i < v->shadow_vqs->len; ++i) {
+-        VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, i);
+-        vhost_svq_stop(svq);
+-    }
+-}
+-
+ static int vhost_vdpa_reset_device(struct vhost_dev *dev)
+ {
+-    struct vhost_vdpa *v = dev->opaque;
+     int ret;
+     uint8_t status = 0;
+ 
+-    vhost_vdpa_reset_svq(v);
+-
+     ret = vhost_vdpa_call(dev, VHOST_VDPA_SET_STATUS, &status);
+     trace_vhost_vdpa_reset_device(dev, status);
+     return ret;
+@@ -1088,6 +1073,8 @@ static void vhost_vdpa_svqs_stop(struct vhost_dev *dev)
+ 
+     for (unsigned i = 0; i < v->shadow_vqs->len; ++i) {
+         VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, i);
++
++        vhost_svq_stop(svq);
+         vhost_vdpa_svq_unmap_rings(dev, svq);
+     }
+ }
diff --git a/debian/patches/extra/0017-vhost-avoid-a-potential-use-of-an-uninitialized-vari.patch b/debian/patches/extra/0017-vhost-avoid-a-potential-use-of-an-uninitialized-vari.patch
new file mode 100644
index 0000000..8ce1973
--- /dev/null
+++ b/debian/patches/extra/0017-vhost-avoid-a-potential-use-of-an-uninitialized-vari.patch
@@ -0,0 +1,132 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Carlos=20L=C3=B3pez?= <clopez@suse.de>
+Date: Mon, 13 Feb 2023 09:57:47 +0100
+Subject: [PATCH] vhost: avoid a potential use of an uninitialized variable in
+ vhost_svq_poll()
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+In vhost_svq_poll(), if vhost_svq_get_buf() fails due to a device
+providing invalid descriptors, len is left uninitialized and returned
+to the caller, potentally leaking stack data or causing undefined
+behavior.
+
+Fix this by initializing len to 0.
+
+Found with GCC 13 and -fanalyzer (abridged):
+
+../hw/virtio/vhost-shadow-virtqueue.c: In function ‘vhost_svq_poll’:
+../hw/virtio/vhost-shadow-virtqueue.c:538:12: warning: use of uninitialized value ‘len’ [CWE-457] [-Wanalyzer-use-of-uninitialized-value]
+  538 |     return len;
+      |            ^~~
+  ‘vhost_svq_poll’: events 1-4
+    |
+    |  522 | size_t vhost_svq_poll(VhostShadowVirtqueue *svq)
+    |      |        ^~~~~~~~~~~~~~
+    |      |        |
+    |      |        (1) entry to ‘vhost_svq_poll’
+    |......
+    |  525 |     uint32_t len;
+    |      |              ~~~
+    |      |              |
+    |      |              (2) region created on stack here
+    |      |              (3) capacity: 4 bytes
+    |......
+    |  528 |         if (vhost_svq_more_used(svq)) {
+    |      |             ~
+    |      |             |
+    |      |             (4) inlined call to ‘vhost_svq_more_used’ from ‘vhost_svq_poll’
+
+    (...)
+
+    |  528 |         if (vhost_svq_more_used(svq)) {
+    |      |            ^~~~~~~~~~~~~~~~~~~~~~~~~
+    |      |            ||
+    |      |            |(8) ...to here
+    |      |            (7) following ‘true’ branch...
+    |......
+    |  537 |     vhost_svq_get_buf(svq, &len);
+    |      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+    |      |     |
+    |      |     (9) calling ‘vhost_svq_get_buf’ from ‘vhost_svq_poll’
+    |
+    +--> ‘vhost_svq_get_buf’: events 10-11
+           |
+           |  416 | static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq,
+           |      |                          ^~~~~~~~~~~~~~~~~
+           |      |                          |
+           |      |                          (10) entry to ‘vhost_svq_get_buf’
+           |......
+           |  423 |     if (!vhost_svq_more_used(svq)) {
+           |      |          ~
+           |      |          |
+           |      |          (11) inlined call to ‘vhost_svq_more_used’ from ‘vhost_svq_get_buf’
+           |
+
+           (...)
+
+           |
+         ‘vhost_svq_get_buf’: event 14
+           |
+           |  423 |     if (!vhost_svq_more_used(svq)) {
+           |      |        ^
+           |      |        |
+           |      |        (14) following ‘false’ branch...
+           |
+         ‘vhost_svq_get_buf’: event 15
+           |
+           |cc1:
+           | (15): ...to here
+           |
+    <------+
+    |
+  ‘vhost_svq_poll’: events 16-17
+    |
+    |  537 |     vhost_svq_get_buf(svq, &len);
+    |      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
+    |      |     |
+    |      |     (16) returning to ‘vhost_svq_poll’ from ‘vhost_svq_get_buf’
+    |  538 |     return len;
+    |      |            ~~~
+    |      |            |
+    |      |            (17) use of uninitialized value ‘len’ here
+
+Note by  Laurent Vivier <lvivier@redhat.com>:
+
+    The return value is only used to detect an error:
+
+    vhost_svq_poll
+        vhost_vdpa_net_cvq_add
+            vhost_vdpa_net_load_cmd
+                vhost_vdpa_net_load_mac
+                  -> a negative return is only used to detect error
+                vhost_vdpa_net_load_mq
+                  -> a negative return is only used to detect error
+            vhost_vdpa_net_handle_ctrl_avail
+              -> a negative return is only used to detect error
+
+Fixes: d368c0b052ad ("vhost: Do not depend on !NULL VirtQueueElement on vhost_svq_flush")
+Signed-off-by: Carlos López <clopez@suse.de>
+Message-Id: <20230213085747.19956-1-clopez@suse.de>
+Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
+Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
+(cherry-picked from commit e4dd39c699b7d63a06f686ec06ded8adbee989c1)
+Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
+---
+ hw/virtio/vhost-shadow-virtqueue.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
+index 5bd14cad96..a723073747 100644
+--- a/hw/virtio/vhost-shadow-virtqueue.c
++++ b/hw/virtio/vhost-shadow-virtqueue.c
+@@ -522,7 +522,7 @@ static void vhost_svq_flush(VhostShadowVirtqueue *svq,
+ size_t vhost_svq_poll(VhostShadowVirtqueue *svq)
+ {
+     int64_t start_us = g_get_monotonic_time();
+-    uint32_t len;
++    uint32_t len = 0;
+ 
+     do {
+         if (vhost_svq_more_used(svq)) {
diff --git a/debian/patches/extra/0018-chardev-char-socket-set-s-listener-NULL-in-char_sock.patch b/debian/patches/extra/0018-chardev-char-socket-set-s-listener-NULL-in-char_sock.patch
new file mode 100644
index 0000000..449bca8
--- /dev/null
+++ b/debian/patches/extra/0018-chardev-char-socket-set-s-listener-NULL-in-char_sock.patch
@@ -0,0 +1,70 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Yajun Wu <yajunw@nvidia.com>
+Date: Tue, 14 Feb 2023 10:14:30 +0800
+Subject: [PATCH] chardev/char-socket: set s->listener = NULL in
+ char_socket_finalize
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+After live migration with virtio block device, qemu crash at:
+
+	#0  0x000055914f46f795 in object_dynamic_cast_assert (obj=0x559151b7b090, typename=0x55914f80fbc4 "qio-channel", file=0x55914f80fb90 "/images/testvfe/sw/qemu.gerrit/include/io/channel.h", line=30, func=0x55914f80fcb8 <__func__.17257> "QIO_CHANNEL") at ../qom/object.c:872
+	#1  0x000055914f480d68 in QIO_CHANNEL (obj=0x559151b7b090) at /images/testvfe/sw/qemu.gerrit/include/io/channel.h:29
+	#2  0x000055914f4812f8 in qio_net_listener_set_client_func_full (listener=0x559151b7a720, func=0x55914f580b97 <tcp_chr_accept>, data=0x5591519f4ea0, notify=0x0, context=0x0) at ../io/net-listener.c:166
+	#3  0x000055914f580059 in tcp_chr_update_read_handler (chr=0x5591519f4ea0) at ../chardev/char-socket.c:637
+	#4  0x000055914f583dca in qemu_chr_be_update_read_handlers (s=0x5591519f4ea0, context=0x0) at ../chardev/char.c:226
+	#5  0x000055914f57b7c9 in qemu_chr_fe_set_handlers_full (b=0x559152bf23a0, fd_can_read=0x0, fd_read=0x0, fd_event=0x0, be_change=0x0, opaque=0x0, context=0x0, set_open=false, sync_state=true) at ../chardev/char-fe.c:279
+	#6  0x000055914f57b86d in qemu_chr_fe_set_handlers (b=0x559152bf23a0, fd_can_read=0x0, fd_read=0x0, fd_event=0x0, be_change=0x0, opaque=0x0, context=0x0, set_open=false) at ../chardev/char-fe.c:304
+	#7  0x000055914f378caf in vhost_user_async_close (d=0x559152bf21a0, chardev=0x559152bf23a0, vhost=0x559152bf2420, cb=0x55914f2fb8c1 <vhost_user_blk_disconnect>) at ../hw/virtio/vhost-user.c:2725
+	#8  0x000055914f2fba40 in vhost_user_blk_event (opaque=0x559152bf21a0, event=CHR_EVENT_CLOSED) at ../hw/block/vhost-user-blk.c:395
+	#9  0x000055914f58388c in chr_be_event (s=0x5591519f4ea0, event=CHR_EVENT_CLOSED) at ../chardev/char.c:61
+	#10 0x000055914f583905 in qemu_chr_be_event (s=0x5591519f4ea0, event=CHR_EVENT_CLOSED) at ../chardev/char.c:81
+	#11 0x000055914f581275 in char_socket_finalize (obj=0x5591519f4ea0) at ../chardev/char-socket.c:1083
+	#12 0x000055914f46f073 in object_deinit (obj=0x5591519f4ea0, type=0x5591519055c0) at ../qom/object.c:680
+	#13 0x000055914f46f0e5 in object_finalize (data=0x5591519f4ea0) at ../qom/object.c:694
+	#14 0x000055914f46ff06 in object_unref (objptr=0x5591519f4ea0) at ../qom/object.c:1202
+	#15 0x000055914f4715a4 in object_finalize_child_property (obj=0x559151b76c50, name=0x559151b7b250 "char3", opaque=0x5591519f4ea0) at ../qom/object.c:1747
+	#16 0x000055914f46ee86 in object_property_del_all (obj=0x559151b76c50) at ../qom/object.c:632
+	#17 0x000055914f46f0d2 in object_finalize (data=0x559151b76c50) at ../qom/object.c:693
+	#18 0x000055914f46ff06 in object_unref (objptr=0x559151b76c50) at ../qom/object.c:1202
+	#19 0x000055914f4715a4 in object_finalize_child_property (obj=0x559151b6b560, name=0x559151b76630 "chardevs", opaque=0x559151b76c50) at ../qom/object.c:1747
+	#20 0x000055914f46ef67 in object_property_del_child (obj=0x559151b6b560, child=0x559151b76c50) at ../qom/object.c:654
+	#21 0x000055914f46f042 in object_unparent (obj=0x559151b76c50) at ../qom/object.c:673
+	#22 0x000055914f58632a in qemu_chr_cleanup () at ../chardev/char.c:1189
+	#23 0x000055914f16c66c in qemu_cleanup () at ../softmmu/runstate.c:830
+	#24 0x000055914eee7b9e in qemu_default_main () at ../softmmu/main.c:38
+	#25 0x000055914eee7bcc in main (argc=86, argv=0x7ffc97cb8d88) at ../softmmu/main.c:48
+
+In char_socket_finalize after s->listener freed, event callback function
+vhost_user_blk_event will be called to handle CHR_EVENT_CLOSED.
+vhost_user_blk_event is calling qio_net_listener_set_client_func_full which
+is still using s->listener.
+
+Setting s->listener = NULL after object_unref(OBJECT(s->listener)) can
+solve this issue.
+
+Signed-off-by: Yajun Wu <yajunw@nvidia.com>
+Acked-by: Jiri Pirko <jiri@nvidia.com>
+Message-Id: <20230214021430.3638579-1-yajunw@nvidia.com>
+Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
+Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
+Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
+(cherry-picked from commit b8a7f51f59e28d5a8e0c07ed3919cc9695560ed2)
+Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
+---
+ chardev/char-socket.c | 1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/chardev/char-socket.c b/chardev/char-socket.c
+index 879564aa8a..b00efb1482 100644
+--- a/chardev/char-socket.c
++++ b/chardev/char-socket.c
+@@ -1065,6 +1065,7 @@ static void char_socket_finalize(Object *obj)
+         qio_net_listener_set_client_func_full(s->listener, NULL, NULL,
+                                               NULL, chr->gcontext);
+         object_unref(OBJECT(s->listener));
++        s->listener = NULL;
+     }
+     if (s->tls_creds) {
+         object_unref(OBJECT(s->tls_creds));
diff --git a/debian/patches/extra/0019-intel-iommu-fail-MAP-notifier-without-caching-mode.patch b/debian/patches/extra/0019-intel-iommu-fail-MAP-notifier-without-caching-mode.patch
new file mode 100644
index 0000000..f0f2d21
--- /dev/null
+++ b/debian/patches/extra/0019-intel-iommu-fail-MAP-notifier-without-caching-mode.patch
@@ -0,0 +1,41 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Jason Wang <jasowang@redhat.com>
+Date: Thu, 23 Feb 2023 14:59:20 +0800
+Subject: [PATCH] intel-iommu: fail MAP notifier without caching mode
+
+Without caching mode, MAP notifier won't work correctly since guest
+won't send IOTLB update event when it establishes new mappings in the
+I/O page tables. Let's fail the IOMMU notifiers early instead of
+misbehaving silently.
+
+Reviewed-by: Eric Auger <eric.auger@redhat.com>
+Tested-by: Viktor Prutyanov <viktor@daynix.com>
+Signed-off-by: Jason Wang <jasowang@redhat.com>
+Message-Id: <20230223065924.42503-2-jasowang@redhat.com>
+Reviewed-by: Peter Xu <peterx@redhat.com>
+Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
+Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
+(cherry-picked from commit b8d78277c091f26fdd64f239bc8bb7e55d74cecf)
+Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
+---
+ hw/i386/intel_iommu.c | 7 +++++++
+ 1 file changed, 7 insertions(+)
+
+diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
+index a08ee85edf..9143376677 100644
+--- a/hw/i386/intel_iommu.c
++++ b/hw/i386/intel_iommu.c
+@@ -3186,6 +3186,13 @@ static int vtd_iommu_notify_flag_changed(IOMMUMemoryRegion *iommu,
+                          "Snoop Control with vhost or VFIO is not supported");
+         return -ENOTSUP;
+     }
++    if (!s->caching_mode && (new & IOMMU_NOTIFIER_MAP)) {
++        error_setg_errno(errp, ENOTSUP,
++                         "device %02x.%02x.%x requires caching mode",
++                         pci_bus_num(vtd_as->bus), PCI_SLOT(vtd_as->devfn),
++                         PCI_FUNC(vtd_as->devfn));
++        return -ENOTSUP;
++    }
+ 
+     /* Update per-address-space notifier flags */
+     vtd_as->notifier_flags = new;
diff --git a/debian/patches/extra/0020-intel-iommu-fail-DEVIOTLB_UNMAP-without-dt-mode.patch b/debian/patches/extra/0020-intel-iommu-fail-DEVIOTLB_UNMAP-without-dt-mode.patch
new file mode 100644
index 0000000..ce87ea5
--- /dev/null
+++ b/debian/patches/extra/0020-intel-iommu-fail-DEVIOTLB_UNMAP-without-dt-mode.patch
@@ -0,0 +1,50 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Jason Wang <jasowang@redhat.com>
+Date: Thu, 23 Feb 2023 14:59:21 +0800
+Subject: [PATCH] intel-iommu: fail DEVIOTLB_UNMAP without dt mode
+
+Without dt mode, device IOTLB notifier won't work since guest won't
+send device IOTLB invalidation descriptor in this case. Let's fail
+early instead of misbehaving silently.
+
+Reviewed-by: Laurent Vivier <lvivier@redhat.com>
+Tested-by: Laurent Vivier <lvivier@redhat.com>
+Tested-by: Viktor Prutyanov <viktor@daynix.com>
+Buglink: https://bugzilla.redhat.com/2156876
+Signed-off-by: Jason Wang <jasowang@redhat.com>
+Message-Id: <20230223065924.42503-3-jasowang@redhat.com>
+Reviewed-by: Peter Xu <peterx@redhat.com>
+Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
+Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
+(cherry-picked from commit 09adb0e021207b60a0c51a68939b4539d98d3ef3)
+Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
+---
+ hw/i386/intel_iommu.c | 8 ++++++++
+ 1 file changed, 8 insertions(+)
+
+diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
+index 9143376677..d025ef2873 100644
+--- a/hw/i386/intel_iommu.c
++++ b/hw/i386/intel_iommu.c
+@@ -3179,6 +3179,7 @@ static int vtd_iommu_notify_flag_changed(IOMMUMemoryRegion *iommu,
+ {
+     VTDAddressSpace *vtd_as = container_of(iommu, VTDAddressSpace, iommu);
+     IntelIOMMUState *s = vtd_as->iommu_state;
++    X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s);
+ 
+     /* TODO: add support for VFIO and vhost users */
+     if (s->snoop_control) {
+@@ -3193,6 +3194,13 @@ static int vtd_iommu_notify_flag_changed(IOMMUMemoryRegion *iommu,
+                          PCI_FUNC(vtd_as->devfn));
+         return -ENOTSUP;
+     }
++    if (!x86_iommu->dt_supported && (new & IOMMU_NOTIFIER_DEVIOTLB_UNMAP)) {
++        error_setg_errno(errp, ENOTSUP,
++                         "device %02x.%02x.%x requires device IOTLB mode",
++                         pci_bus_num(vtd_as->bus), PCI_SLOT(vtd_as->devfn),
++                         PCI_FUNC(vtd_as->devfn));
++        return -ENOTSUP;
++    }
+ 
+     /* Update per-address-space notifier flags */
+     vtd_as->notifier_flags = new;
diff --git a/debian/patches/series b/debian/patches/series
index eca4614..ba3279e 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -9,6 +9,15 @@ extra/0008-memory-prevent-dma-reentracy-issues.patch
 extra/0009-block-iscsi-fix-double-free-on-BUSY-or-similar-statu.patch
 extra/0010-scsi-megasas-Internal-cdbs-have-16-byte-length.patch
 extra/0011-ide-avoid-potential-deadlock-when-draining-during-tr.patch
+extra/0012-hw-nvme-fix-missing-endian-conversions-for-doorbell-.patch
+extra/0013-hw-smbios-fix-field-corruption-in-type-4-table.patch
+extra/0014-virtio-rng-pci-fix-transitional-migration-compat-for.patch
+extra/0015-hw-timer-hpet-Fix-expiration-time-overflow.patch
+extra/0016-vdpa-stop-all-svq-on-device-deletion.patch
+extra/0017-vhost-avoid-a-potential-use-of-an-uninitialized-vari.patch
+extra/0018-chardev-char-socket-set-s-listener-NULL-in-char_sock.patch
+extra/0019-intel-iommu-fail-MAP-notifier-without-caching-mode.patch
+extra/0020-intel-iommu-fail-DEVIOTLB_UNMAP-without-dt-mode.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.30.2





^ permalink raw reply	[flat|nested] 3+ messages in thread

* [pve-devel] applied-series: [PATCH qemu 1/2] fixup patch "ide: avoid potential deadlock when draining during trim"
  2023-03-09 13:37 [pve-devel] [PATCH qemu 1/2] fixup patch "ide: avoid potential deadlock when draining during trim" Fiona Ebner
  2023-03-09 13:37 ` [pve-devel] [PATCH qemu 2/2] add more stable fixes Fiona Ebner
@ 2023-03-13 16:53 ` Thomas Lamprecht
  1 sibling, 0 replies; 3+ messages in thread
From: Thomas Lamprecht @ 2023-03-13 16:53 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fiona Ebner

Am 09/03/2023 um 14:37 schrieb Fiona Ebner:
> The patch was incomplete and (re-)introduced an issue with a potential
> failing assertion upon cancelation of the DMA request.
> 
> There is a patch on qemu-devel now[0], and it's the same as this one
> code-wise (except for comments). But the discussion is still ongoing.
> While there shouldn't be a real issue with the patch, there might be
> better approaches. The plan is to use this as a stop-gap for now and
> pick up the proper solution once it's ready.
> 
> [0]: https://lists.nongnu.org/archive/html/qemu-devel/2023-03/msg03325.html
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  ...ial-deadlock-when-draining-during-tr.patch | 28 +++++++++++++++++--
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
>

applied, thanks!




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-03-13 16:53 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-09 13:37 [pve-devel] [PATCH qemu 1/2] fixup patch "ide: avoid potential deadlock when draining during trim" Fiona Ebner
2023-03-09 13:37 ` [pve-devel] [PATCH qemu 2/2] add more stable fixes Fiona Ebner
2023-03-13 16:53 ` [pve-devel] applied-series: [PATCH qemu 1/2] fixup patch "ide: avoid potential deadlock when draining during trim" Thomas Lamprecht

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal