Re: [RFC PATCH v3 0/3] physmem: Have flaview API check bus permission from MemTxAttrs argument

[Stefan Hajnoczi]

On Mon, Jan 24, 2022 at 11:50:10AM -0500, Alexander Bulekov wrote:
> On 220124 1630, Stefan Hajnoczi wrote:
> > On Wed, Dec 15, 2021 at 07:24:18PM +0100, Philippe Mathieu-Daudé wrote:
> > > This series aim to kill a recent class of bug, the infamous
> > > "DMA reentrancy" issues found by Alexander while fuzzing.
> > 
> > I took a look at how to protect DMA transactions in VIRTIO devices. It
> > will require setting the MemTxAttrs for address_space_ld/st_le/be_cached
> > calls. Errors on write (store) can be ignored. Errors on read (load) are
> > a bit more questionable since the device performs some operation based
> > on the loaded value, but at this point the driver has already caused the
> > device to do something no correct driver does (as of today, it could
> > change in the future...) so undefined device behavior might be okay.
> > 
> > It would be easier to be confident if there was a single place to
> > disable DMA re-entrancy for a device. The currently proposed API
> > requires per-device code audits and fixes. It leaves decisions to the
> > developer of each device. This will be a lot of work to fix and we
> > cannot be confident that everything has been covered since this is an
> > opt-in mechanism.
> > 
> > For these reasons it seems likely that DMA re-entrancy issues will
> > continue to creep in. I think the only way to rule out this class of
> > bugs is to implement a centralized change that doesn't involve fixing
> > every DMA access in QEMU.
> > 
> > Thoughts?
> 
> Hi Stefan,
> Do you have some ideas about how to do this centrally?
> There were at least two attempts to do this in a centralized way, but it
> seems there is some worry that edge cases will break. However, I'm
> not sure there were any concrete examples of such breakages.
> 
> [1] 
> https://lore.kernel.org/all/20210824120153.altqys6jjiuxh35p@sirius.home.kraxel.org/
> [2] https://lore.kernel.org/all/20211217030858.834822-1-alxndr@bu.edu/
> (AFAIK Neither handles the BH->DMA->MMIO case, at the moment)

Regressions are the problem with defaulting to RAM-only DMA. There's no
way to avoid the risk if we change the default. On the other hand, it's
the only way to squash this class of bugs - most existing devices just
aren't written to cope with DMA re-entrancy.

The approach in your patch sounds good to me, but I haven't followed the
discussions so maybe there were valid reasons to look for alternatives.

Stefan

signature.asc
Description: PGP signature