From 6f8e3e6343d6f6b1b92ea8d890ffa8a2f407796e Mon Sep 17 00:00:00 2001 From: Alexander Bulekov Date: Thu, 27 Apr 2023 17:10:06 -0400 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 Resolves: CVE-2023-0330 Signed-off-by: Alexander Bulekov Reviewed-by: Thomas Huth Message-Id: <20230427211013.2994127-2-alxndr@bu.edu> [thuth: Replace warn_report() with warn_report_once()] Signed-off-by: Thomas Huth Signed-off-by: liuxiangdong --- include/exec/memory.h | 5 +++++ include/hw/qdev-core.h | 7 +++++++ include/net/net.h | 1 + memory.c | 16 ++++++++++++++++ 4 files changed, 29 insertions(+) diff --git a/include/exec/memory.h b/include/exec/memory.h index dca8184277..ad260ebbd6 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -365,6 +365,8 @@ struct MemoryRegion { bool is_iommu; RAMBlock *ram_block; Object *owner; + /* owner as TYPE_DEVICE. Used for re-entrancy checks in MR access hotpath */ + DeviceState *dev; const MemoryRegionOps *ops; void *opaque; @@ -387,6 +389,9 @@ struct MemoryRegion { const char *name; unsigned ioeventfd_nb; MemoryRegionIoeventfd *ioeventfds; + + /* For devices designed to perform re-entrant IO into their own IO MRs */ + bool disable_reentrancy_guard; }; struct IOMMUMemoryRegion { diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 136df7774c..364771f3a2 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -129,6 +129,10 @@ struct NamedGPIOList { QLIST_ENTRY(NamedGPIOList) node; }; +typedef struct { + bool engaged_in_io; +} MemReentrancyGuard; + /** * DeviceState: * @realized: Indicates whether the device has been fully constructed. @@ -153,6 +157,9 @@ struct DeviceState { int num_child_bus; int instance_id_alias; int alias_required_for_version; + + /* Is the device currently in mmio/pio/dma? Used to prevent re-entrancy */ + MemReentrancyGuard mem_reentrancy_guard; }; struct DeviceListener { diff --git a/include/net/net.h b/include/net/net.h index 5609b2ecba..8a362e7279 100644 --- a/include/net/net.h +++ b/include/net/net.h @@ -5,6 +5,7 @@ #include "qapi/qapi-types-net.h" #include "net/queue.h" #include "migration/vmstate.h" +#include "hw/qdev-core.h" #define MAC_FMT "%02X:%02X:%02X:%02X:%02X:%02X" #define MAC_ARG(x) ((uint8_t *)(x))[0], ((uint8_t *)(x))[1], \ diff --git a/memory.c b/memory.c index 5d8c9a9234..fa7053f9cb 100644 --- a/memory.c +++ b/memory.c @@ -561,6 +561,18 @@ static MemTxResult access_with_adjusted_size(hwaddr addr, access_size_max = 4; } + /* Do not allow more than one simultaneous access to a device's IO Regions */ + if (mr->dev && !mr->disable_reentrancy_guard && + !mr->ram_device && !mr->ram && !mr->rom_device && !mr->readonly) { + if (mr->dev->mem_reentrancy_guard.engaged_in_io) { + warn_report_once("Blocked re-entrant IO on MemoryRegion: " + "%s at addr: 0x%" HWADDR_PRIX, + memory_region_name(mr), addr); + return MEMTX_ACCESS_ERROR; + } + mr->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); @@ -575,6 +587,9 @@ static MemTxResult access_with_adjusted_size(hwaddr addr, access_mask, attrs); } } + if (mr->dev) { + mr->dev->mem_reentrancy_guard.engaged_in_io = false; + } return r; } @@ -1155,6 +1170,7 @@ static void memory_region_do_init(MemoryRegion *mr, } mr->name = g_strdup(name); mr->owner = owner; + mr->dev = (DeviceState *) object_dynamic_cast(mr->owner, TYPE_DEVICE); mr->ram_block = NULL; if (name) { -- 2.27.0