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

[Alexander Bulekov]

On 220530 1539, Philippe Mathieu-Daudé wrote:
> On 30/5/22 15:28, Peter Maydell wrote:
> > On Mon, 30 May 2022 at 14:10, Alexander Bulekov <alxndr@bu.edu> wrote:
> > > 
> > > 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?
> > 
> > That's a different question, ie "are there legitimate cases where
> > devices try to DMA to themselves?". I don't know the answer, but
> > I suspect not.
> 
> There is a niche where it might not be legitimate, but it is (ab)used
> and Paolo wants to keep such cases working. I already responded to
> Alexander here:
> https://lore.kernel.org/qemu-devel/380ea0e5-a006-c570-4ec8-d67e837547ee@redhat.com/

I'm not sure we confirmed that this is actually an example of a device
performing DMA to its own MMIO. Unless I am missing something, the BLOAD
example simply performs repeated writes to VRAM?

That said video-related devices seem like possible candidates where such
behavior is conceivable. But even in those cases, the memory regions
would likely be ram/rom devices (which are excluded from the re-entrancy
check).