On Mon, Nov 18, 2024 at 11:43 AM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx/ [3] https://lore.kernel.org/all/20220120000409.2706549-1-rajatja@xxxxxxxxxx/ [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@xxxxxxxxxx/ [6] https://lore.kernel.org/all/20220216220541.1635665-1-rajatja@xxxxxxxxxx/
