Re: [PATCH v2 1/3] memory: Track whether a Device is engaged in IO

[Alexander Bulekov]

On 220530 1219, Peter Maydell wrote:
> On Fri, 27 May 2022 at 17:19, Alexander Bulekov <alxndr@bu.edu> wrote:
> >
> > Add a flag to the DeviceState, when a device is engaged in PIO/MMIO/DMA.
> > This flag should be 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 DMA reentrancy issues. E.g.:
> > sdhci pio -> dma write -> sdhci mmio
> > nvme bh -> dma write -> nvme mmio
> >
> > These issues have led to problems such as stack-exhaustion and
> > use-after-frees.
> >
> > Assumptions:
> >  * Devices do not interact with their own PIO/MMIO memory-regions using
> >    DMA.
> 
> If you're trying to protect against malicious guest-controlled
> DMA operations, you can't assume that. The guest can program
> a DMA controller to DMA to its own MMIO register bank if it likes.

If this is the case, then it seems the only way to fix this class of
problems is to rewrite device code so that it is safe for re-entrancy.
That seems to require significant upfront work to support behavior that
is often not even specified.
Simply spot-fixing the fuzzer re-entracy bugs seems like a dangerous,
incomplete solution.

Can we disable re-entracy by default, to fix the security issues, and
allow device code to "opt-in" to re-entrancy?

-Alex

> 
> >  * There is now way for there to be multiple simultaneous accesses to a
> >    device's PIO/MMIO memory-regions, or for multiple threads to perform
> >    DMA accesses simultaneously on behalf of a single device.
> 
> This one is generally true because device code runs with
> the iothread lock held.
> 
> -- PMM