iommu.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed
From: Robin Murphy <robin.murphy@arm.com>
To: Rob Herring <robh@kernel.org>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Grant Likely <grant.likely@arm.com>,
	Paul Mackerras <paulus@samba.org>,
	Frank Rowand <frowand.list@gmail.com>,
	Ingo Molnar <mingo@kernel.org>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Saravana Kannan <saravanak@google.com>,
	Heinrich Schuchardt <xypron.glpk@gmx.de>,
	"Wysocki, Rafael J" <rafael.j.wysocki@intel.com>,
	Christoph Hellwig <hch@lst.de>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	xen-devel@lists.xenproject.org,
	Thierry Reding <treding@nvidia.com>,
	devicetree@vger.kernel.org, Will Deacon <will@kernel.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Nicolas Boichat <drinkcat@chromium.org>,
	Claire Chang <tientzu@chromium.org>,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Juergen Gross <jgross@suse.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Randy Dunlap <rdunlap@infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Linux IOMMU <iommu@lists.linux-foundation.org>,
	Michael Ellerman <mpe@ellerman.id.au>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [RFC PATCH v3 5/6] dt-bindings: of: Add restricted DMA pool
Date: Thu, 21 Jan 2021 17:29:13 +0000	[thread overview]
Message-ID: <1a570c5c-e0da-7d86-4384-4a4c50193c94@arm.com> (raw)
In-Reply-To: <CAL_JsqLv-FaiY_k+wS=iXG5AtccsXSBtvTfEGHvsN-VNqXdwpA@mail.gmail.com>

On 2021-01-21 15:48, Rob Herring wrote:
> On Wed, Jan 20, 2021 at 7:10 PM Robin Murphy <robin.murphy@arm.com>
> wrote:
>> 
>> On 2021-01-20 21:31, Rob Herring wrote:
>>> On Wed, Jan 20, 2021 at 11:30 AM Robin Murphy
>>> <robin.murphy@arm.com> wrote:
>>>> 
>>>> On 2021-01-20 16:53, Rob Herring wrote:
>>>>> On Wed, Jan 06, 2021 at 11:41:23AM +0800, Claire Chang
>>>>> wrote:
>>>>>> Introduce the new compatible string, restricted-dma-pool,
>>>>>> for restricted DMA. One can specify the address and length
>>>>>> of the restricted DMA memory region by restricted-dma-pool
>>>>>> in the device tree.
>>>>> 
>>>>> If this goes into DT, I think we should be able to use
>>>>> dma-ranges for this purpose instead. Normally, 'dma-ranges'
>>>>> is for physical bus restrictions, but there's no reason it
>>>>> can't be used for policy or to express restrictions the
>>>>> firmware has enabled.
>>>> 
>>>> There would still need to be some way to tell SWIOTLB to pick
>>>> up the corresponding chunk of memory and to prevent the kernel
>>>> from using it for anything else, though.
>>> 
>>> Don't we already have that problem if dma-ranges had a very
>>> small range? We just get lucky because the restriction is
>>> generally much more RAM than needed.
>> 
>> Not really - if a device has a naturally tiny addressing capability
>> that doesn't even cover ZONE_DMA32 where the regular SWIOTLB buffer
>> will be allocated then it's unlikely to work well, but that's just
>> crap system design. Yes, memory pressure in ZONE_DMA{32} is
>> particularly problematic for such limited devices, but it's
>> irrelevant to the issue at hand here.
> 
> Yesterday's crap system design is today's security feature. Couldn't 
> this feature make crap system design work better?

Indeed! Say you bring out your shiny new "Strawberry Flan 4" machine
with all the latest connectivity, but tragically its PCIe can only
address 25% of the RAM. So you decide to support deploying it in two
configurations: one where it runs normally for best performance, and
another "secure" one where it dedicates that quarter of RAM as a 
restricted DMA pool for any PCIe devices - that way, even if that hotel 
projector you plug in turns out to be a rogue Thunderbolt endpoint, it 
can never snarf your private keys off your eMMC out of the page cache.

(Yes, is is the thinnest of strawmen, but it sets the scene for the 
point you raised...)

...which is that in both cases the dma-ranges will still be identical. 
So how is the kernel going to know whether to steal that whole area from 
memblock before anything else can allocate from it, or not?

I don't disagree that even in Claire's original intended case it would 
be semantically correct to describe the hardware-firewalled region with 
dma-ranges. It just turns out not to be necessary, and you're already 
arguing for not adding anything in DT that doesn't need to be.

>> What we have here is a device that's not allowed to see *kernel*
>> memory at all. It's been artificially constrained to a particular
>> region by a TZASC or similar, and the only data which should ever
>> be placed in that
> 
> May have been constrained, but that's entirely optional.
> 
> In the optional case where the setup is entirely up to the OS, I
> don't think this belongs in the DT at all. Perhaps that should be
> solved first.

Yes! Let's definitely consider that case! Say you don't have any 
security or physical limitations but want to use a bounce pool for some 
device anyway because reasons (perhaps copying streaming DMA data to a 
better guaranteed alignment gives an overall performance win). Now the 
*only* relevant thing to communicate to the kernel is to, ahem, reserve 
a large chunk of memory, and use it for this special purpose. Isn't that 
literally what reserved-memory bindings are for?

>> region is data intended for that device to see. That way if it
>> tries to go rogue it physically can't start slurping data intended
>> for other devices or not mapped for DMA at all. The bouncing is an
>> important part of this - I forget the title off-hand but there was
>> an interesting paper a few years ago which demonstrated that even
>> with an IOMMU, streaming DMA of in-place buffers could reveal
>> enough adjacent data from the same page to mount an attack on the
>> system. Memory pressure should be immaterial since the size of each
>> bounce pool carveout will presumably be tuned for the needs of the
>> given device.
>> 
>>> In any case, wouldn't finding all the dma-ranges do this? We're 
>>> already walking the tree to find the max DMA address now.
>> 
>> If all you can see are two "dma-ranges" properties, how do you
>> propose to tell that one means "this is the extent of what I can
>> address, please set my masks and dma-range-map accordingly and try
>> to allocate things where I can reach them" while the other means
>> "take this output range away from the page allocator and hook it up
>> as my dedicated bounce pool, because it is Serious Security Time"?
>> Especially since getting that choice wrong either way would be a
>> Bad Thing.
> 
> Either we have some heuristic based on the size or we add some hint. 
> The point is let's build on what we already have for defining DMA 
> accessible memory in DT rather than some parallel mechanism.

The point I'm trying to bang home is that it's really not about the DMA 
accessibility, it's about the purpose of the memory itself. Even when 
DMA accessibility *is* relevant it's already implied by that purpose, 
from the point of view of the implementation. The only difference it 
might make is to the end user if they want to ascertain whether the 
presence of such a pool represents protection against an untrusted 
device or just some DMA optimisation tweak.

Robin.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

  reply	other threads:[~2021-01-21 17:29 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-06  3:41 [RFC PATCH v3 0/6] Restricted DMA Claire Chang
2021-01-06  3:41 ` [RFC PATCH v3 1/6] swiotlb: Add io_tlb_mem struct Claire Chang
2021-01-13 11:50   ` Christoph Hellwig
2021-01-06  3:41 ` [RFC PATCH v3 2/6] swiotlb: Add restricted DMA pool Claire Chang
2021-01-06  7:50   ` Greg KH
2021-01-13 11:51     ` Christoph Hellwig
2021-01-13 12:29       ` Greg KH
2021-01-13 12:37         ` Christoph Hellwig
2021-01-06 18:52   ` Konrad Rzeszutek Wilk
2021-01-07 17:39     ` Claire Chang
2021-01-07 17:57       ` Konrad Rzeszutek Wilk
2021-01-07 18:09         ` Florian Fainelli
2021-01-07 21:19           ` Konrad Rzeszutek Wilk
2021-01-12 23:52             ` Florian Fainelli
2021-01-25  5:26           ` Jon Masters
2021-01-13  1:53         ` Robin Murphy
2021-01-13  0:03   ` Florian Fainelli
2021-01-13 13:59     ` Nicolas Saenz Julienne
2021-01-13 15:27       ` Robin Murphy
2021-01-13 17:43         ` Florian Fainelli
2021-01-13 18:03           ` Robin Murphy
2021-01-13 12:42   ` Christoph Hellwig
2021-01-14  9:06     ` Claire Chang
2021-01-06  3:41 ` [RFC PATCH v3 3/6] swiotlb: Use restricted DMA pool if available Claire Chang
2021-01-12 23:39   ` Florian Fainelli
2021-01-13 12:44   ` Christoph Hellwig
2021-01-06  3:41 ` [RFC PATCH v3 4/6] swiotlb: Add restricted DMA alloc/free support Claire Chang
2021-01-12 23:41   ` Florian Fainelli
2021-01-13 12:48   ` Christoph Hellwig
2021-01-13 18:27     ` Robin Murphy
2021-01-13 18:32       ` Christoph Hellwig
2021-01-06  3:41 ` [RFC PATCH v3 5/6] dt-bindings: of: Add restricted DMA pool Claire Chang
2021-01-06 18:57   ` Konrad Rzeszutek Wilk
2021-01-07 17:39     ` Claire Chang
2021-01-07 18:00       ` Konrad Rzeszutek Wilk
2021-01-07 18:14         ` Florian Fainelli
2021-01-12  7:47           ` Claire Chang
2021-01-20 16:53   ` Rob Herring
2021-01-20 17:30     ` Robin Murphy
2021-01-20 21:31       ` Rob Herring
2021-01-21  1:09         ` Robin Murphy
2021-01-21 15:48           ` Rob Herring
2021-01-21 17:29             ` Robin Murphy [this message]
2021-01-06  3:41 ` [RFC PATCH v3 6/6] of: Add plumbing for " Claire Chang
2021-01-12 23:48   ` Florian Fainelli
2021-01-14  9:08     ` Claire Chang
2021-01-14 18:52       ` Florian Fainelli
2021-01-15  3:46         ` Claire Chang
2021-01-06 18:48 ` [RFC PATCH v3 0/6] Restricted DMA Florian Fainelli
2021-01-07 17:38   ` Claire Chang
2021-01-07 17:42   ` Claire Chang
2021-01-07 17:59     ` Florian Fainelli
2021-01-12  7:48       ` Claire Chang
2021-01-12 18:01         ` Florian Fainelli
2021-01-13  2:29           ` Tomasz Figa
2021-01-13  3:56             ` Florian Fainelli
2021-01-13  4:25               ` Tomasz Figa
2021-01-13  4:41                 ` Florian Fainelli
2021-02-09  6:27                   ` Claire Chang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1a570c5c-e0da-7d86-4384-4a4c50193c94@arm.com \
    --to=robin.murphy@arm.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=benh@kernel.crashing.org \
    --cc=bgolaszewski@baylibre.com \
    --cc=boris.ostrovsky@oracle.com \
    --cc=dan.j.williams@intel.com \
    --cc=devicetree@vger.kernel.org \
    --cc=drinkcat@chromium.org \
    --cc=frowand.list@gmail.com \
    --cc=grant.likely@arm.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hch@lst.de \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=iommu@lists.linux-foundation.org \
    --cc=jgross@suse.com \
    --cc=konrad.wilk@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mingo@kernel.org \
    --cc=mpe@ellerman.id.au \
    --cc=paulus@samba.org \
    --cc=peterz@infradead.org \
    --cc=rafael.j.wysocki@intel.com \
    --cc=rdunlap@infradead.org \
    --cc=robh@kernel.org \
    --cc=saravanak@google.com \
    --cc=sstabellini@kernel.org \
    --cc=tientzu@chromium.org \
    --cc=treding@nvidia.com \
    --cc=will@kernel.org \
    --cc=xen-devel@lists.xenproject.org \
    --cc=xypron.glpk@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).