v8-> v9:
    - Replace trace-events and attempt at making re-entrancy fatal with
      a warn_report message. This message should only be printed if a
      device is broken (and needs to be marked re-entrancy safe), or if
      something in the guest is attempting to trigger unintentional
      re-entrancy.
    - Added APIC change to the series

v7 -> v8:
    - Disable reentrancy checks for bcm2835_property's iomem (Patch 7)
    - Cache DeviceState* in the MemoryRegion to avoid dynamic cast for
      each MemoryRegion access. (Patch 1)
    - Make re-entrancy fatal for debug-builds (Patch 8)

v6 -> v7:
    - Fix bad qemu_bh_new_guarded calls found by Thomas (Patch 4)
    - Add an MR-specific flag to disable reentrancy (Patch 5)
    - Disable reentrancy checks for lsi53c895a's RAM-like MR (Patch 6)
    
    Patches 5 and 6 need review. I left the review-tags for Patch 4,
    however a few of the qemu_bh_new_guarded calls have changed.
  
v5 -> v6:
    - Only apply checkpatch checks to code in paths containing "/hw/"
      (/hw/ and include/hw/)
    - Fix a bug in a _guarded call added to hw/block/virtio-blk.c
v4-> v5:
    - Add corresponding checkpatch checks
    - Save/restore reentrancy-flag when entering/exiting BHs
    - Improve documentation
    - Check object_dynamic_cast return value
    
v3 -> v4: Instead of changing all of the DMA APIs, instead add an
    optional reentrancy guard to the BH API.
    
v2 -> v3: Bite the bullet and modify the DMA APIs, rather than
    attempting to guess DeviceStates in BHs.
    
These patches aim to solve two types of DMA-reentrancy issues:

1.) mmio -> dma -> mmio case
To solve this, we track whether the device is engaged in io by
checking/setting a reentrancy-guard within APIs used for MMIO access.

2.) bh -> dma write -> mmio case
This case is trickier, since we dont have a generic way to associate a
bh with the underlying Device/DeviceState. Thus, this version allows a
device to associate a reentrancy-guard with a bh, when creating it.
(Instead of calling qemu_bh_new, you call qemu_bh_new_guarded)

I replaced most of the qemu_bh_new invocations with the guarded analog,
except for the ones where the DeviceState was not trivially accessible.

Alexander Bulekov (8):
  memory: prevent dma-reentracy issues
  async: Add an optional reentrancy guard to the BH API
  checkpatch: add qemu_bh_new/aio_bh_new checks
  hw: replace most qemu_bh_new calls with qemu_bh_new_guarded
  lsi53c895a: disable reentrancy detection for script RAM
  bcm2835_property: disable reentrancy detection for iomem
  raven: disable reentrancy detection for iomem
  apic: disable reentrancy detection for apic-msi

 docs/devel/multiple-iothreads.txt |  7 +++++++
 hw/9pfs/xen-9p-backend.c          |  5 ++++-
 hw/block/dataplane/virtio-blk.c   |  3 ++-
 hw/block/dataplane/xen-block.c    |  5 +++--
 hw/char/virtio-serial-bus.c       |  3 ++-
 hw/display/qxl.c                  |  9 ++++++---
 hw/display/virtio-gpu.c           |  6 ++++--
 hw/ide/ahci.c                     |  3 ++-
 hw/ide/ahci_internal.h            |  1 +
 hw/ide/core.c                     |  4 +++-
 hw/intc/apic.c                    |  7 +++++++
 hw/misc/bcm2835_property.c        |  7 +++++++
 hw/misc/imx_rngc.c                |  6 ++++--
 hw/misc/macio/mac_dbdma.c         |  2 +-
 hw/net/virtio-net.c               |  3 ++-
 hw/nvme/ctrl.c                    |  6 ++++--
 hw/pci-host/raven.c               |  7 +++++++
 hw/scsi/lsi53c895a.c              |  6 ++++++
 hw/scsi/mptsas.c                  |  3 ++-
 hw/scsi/scsi-bus.c                |  3 ++-
 hw/scsi/vmw_pvscsi.c              |  3 ++-
 hw/usb/dev-uas.c                  |  3 ++-
 hw/usb/hcd-dwc2.c                 |  3 ++-
 hw/usb/hcd-ehci.c                 |  3 ++-
 hw/usb/hcd-uhci.c                 |  2 +-
 hw/usb/host-libusb.c              |  6 ++++--
 hw/usb/redirect.c                 |  6 ++++--
 hw/usb/xen-usb.c                  |  3 ++-
 hw/virtio/virtio-balloon.c        |  5 +++--
 hw/virtio/virtio-crypto.c         |  3 ++-
 include/block/aio.h               | 18 ++++++++++++++++--
 include/exec/memory.h             |  5 +++++
 include/hw/qdev-core.h            |  7 +++++++
 include/qemu/main-loop.h          |  7 +++++--
 scripts/checkpatch.pl             |  8 ++++++++
 softmmu/memory.c                  | 16 ++++++++++++++++
 tests/unit/ptimer-test-stubs.c    |  3 ++-
 util/async.c                      | 18 +++++++++++++++++-
 util/main-loop.c                  |  5 +++--
 util/trace-events                 |  1 +
 40 files changed, 180 insertions(+), 41 deletions(-)

-- 
2.39.0

On 230427 1710, Alexander Bulekov wrote:

<snip>
> These patches aim to solve two types of DMA-reentrancy issues:
> 
> 1.) mmio -> dma -> mmio case
> To solve this, we track whether the device is engaged in io by
> checking/setting a reentrancy-guard within APIs used for MMIO access.
> 
> 2.) bh -> dma write -> mmio case
> This case is trickier, since we dont have a generic way to associate a
> bh with the underlying Device/DeviceState. Thus, this version allows a
> device to associate a reentrancy-guard with a bh, when creating it.
> (Instead of calling qemu_bh_new, you call qemu_bh_new_guarded)
<snip>

Later on there was also a guard added by Akihiko Odaki for
network-backends.
7d0fefdf81f: net: Provide MemReentrancyGuard * to qemu_new_nic()

Recently a talk came out about re-entrancy bugs in qemu
(unfortuantely I could not find a non-video version of the slides):
https://www.youtube.com/watch?v=wL3LK9Dp4os

The talk gives an overview of these bugs and also demonstrates how they
can be used for VM escapes. It lists the following vectors:
 1. MMIO
 2. Bottom-Halves
 3. Netqueues
 4. IOEventfd
 5. Timers

The first three should be covered by the current defenses. However,
afaik IOEventfds and Timers are still unprotected. The talk demonstates
an attack via IOEventFd, but not one with timers. In any case, it might
make sense to revist the guard to ensure that we are covering all types
of code that perform DMA.

On 2024/11/09 4:56, Alexander Bulekov wrote:
> On 230427 1710, Alexander Bulekov wrote:
> 
> <snip>
>> These patches aim to solve two types of DMA-reentrancy issues:
>>
>> 1.) mmio -> dma -> mmio case
>> To solve this, we track whether the device is engaged in io by
>> checking/setting a reentrancy-guard within APIs used for MMIO access.
>>
>> 2.) bh -> dma write -> mmio case
>> This case is trickier, since we dont have a generic way to associate a
>> bh with the underlying Device/DeviceState. Thus, this version allows a
>> device to associate a reentrancy-guard with a bh, when creating it.
>> (Instead of calling qemu_bh_new, you call qemu_bh_new_guarded)
> <snip>
> 
> Later on there was also a guard added by Akihiko Odaki for
> network-backends.
> 7d0fefdf81f: net: Provide MemReentrancyGuard * to qemu_new_nic()
> 
> Recently a talk came out about re-entrancy bugs in qemu
> (unfortuantely I could not find a non-video version of the slides):
> https://www.youtube.com/watch?v=wL3LK9Dp4os
> 
> The talk gives an overview of these bugs and also demonstrates how they
> can be used for VM escapes. It lists the following vectors:
>   1. MMIO
>   2. Bottom-Halves
>   3. Netqueues
>   4. IOEventfd
>   5. Timers
> 
> The first three should be covered by the current defenses. However,
> afaik IOEventfds and Timers are still unprotected. The talk demonstates
> an attack via IOEventFd, but not one with timers. In any case, it might
> make sense to revist the guard to ensure that we are covering all types
> of code that perform DMA.

Hi,

Thank you for reporting this.

Was this problem reported at qemu-security or is the presentation the first?

28.04.2023 00:10, Alexander Bulekov wrote:
..
> These patches aim to solve two types of DMA-reentrancy issues:
> 
> 1.) mmio -> dma -> mmio case
> To solve this, we track whether the device is engaged in io by
> checking/setting a reentrancy-guard within APIs used for MMIO access.
> 
> 2.) bh -> dma write -> mmio case
> This case is trickier, since we dont have a generic way to associate a
> bh with the underlying Device/DeviceState. Thus, this version allows a
> device to associate a reentrancy-guard with a bh, when creating it.
> (Instead of calling qemu_bh_new, you call qemu_bh_new_guarded)

Should this patchset (with any follow-up fixes) go to 8.0-stable?

Thanks,

/mjt