|
| From: | Song Gao |
| Subject: | Re: [PATCH v10 1/8] memory: prevent dma-reentracy issues |
| Date: | Sat, 6 May 2023 17:25:40 +0800 |
| User-agent: | Mozilla/5.0 (X11; Linux loongarch64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 |
Hi Alexander 在 2023/4/28 下午5:14, Thomas Huth 写道:
This patch causes the loongarch virtual machine to fail to start the slave cpu.On 28/04/2023 11.11, Alexander Bulekov wrote:On 230428 1015, Thomas Huth wrote:On 28/04/2023 10.12, Daniel P. Berrangé wrote:On Thu, Apr 27, 2023 at 05:10:06PM -0400, Alexander Bulekov wrote: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 MemoryRegionhandlers, and set when device code initiates DMA. The purpose of thisflag 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.comResolves: 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 <alxndr@bu.edu> Reviewed-by: Thomas Huth <thuth@redhat.com> --- include/exec/memory.h | 5 +++++ include/hw/qdev-core.h | 7 +++++++ softmmu/memory.c | 16 ++++++++++++++++ 3 files changed, 28 insertions(+) diff --git a/include/exec/memory.h b/include/exec/memory.h index 15ade918ba..e45ce6061f 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -767,6 +767,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; @@ -791,6 +793,9 @@ struct MemoryRegion { unsigned ioeventfd_nb; MemoryRegionIoeventfd *ioeventfds; RamDiscardManager *rdm; /* Only for RAM */ ++ /* 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 bd50ad5ee1..7623703943 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 b1a6cae6f5..fe23f0e5ce 100644 --- a/softmmu/memory.c +++ b/softmmu/memory.c@@ -542,6 +542,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("Blocked re-entrant IO on " + "MemoryRegion: %s at addr: 0x%" HWADDR_PRIX, + memory_region_name(mr), addr); + return MEMTX_ACCESS_ERROR;If we issue this warn_report on every invalid memory access, is this going to become a denial of service by flooding logs, or is the return MEMTX_ACCESS_ERROR, sufficient to ensure this is only printed *once* in the lifetime of the QEMU process ?Maybe it's better to use warn_report_once() here instead?Sounds good - should I respin the series to change this?Not necessary, I've got v10 already queued, I'll fix it up there Thomas
./build/qemu-system-loongarch64 -machine virt -m 8G -cpu la464 \-smp 4 -bios QEMU_EFI.fd -kernel vmlinuz.efi -initrd ramdisk \ -serial stdio -monitor telnet:localhost:4495,server,nowait \ -append "root=/dev/ram rdinit=/sbin/init console=ttyS0,115200" --nographic
....qemu-system-loongarch64: warning: Blocked re-entrant IO on MemoryRegion: loongarch_ipi_iocsr at addr: 0x24
.... [ 0.059284] smp: Bringing up secondary CPUs ... [ 0.062540] Booting CPU#1... [ 5.204340] CPU1: failed to start [ 5.211435] Booting CPU#2... [ 10.465762] CPU2: failed to start [ 10.467757] Booting CPU#3... [ 15.805430] CPU3: failed to start [ 15.805980] smp: Brought up 1 node, 1 CPU [ 15.818832] devtmpfs: initialized[ 15.830287] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 7645041785100000 ns [ 15.830429] futex hash table entries: 1024 (order: 2, 65536 bytes, linear)
[ 15.840470] NET: Registered PF_NETLINK/PF_ROUTE protocol family [ 15.845424] audit: initializing netlink subsys (disabled) ... gdb (gdb) nqemu-system-loongarch64: warning: Blocked re-entrant IO on MemoryRegion: loongarch_ipi_iocsr at addr: 0x24
552 return MEMTX_ACCESS_ERROR; (gdb) bt #0 0x0000555555ae93aa in access_with_adjusted_size(addr=36, value=0x7fffd299f988, size=4, access_size_min=<optimized out>, access_size_max=<optimized out>, access_fn= 0x555555ae98f0 <memory_region_write_accessor>, mr=0x555556e3a740, attrs=...) at ../softmmu/memory.c:552 #1 0x0000555555ae962c in memory_region_dispatch_write (mr=0x555556e3a740, addr=36, data=<optimized out>, op=<optimized out>, attrs=...)
at ../softmmu/memory.c:1531#2 0x0000555555af5739 in address_space_stl_internal (endian=DEVICE_NATIVE_ENDIAN, result=0x0, attrs=..., val=0, addr=<optimized out>, as=0x5555567b4bb8) at /home3/gaosong/code/vec/github/send/v2/qemu/include/exec/memory.h:2997 #3 0x0000555555af5739 in address_space_stl (as=0x5555567b4bb8, addr=<optimized out>, val=0, attrs=..., result=0x0)
at /home3/gaosong/code/vec/github/send/v2/qemu/memory_ldst.c.inc:350 #4 0x0000555555ae99e8 in memory_region_write_accessor(mr=0x555556e3ab80, addr=0, value=<optimized out>, size=8, shift=<optimized out>, mask=<optimized out>, attrs=...) at ../softmmu/memory.c:493
#5 0x0000555555ae92ae in access_with_adjusted_size(addr=0, value=0x7fffd299fb88, size=8, access_size_min=<optimized out>, access_size_max=<optimized out>, access_fn= 0x555555ae98f0 <memory_region_write_accessor>, mr=0x555556e3ab80, attrs=...) at ../softmmu/memory.c:567 #6 0x0000555555ae962c in memory_region_dispatch_write (mr=0x555556e3ab80, addr=0, data=<optimized out>, op=<optimized out>, attrs=...) at ../softmmu/memory.c:1531 #7 0x0000555555af2591 in address_space_stq_internal (as=<optimized out>, addr=<optimized out>, val=2147483652, attrs=..., result=0x0, endian=DEVICE_NATIVE_ENDIAN) at /home3/gaosong/code/vec/github/send/v2/qemu/include/exec/memory.h:2997
#8 0x00007fff8d922923 in code_gen_buffer ()#9 0x0000555555b4b9e0 in cpu_tb_exec (tb_exit=<synthetic pointer>, itb=<optimized out>, cpu=0x5555567a5d00) at ../accel/tcg/cpu-exec.c:460 #10 0x0000555555b4b9e0 in cpu_loop_exec_tb (tb_exit=<synthetic pointer>, last_tb=<synthetic pointer>, pc=<optimized out>, tb=<optimized out>, cpu=0x5555567a5d00)
at ../accel/tcg/cpu-exec.c:893#11 0x0000555555b4b9e0 in cpu_exec_loop (cpu=0x5555567a5d00, sc=<optimized out>) at ../accel/tcg/cpu-exec.c:1013 #12 0x0000555555b4c67d in cpu_exec_setjmp (cpu=0x5555567a5d00, sc=0x7fffd29a0220) at ../accel/tcg/cpu-exec.c:1043 #13 0x0000555555b4c746 in cpu_exec (cpu=0x5555567a5d00) at ../accel/tcg/cpu-exec.c:1069 #14 0x0000555555b634bf in tcg_cpus_exec (cpu=0x5555567a5d00) at ../accel/tcg/tcg-accel-ops.c:81 #15 0x0000555555b63613 in mttcg_cpu_thread_fn (arg=0x5555567a5d00) at ../accel/tcg/tcg-accel-ops-mttcg.c:95 #16 0x0000555555cdefba in qemu_thread_start (args=<optimized out>) at ../util/qemu-thread-posix.c:541
#17 0x00007fffefcb717a in start_thread () at /lib64/libpthread.so.0 #18 0x00007fffef9e6dc3 in clone () at /lib64/libc.so.6 About LoongArch: - docs/system/loongarch/virt.rst Thanks. Song Gao
| [Prev in Thread] | Current Thread | [Next in Thread] |