Message-ID: <CAFRSFxLF-i9Yvcf653-5gThV6PS_USqM3C5C8AaWrXuFaj8EZg@mail.gmail.com>
Date: Wed, 19 Feb 2025 13:53:48 -0800
From: Joshua Peraza <jperaza@...gle.com>
To: Greg KH <gregkh@...uxfoundation.org>
Cc: baolu.lu@...ux.intel.com, bhelgaas@...gle.com, dtor@...gle.com, 
	dwmw2@...radead.org, helgaas@...nel.org, iommu@...ts.linux-foundation.org, 
	jean-philippe@...aro.org, joro@...tes.org, jsbarnes@...gle.com, 
	lenb@...nel.org, linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-pci@...r.kernel.org, mika.westerberg@...ux.intel.com, oohall@...il.com, 
	pavel@...x.de, rafael.j.wysocki@...el.com, rafael@...nel.org, 
	rajatja@...gle.com, rajatxjain@...il.com, will@...nel.org
Subject: Re: [v8 PATCH 0/2] PCI/ACPI: Support Microsoft's "DmaProperty"

On Mon, Nov 18, 2024 at 11:43 AM Greg KH <gregkh@...uxfoundation.org> wrote:
>
> On Mon, Nov 18, 2024 at 07:30:22PM +0000, Joshua Peraza wrote:
> > This patchset rebases two previously posted patches supporting
> > recognition of Microsoft's DmaProperty.
> >
> > v8: Joshua renames untrusted_dma to requires_dma_protection and updates
> > some comments, reducing use of the word "trust" to refer to PCI devices
> > and matching the word choice in Microsoft's documentation.
>
> So this is the "clarity"?  I'm not sold, sorry.  Again, did you look at
> the previous discussions we had about this name?  We don't have to use
> Microsoft's term here as it is used differently by Linux today, right?
> If you really want to support the DmaProperty, why not just support that
> with a new bit as that's something different here, right?
>
> Again, look at what this is supposed to be conveying.  They ability to
> DMA to anywhere isn't really the root issue here, or is it?  What is the
> threat model you are trying to mitigate?
>
> > v7: Rajat updates a comment with Robin's suggestion. Joshua re-sends and
> > Greg requests clarity and documentation on why untrusted_dma is the
> > right name.
> >
> > v6: Rajat renames pci_dev_has_dma_property and links to Microsoft's
> > documentation in the commit message. Robin suggests clarifying a
> > comment.
> >
> > v5: Rajat changes the name to untrusted_dma. Bjorn suggesting changing
> > another function's name pci_acpi_check_for_dma_protection to
> > pci_dev_has_dma_property and seeks clarified documentation.
> >
> > v4: Rajat changes the name to poses_dma_risk. Christoph suggests this
> > name doesn't capture the intent as well as untrusted_dma and Rafael
> > agrees.
> >
> > v1,v2,v3: Greg suggests that (un)trusted is the wrong word for referring
> > to PCI devices, recommending a name something like "platform wants to
> > protect dma access for this device."
>
> Or is it?  I said this when?  Just how old is this patch series?
>
> confused,
>
> greg k-h

(sorry if you're getting this again; re-sending as plain text)

Sorry for the confusion! What do you think about the following for a
new cover letter?

Threat model: An overview of the security implications of strict vs
non-strict IOMMU is presented at [1]. This change is motivated by
“Case 1” where a DMA-capable device is processing untrusted inputs,
e.g. network devices.

This patchset proposes using “DMA protection” to mitigate this risk.
This has the following effects, currently controlled by the
“pci_dev::untrusted” flag.

- Separate IOMMU DMA domains
- Use of SWIOTLB if CONFIG_SWIOTLB
- Disables “quirks” in intel IOMMU
- Disables Address Translation Services

The “untrusted” flag was introduced in 2018 in [2]. The motivation for
that change was to enable using IOMMU to protect against DMA attacks
from externally facing devices such as thunderbolt ports. The patchset
introduces the “untrusted” flag which “is supposed to cover various
PCIe devices that may be used to conduct DMA attacks.” The patchset
originally proposes naming the flag “is_external” but is renamed to
“is_untrusted” and then “untrusted” supposing that it could apply to
more than just externally facing thunderbolt devices. The fact that
“ExternalFacingPort” is not part of any standard is called out during
review but also that Windows expecting firmware to identify external
facing ports makes it “as good as a formal standard in the Windows
world.”

This current patch series was first proposed in January 2022 [3]. It
originally proposed a new property “UntrustedDevice” which would cause
the untrusted flag to be set. In V1 Greg questions whether the new
property is part of the ACPI standard, asks who is making this policy
decision, and states “This notion of ‘trust’ for PCI devices is
crazy….” Mika links to Microsoft's documentation of “DmaProperty” and
suggests that property is adopted instead. Greg objects that Linux
does not have “dma protection” but Mika says that this is the IOMMU.
The term “DMA protection” is also used in thunderbolt driver code for
the same purpose and in an Intel white paper [4] describing the
technique. Mika also observes that Linux has recognized several
properties documented by Microsoft but not part of the ACPI standard.
There is discussion between Mika, Rafael, and Rajat about seeking to
align with Microsoft on the semantics of “DmaProperty” for
compatibility with firmware produced for Windows.

V2 of this patch series [5] again proposed an “UntrustedDevice”
property which Greg objects to because it is not sufficiently
descriptive, not sufficiently documented, and policies about trust
don’t belong in the kernel. Rajat describes the “untrusted” flag’s
current use, controlling IOMMU and Greg suggests naming the flag
“use_iommu” or “able to do DMA”

V3 of this patch series [6] proposes recognizing “DmaProperty” with
slightly altered semantics from Microsoft’s documentation. Greg
suggests adhering to Microsoft’s semantics for “DmaProperty” and to
introduce a new property with new semantics instead. Greg again states
that the flag being named “untrusted” (not changed in this patch) is
confusing.

V4 renames “untrusted” to “poses_dma_risk”. Christoph suggests
“untrusted_dma” and Rafael agrees.

V5 renames the flag to “untrusted_dma”. Bjorn asks for clarification
about whether the semantics of this flag will match Microsoft’s
documentation. Rajat responds that Microsoft has agreed to update
their documentation to have aligned semantics, in particular “the
property is not restricted to identify ‘internal PCIe hierarchies’
(starting at root port), but to "any PCI device". As of today,
Microsoft’s documentation does not appear to have been updated.

In V6 Rajat updates a link to Microsoft’s documentation, renames a
function to pci_dev_has_dma_property() and uses
acpi_dev_get_property() to read “DmaProperty”.

In V7-V8 Joshua re-sends and Greg requests a summary of the history of
debate about the name for the “untrusted” flag as well as what is the
threat model, what does this property convey, and why we should use
Microsoft’s DmaProperty and its semantics instead of inventing
something new.

Links:
[1] https://lore.kernel.org/linux-arm-msm/20210624101557.v2.3.Icde6be7601a5939960caf802056c88cd5132eb4e@changeid/
[2] https://lore.kernel.org/lkml/20181129155153.35840-1-mika.westerberg@linux.intel.com/
[3] https://lore.kernel.org/all/20220120000409.2706549-1-rajatja@google.com/
[4] https://www.intel.com/content/dam/develop/external/us/en/documents/intel-whitepaper-using-iommu-for-dma-protection-in-uefi-820238.pdf
[5] https://lore.kernel.org/all/20220202020103.2149130-1-rajatja@google.com/
[6] https://lore.kernel.org/all/20220216220541.1635665-1-rajatja@google.com/

Powered by blists - more mailing lists