| Message ID | 20220216220541.1635665-1-rajatja@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3] PCI: ACPI: Support Microsoft's "DmaProperty" | expand |
On Wed, Feb 16, 2022 at 02:05:41PM -0800, Rajat Jain wrote: > The "DmaProperty" is supported and documented by Microsoft here: > https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports > They use this property for DMA protection: > https://docs.microsoft.com/en-us/windows/security/information-protection/kernel-dma-protection-for-thunderbolt > > Support the "DmaProperty" with the same semantics. Windows documents the > property to apply to PCIe root ports only. Extend it to apply to any > PCI device. This is useful for internal PCI devices that do not hang off > a PCIe rootport, but offer an attack surface for DMA attacks (e.g. > internal network devices). > > Signed-off-by: Rajat Jain <rajatja@google.com> > --- > v3: * Use Microsoft's documented property "DmaProperty" > * Resctrict to ACPI only > > drivers/pci/pci-acpi.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c > index a42dbf448860..660baa60c040 100644 > --- a/drivers/pci/pci-acpi.c > +++ b/drivers/pci/pci-acpi.c > @@ -1350,12 +1350,30 @@ static void pci_acpi_set_external_facing(struct pci_dev *dev) > dev->external_facing = 1; > } > > +static void pci_acpi_check_for_dma_protection(struct pci_dev *dev) > +{ > + u8 val; > + > + /* > + * Microsoft Windows uses this property, and is documented here: > + * https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports > + * While Microsoft documents this property as only applicable to PCIe > + * root ports, we expand it to be applicable to any PCI device. > + */ > + if (device_property_read_u8(&dev->dev, "DmaProperty", &val)) > + return; Why not continue to only do this for PCIe devices like it is actually being used for? Why expand it? And what driver/device is going to use this? thanks, greg k-h
Hello, On Wed, Feb 16, 2022 at 10:16 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Wed, Feb 16, 2022 at 02:05:41PM -0800, Rajat Jain wrote: > > The "DmaProperty" is supported and documented by Microsoft here: > > https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports > > They use this property for DMA protection: > > https://docs.microsoft.com/en-us/windows/security/information-protection/kernel-dma-protection-for-thunderbolt > > > > Support the "DmaProperty" with the same semantics. Windows documents the > > property to apply to PCIe root ports only. Extend it to apply to any > > PCI device. This is useful for internal PCI devices that do not hang off > > a PCIe rootport, but offer an attack surface for DMA attacks (e.g. > > internal network devices). > > > > Signed-off-by: Rajat Jain <rajatja@google.com> > > --- > > v3: * Use Microsoft's documented property "DmaProperty" > > * Resctrict to ACPI only > > > > drivers/pci/pci-acpi.c | 18 ++++++++++++++++++ > > 1 file changed, 18 insertions(+) > > > > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c > > index a42dbf448860..660baa60c040 100644 > > --- a/drivers/pci/pci-acpi.c > > +++ b/drivers/pci/pci-acpi.c > > @@ -1350,12 +1350,30 @@ static void pci_acpi_set_external_facing(struct pci_dev *dev) > > dev->external_facing = 1; > > } > > > > +static void pci_acpi_check_for_dma_protection(struct pci_dev *dev) > > +{ > > + u8 val; > > + > > + /* > > + * Microsoft Windows uses this property, and is documented here: > > + * https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports > > + * While Microsoft documents this property as only applicable to PCIe > > + * root ports, we expand it to be applicable to any PCI device. > > + */ > > + if (device_property_read_u8(&dev->dev, "DmaProperty", &val)) > > + return; > > Why not continue to only do this for PCIe devices like it is actually > being used for? Why expand it? Because devices hanging off of PCIe root ports are not the only ones that may need DMA protection. There may be internal PCI devices (that don't hang off a PCIe root port) that may need DMA protection. Examples include internal network controllers that may offer an attack surface by handling network data or running vendor firmware. > > And what driver/device is going to use this? This is already used by PCI subsystem to enforce stricter ACS settings, and IOMMU drivers to enforce stricter IOMMU settings. Thanks & Best Regards, Rajat > > thanks, > > greg k-h
On Thu, Feb 17, 2022 at 10:26:39AM -0800, Rajat Jain wrote: > Hello, > > On Wed, Feb 16, 2022 at 10:16 PM Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > > > On Wed, Feb 16, 2022 at 02:05:41PM -0800, Rajat Jain wrote: > > > The "DmaProperty" is supported and documented by Microsoft here: > > > https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports > > > They use this property for DMA protection: > > > https://docs.microsoft.com/en-us/windows/security/information-protection/kernel-dma-protection-for-thunderbolt > > > > > > Support the "DmaProperty" with the same semantics. Windows documents the > > > property to apply to PCIe root ports only. Extend it to apply to any > > > PCI device. This is useful for internal PCI devices that do not hang off > > > a PCIe rootport, but offer an attack surface for DMA attacks (e.g. > > > internal network devices). > > > > > > Signed-off-by: Rajat Jain <rajatja@google.com> > > > --- > > > v3: * Use Microsoft's documented property "DmaProperty" > > > * Resctrict to ACPI only > > > > > > drivers/pci/pci-acpi.c | 18 ++++++++++++++++++ > > > 1 file changed, 18 insertions(+) > > > > > > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c > > > index a42dbf448860..660baa60c040 100644 > > > --- a/drivers/pci/pci-acpi.c > > > +++ b/drivers/pci/pci-acpi.c > > > @@ -1350,12 +1350,30 @@ static void pci_acpi_set_external_facing(struct pci_dev *dev) > > > dev->external_facing = 1; > > > } > > > > > > +static void pci_acpi_check_for_dma_protection(struct pci_dev *dev) > > > +{ > > > + u8 val; > > > + > > > + /* > > > + * Microsoft Windows uses this property, and is documented here: > > > + * https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports > > > + * While Microsoft documents this property as only applicable to PCIe > > > + * root ports, we expand it to be applicable to any PCI device. Web pages have a tendancy to die over time (will it be here in 20+ years?) Please describe how Windows uses this attribute and what it uses it for in the comment. > > > + */ > > > + if (device_property_read_u8(&dev->dev, "DmaProperty", &val)) > > > + return; > > > > Why not continue to only do this for PCIe devices like it is actually > > being used for? Why expand it? > > Because devices hanging off of PCIe root ports are not the only ones > that may need DMA protection. There may be internal PCI devices (that > don't hang off a PCIe root port) that may need DMA protection. > Examples include internal network controllers that may offer an attack > surface by handling network data or running vendor firmware. And why does Microsoft not do the same for them? What attribute do they use for that? And again, this is for "dma protection" not "trusted / untrusted". That name here is getting very confusing and as I have stated in the past, is probably incorrect and needs to be changed. Also userspace policy decisions need to be made here which would define the trust/untrusted value. So how about just passing this on as what Windows does, and have a new attribute for the device called "platform wants to protect dma accesses for this device" or something like that? naming is hard, greg k-h
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c index a42dbf448860..660baa60c040 100644 --- a/drivers/pci/pci-acpi.c +++ b/drivers/pci/pci-acpi.c @@ -1350,12 +1350,30 @@ static void pci_acpi_set_external_facing(struct pci_dev *dev) dev->external_facing = 1; } +static void pci_acpi_check_for_dma_protection(struct pci_dev *dev) +{ + u8 val; + + /* + * Microsoft Windows uses this property, and is documented here: + * https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports + * While Microsoft documents this property as only applicable to PCIe + * root ports, we expand it to be applicable to any PCI device. + */ + if (device_property_read_u8(&dev->dev, "DmaProperty", &val)) + return; + + if (val) + dev->untrusted = 1; +} + void pci_acpi_setup(struct device *dev, struct acpi_device *adev) { struct pci_dev *pci_dev = to_pci_dev(dev); pci_acpi_optimize_delay(pci_dev, adev->handle); pci_acpi_set_external_facing(pci_dev); + pci_acpi_check_for_dma_protection(pci_dev); pci_acpi_add_edr_notifier(pci_dev); pci_acpi_add_pm_notifier(adev, pci_dev);
The "DmaProperty" is supported and documented by Microsoft here: https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports They use this property for DMA protection: https://docs.microsoft.com/en-us/windows/security/information-protection/kernel-dma-protection-for-thunderbolt Support the "DmaProperty" with the same semantics. Windows documents the property to apply to PCIe root ports only. Extend it to apply to any PCI device. This is useful for internal PCI devices that do not hang off a PCIe rootport, but offer an attack surface for DMA attacks (e.g. internal network devices). Signed-off-by: Rajat Jain <rajatja@google.com> --- v3: * Use Microsoft's documented property "DmaProperty" * Resctrict to ACPI only drivers/pci/pci-acpi.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)