* [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