From: Fiona Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH qemu] add patch fixing DMA reentrancy issues
Date: Tue, 21 Feb 2023 10:08:59 +0100 [thread overview]
Message-ID: <20230221090859.36780-1-f.ebner@proxmox.com> (raw)
that could lead to use-after-frees and stack overflows with a
malicious (or buggy) guest. See [0] for a good summary:
[0]: https://lore.kernel.org/qemu-devel/CAFEAcA_23vc7hE3iaM-JVA6W38LK4hJoWae5KcknhPRD5fPBZA@mail.gmail.com
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
...-memory-prevent-dma-reentracy-issues.patch | 118 ++++++++++++++++++
debian/patches/series | 1 +
2 files changed, 119 insertions(+)
create mode 100644 debian/patches/extra/0008-memory-prevent-dma-reentracy-issues.patch
diff --git a/debian/patches/extra/0008-memory-prevent-dma-reentracy-issues.patch b/debian/patches/extra/0008-memory-prevent-dma-reentracy-issues.patch
new file mode 100644
index 0000000..4f3af9a
--- /dev/null
+++ b/debian/patches/extra/0008-memory-prevent-dma-reentracy-issues.patch
@@ -0,0 +1,118 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Alexander Bulekov <alxndr@bu.edu>
+Date: Sat, 4 Feb 2023 23:07:34 -0500
+Subject: [PATCH] memory: prevent dma-reentracy issues
+
+Add a flag to the DeviceState, when a device is engaged in PIO/MMIO/DMA.
+This flag is set/checked prior to calling a device's MemoryRegion
+handlers, and set when device code initiates DMA. The purpose of this
+flag is to prevent two types of DMA-based reentrancy issues:
+
+1.) mmio -> dma -> mmio case
+2.) bh -> dma write -> mmio case
+
+These issues have led to problems such as stack-exhaustion and
+use-after-frees.
+
+Summary of the problem from Peter Maydell:
+https://lore.kernel.org/qemu-devel/CAFEAcA_23vc7hE3iaM-JVA6W38LK4hJoWae5KcknhPRD5fPBZA@mail.gmail.com
+
+Resolves: https://gitlab.com/qemu-project/qemu/-/issues/62
+Resolves: https://gitlab.com/qemu-project/qemu/-/issues/540
+Resolves: https://gitlab.com/qemu-project/qemu/-/issues/541
+Resolves: https://gitlab.com/qemu-project/qemu/-/issues/556
+Resolves: https://gitlab.com/qemu-project/qemu/-/issues/557
+Resolves: https://gitlab.com/qemu-project/qemu/-/issues/827
+Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1282
+
+Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
+Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
+Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
+Acked-by: Peter Xu <peterx@redhat.com>
+(picked-up from https://lists.nongnu.org/archive/html/qemu-devel/2023-02/msg01142.html)
+Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
+---
+ include/hw/qdev-core.h | 7 +++++++
+ softmmu/memory.c | 17 +++++++++++++++++
+ softmmu/trace-events | 1 +
+ 3 files changed, 25 insertions(+)
+
+diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
+index 785dd5a56e..886f6bb79e 100644
+--- a/include/hw/qdev-core.h
++++ b/include/hw/qdev-core.h
+@@ -162,6 +162,10 @@ struct NamedClockList {
+ QLIST_ENTRY(NamedClockList) node;
+ };
+
++typedef struct {
++ bool engaged_in_io;
++} MemReentrancyGuard;
++
+ /**
+ * DeviceState:
+ * @realized: Indicates whether the device has been fully constructed.
+@@ -194,6 +198,9 @@ struct DeviceState {
+ int alias_required_for_version;
+ ResettableState reset;
+ GSList *unplug_blockers;
++
++ /* Is the device currently in mmio/pio/dma? Used to prevent re-entrancy */
++ MemReentrancyGuard mem_reentrancy_guard;
+ };
+
+ struct DeviceListener {
+diff --git a/softmmu/memory.c b/softmmu/memory.c
+index bc0be3f62c..7dcb3347aa 100644
+--- a/softmmu/memory.c
++++ b/softmmu/memory.c
+@@ -533,6 +533,7 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
+ uint64_t access_mask;
+ unsigned access_size;
+ unsigned i;
++ DeviceState *dev = NULL;
+ MemTxResult r = MEMTX_OK;
+
+ if (!access_size_min) {
+@@ -542,6 +543,19 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
+ access_size_max = 4;
+ }
+
++ /* Do not allow more than one simultanous access to a device's IO Regions */
++ if (mr->owner &&
++ !mr->ram_device && !mr->ram && !mr->rom_device && !mr->readonly) {
++ dev = (DeviceState *) object_dynamic_cast(mr->owner, TYPE_DEVICE);
++ if (dev) {
++ if (dev->mem_reentrancy_guard.engaged_in_io) {
++ trace_memory_region_reentrant_io(get_cpu_index(), mr, addr, size);
++ return MEMTX_ERROR;
++ }
++ dev->mem_reentrancy_guard.engaged_in_io = true;
++ }
++ }
++
+ /* FIXME: support unaligned access? */
+ access_size = MAX(MIN(size, access_size_max), access_size_min);
+ access_mask = MAKE_64BIT_MASK(0, access_size * 8);
+@@ -556,6 +570,9 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
+ access_mask, attrs);
+ }
+ }
++ if (dev) {
++ dev->mem_reentrancy_guard.engaged_in_io = false;
++ }
+ return r;
+ }
+
+diff --git a/softmmu/trace-events b/softmmu/trace-events
+index 22606dc27b..62d04ea9a7 100644
+--- a/softmmu/trace-events
++++ b/softmmu/trace-events
+@@ -13,6 +13,7 @@ memory_region_ops_read(int cpu_index, void *mr, uint64_t addr, uint64_t value, u
+ memory_region_ops_write(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size, const char *name) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u name '%s'"
+ memory_region_subpage_read(int cpu_index, void *mr, uint64_t offset, uint64_t value, unsigned size) "cpu %d mr %p offset 0x%"PRIx64" value 0x%"PRIx64" size %u"
+ memory_region_subpage_write(int cpu_index, void *mr, uint64_t offset, uint64_t value, unsigned size) "cpu %d mr %p offset 0x%"PRIx64" value 0x%"PRIx64" size %u"
++memory_region_reentrant_io(int cpu_index, void *mr, uint64_t offset, unsigned size) "cpu %d mr %p offset 0x%"PRIx64" size %u"
+ memory_region_ram_device_read(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"
+ memory_region_ram_device_write(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"
+ memory_region_sync_dirty(const char *mr, const char *listener, int global) "mr '%s' listener '%s' synced (global=%d)"
diff --git a/debian/patches/series b/debian/patches/series
index 06c775f..3ecc9d9 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -5,6 +5,7 @@ extra/0004-virtio-mem-Fix-the-iterator-variable-in-a-vmem-rdl_l.patch
extra/0005-vhost-fix-vq-dirty-bitmap-syncing-when-vIOMMU-is-ena.patch
extra/0006-virtio-rng-pci-fix-migration-compat-for-vectors.patch
extra/0007-block-fix-detect-zeroes-with-BDRV_REQ_REGISTERED_BUF.patch
+extra/0008-memory-prevent-dma-reentracy-issues.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
next reply other threads:[~2023-02-21 9:09 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-21 9:08 Fiona Ebner [this message]
2023-02-21 9:40 ` [pve-devel] applied: " Thomas Lamprecht
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230221090859.36780-1-f.ebner@proxmox.com \
--to=f.ebner@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.