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

From: Stefan Hajnoczi <stefanha@redhat.com>
To: Alexander Bulekov <alxndr@bu.edu>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	"David Hildenbrand" <david@redhat.com>,
	"Jason Wang" <jasowang@redhat.com>, "Li Qiang" <liq3ea@gmail.com>,
	qemu-devel@nongnu.org, "Peter Xu" <peterx@redhat.com>,
	"Qiuhao Li" <Qiuhao.Li@outlook.com>,
	qemu-arm@nongnu.org, "Gerd Hoffmann" <kraxel@redhat.com>,
	"Edgar E . Iglesias" <edgar.iglesias@gmail.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: Re: [RFC PATCH v3 0/3] physmem: Have flaview API check bus permission from MemTxAttrs argument
Date: Tue, 25 Jan 2022 11:51:34 +0000	[thread overview]
Message-ID: <Ye/kRldjGk4gWX9g@stefanha-x1.localdomain> (raw)
In-Reply-To: <20220124165010.jn3lrwk64i56u7cd@mozz.bu.edu>

[-- Attachment #1: Type: text/plain, Size: 2475 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]