public inbox for pve-devel@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal