| Message ID | 20181129155153.35840-2-mika.westerberg@linux.intel.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | PCI / iommu / thunderbolt: IOMMU based DMA protection | expand |
On Thu, Nov 29, 2018 at 4:52 PM Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > > A malicious PCI device may use DMA to attack the system. An external > Thunderbolt port is a convenient point to attach such a device. The OS > may use IOMMU to defend against DMA attacks. > > Recent BIOSes with Thunderbolt ports mark these externally facing root > ports with this ACPI _DSD [1]: > > Name (_DSD, Package () { > ToUUID ("efcc06cc-73ac-4bc3-bff0-76143807c389"), > Package () { > Package () {"ExternalFacingPort", 1}, > Package () {"UID", 0 } > } > }) > > If we find such a root port, mark it and all its children as untrusted. > The rest of the OS may use this information to enable DMA protection > against malicious devices. For instance the device may be put behind an > IOMMU to keep it from accessing memory outside of what the driver has > allocated for it. > > While at it, add a comment on top of prp_guids array explaining the > possible caveat resulting when these GUIDs are treated equivalent. > > [1] https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports#identifying-externally-exposed-pcie-root-ports > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/acpi/property.c | 11 +++++++++++ > drivers/pci/pci-acpi.c | 19 +++++++++++++++++++ > drivers/pci/probe.c | 15 +++++++++++++++ > include/linux/pci.h | 8 ++++++++ > 4 files changed, 53 insertions(+) > > diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c > index 8c7c4583b52d..77abe0ec4043 100644 > --- a/drivers/acpi/property.c > +++ b/drivers/acpi/property.c > @@ -24,6 +24,14 @@ static int acpi_data_get_property_array(const struct acpi_device_data *data, > acpi_object_type type, > const union acpi_object **obj); > > +/* > + * The GUIDs here are made equivalent to each other in order to avoid extra > + * complexity in the properties handling code, with the caveat that the > + * kernel will accept certain combinations of GUID and properties that are > + * not defined without a warning. For instance if any of the properties > + * from different GUID appear in a property list of another, it will be > + * accepted by the kernel. Firmware validation tools should catch these. > + */ > static const guid_t prp_guids[] = { > /* ACPI _DSD device properties GUID: daffd814-6eba-4d8c-8a91-bc9bbf4aa301 */ > GUID_INIT(0xdaffd814, 0x6eba, 0x4d8c, > @@ -31,6 +39,9 @@ static const guid_t prp_guids[] = { > /* Hotplug in D3 GUID: 6211e2c0-58a3-4af3-90e1-927a4e0c55a4 */ > GUID_INIT(0x6211e2c0, 0x58a3, 0x4af3, > 0x90, 0xe1, 0x92, 0x7a, 0x4e, 0x0c, 0x55, 0xa4), > + /* External facing port GUID: efcc06cc-73ac-4bc3-bff0-76143807c389 */ > + GUID_INIT(0xefcc06cc, 0x73ac, 0x4bc3, > + 0xbf, 0xf0, 0x76, 0x14, 0x38, 0x07, 0xc3, 0x89), > }; > > static const guid_t ads_guid = > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c > index 921db6f80340..e1949f7efd9c 100644 > --- a/drivers/pci/pci-acpi.c > +++ b/drivers/pci/pci-acpi.c > @@ -789,6 +789,24 @@ static void pci_acpi_optimize_delay(struct pci_dev *pdev, > ACPI_FREE(obj); > } > > +static void pci_acpi_set_untrusted(struct pci_dev *dev) > +{ > + u8 val; > + > + if (pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT) > + return; > + if (device_property_read_u8(&dev->dev, "ExternalFacingPort", &val)) > + return; > + > + /* > + * These root ports expose PCIe (including DMA) outside of the > + * system so make sure we treat them and everything behind as > + * untrusted. > + */ > + if (val) > + dev->untrusted = 1; > +} > + > static void pci_acpi_setup(struct device *dev) > { > struct pci_dev *pci_dev = to_pci_dev(dev); > @@ -798,6 +816,7 @@ static void pci_acpi_setup(struct device *dev) > return; > > pci_acpi_optimize_delay(pci_dev, adev->handle); > + pci_acpi_set_untrusted(pci_dev); > > pci_acpi_add_pm_notifier(adev, pci_dev); > if (!adev->wakeup.flags.valid) > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index b1c05b5054a0..257b9f6f2ebb 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -1378,6 +1378,19 @@ static void set_pcie_thunderbolt(struct pci_dev *dev) > } > } > > +static void set_pcie_untrusted(struct pci_dev *dev) > +{ > + struct pci_dev *parent; > + > + /* > + * If the upstream bridge is untrusted we treat this device > + * untrusted as well. > + */ > + parent = pci_upstream_bridge(dev); > + if (parent && parent->untrusted) > + dev->untrusted = true; > +} > + > /** > * pci_ext_cfg_is_aliased - Is ext config space just an alias of std config? > * @dev: PCI device > @@ -1638,6 +1651,8 @@ int pci_setup_device(struct pci_dev *dev) > /* Need to have dev->cfg_size ready */ > set_pcie_thunderbolt(dev); > > + set_pcie_untrusted(dev); > + > /* "Unknown power state" */ > dev->current_state = PCI_UNKNOWN; > > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 11c71c4ecf75..c786a2f27bee 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -396,6 +396,14 @@ struct pci_dev { > unsigned int is_hotplug_bridge:1; > unsigned int shpc_managed:1; /* SHPC owned by shpchp */ > unsigned int is_thunderbolt:1; /* Thunderbolt controller */ > + /* > + * Devices marked being untrusted are the ones that can potentially > + * execute DMA attacks and similar. They are typically connected > + * through external ports such as Thunderbolt but not limited to > + * that. When an IOMMU is enabled they should be getting full > + * mappings to make sure they cannot access arbitrary memory. > + */ > + unsigned int untrusted:1; > unsigned int __aer_firmware_first_valid:1; > unsigned int __aer_firmware_first:1; > unsigned int broken_intx_masking:1; /* INTx masking can't be used */ > -- > 2.19.2 >
On Thu, Nov 29, 2018 at 06:51:50PM +0300, Mika Westerberg wrote: > A malicious PCI device may use DMA to attack the system. An external > Thunderbolt port is a convenient point to attach such a device. The OS > may use IOMMU to defend against DMA attacks. > > Recent BIOSes with Thunderbolt ports mark these externally facing root > ports with this ACPI _DSD [1]: I'm not 100% comfortable with the "Recent BIOSes" wording because that suggests that we can rely on the fact that *all* BIOSes newer than some date X mark these ports. Since this _DSD usage is Microsoft-specific and not required by either PCIe or ACPI specs, we can't rely on it. A BIOS that doesn't implement it may not be Windows-certified, but it's perfectly spec-compliant otherwise and we have to keep in mind the possibility that ports without this _DSD may still be externally visible and may still be attack vectors. > Name (_DSD, Package () { > ToUUID ("efcc06cc-73ac-4bc3-bff0-76143807c389"), > Package () { > Package () {"ExternalFacingPort", 1}, > Package () {"UID", 0 } > } > }) > > If we find such a root port, mark it and all its children as untrusted. > The rest of the OS may use this information to enable DMA protection > against malicious devices. For instance the device may be put behind an > IOMMU to keep it from accessing memory outside of what the driver has > allocated for it. > > While at it, add a comment on top of prp_guids array explaining the > possible caveat resulting when these GUIDs are treated equivalent. > > [1] https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports#identifying-externally-exposed-pcie-root-ports > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> Acked-by: Bjorn Helgaas <bhelgaas@google.com> > --- > drivers/acpi/property.c | 11 +++++++++++ > drivers/pci/pci-acpi.c | 19 +++++++++++++++++++ > drivers/pci/probe.c | 15 +++++++++++++++ > include/linux/pci.h | 8 ++++++++ > 4 files changed, 53 insertions(+) > > diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c > index 8c7c4583b52d..77abe0ec4043 100644 > --- a/drivers/acpi/property.c > +++ b/drivers/acpi/property.c > @@ -24,6 +24,14 @@ static int acpi_data_get_property_array(const struct acpi_device_data *data, > acpi_object_type type, > const union acpi_object **obj); > > +/* > + * The GUIDs here are made equivalent to each other in order to avoid extra > + * complexity in the properties handling code, with the caveat that the > + * kernel will accept certain combinations of GUID and properties that are > + * not defined without a warning. For instance if any of the properties > + * from different GUID appear in a property list of another, it will be > + * accepted by the kernel. Firmware validation tools should catch these. > + */ > static const guid_t prp_guids[] = { > /* ACPI _DSD device properties GUID: daffd814-6eba-4d8c-8a91-bc9bbf4aa301 */ > GUID_INIT(0xdaffd814, 0x6eba, 0x4d8c, > @@ -31,6 +39,9 @@ static const guid_t prp_guids[] = { > /* Hotplug in D3 GUID: 6211e2c0-58a3-4af3-90e1-927a4e0c55a4 */ > GUID_INIT(0x6211e2c0, 0x58a3, 0x4af3, > 0x90, 0xe1, 0x92, 0x7a, 0x4e, 0x0c, 0x55, 0xa4), > + /* External facing port GUID: efcc06cc-73ac-4bc3-bff0-76143807c389 */ > + GUID_INIT(0xefcc06cc, 0x73ac, 0x4bc3, > + 0xbf, 0xf0, 0x76, 0x14, 0x38, 0x07, 0xc3, 0x89), > }; > > static const guid_t ads_guid = > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c > index 921db6f80340..e1949f7efd9c 100644 > --- a/drivers/pci/pci-acpi.c > +++ b/drivers/pci/pci-acpi.c > @@ -789,6 +789,24 @@ static void pci_acpi_optimize_delay(struct pci_dev *pdev, > ACPI_FREE(obj); > } > > +static void pci_acpi_set_untrusted(struct pci_dev *dev) > +{ > + u8 val; > + > + if (pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT) > + return; > + if (device_property_read_u8(&dev->dev, "ExternalFacingPort", &val)) > + return; > + > + /* > + * These root ports expose PCIe (including DMA) outside of the > + * system so make sure we treat them and everything behind as > + * untrusted. > + */ > + if (val) > + dev->untrusted = 1; > +} > + > static void pci_acpi_setup(struct device *dev) > { > struct pci_dev *pci_dev = to_pci_dev(dev); > @@ -798,6 +816,7 @@ static void pci_acpi_setup(struct device *dev) > return; > > pci_acpi_optimize_delay(pci_dev, adev->handle); > + pci_acpi_set_untrusted(pci_dev); > > pci_acpi_add_pm_notifier(adev, pci_dev); > if (!adev->wakeup.flags.valid) > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index b1c05b5054a0..257b9f6f2ebb 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -1378,6 +1378,19 @@ static void set_pcie_thunderbolt(struct pci_dev *dev) > } > } > > +static void set_pcie_untrusted(struct pci_dev *dev) > +{ > + struct pci_dev *parent; > + > + /* > + * If the upstream bridge is untrusted we treat this device > + * untrusted as well. > + */ > + parent = pci_upstream_bridge(dev); > + if (parent && parent->untrusted) > + dev->untrusted = true; > +} > + > /** > * pci_ext_cfg_is_aliased - Is ext config space just an alias of std config? > * @dev: PCI device > @@ -1638,6 +1651,8 @@ int pci_setup_device(struct pci_dev *dev) > /* Need to have dev->cfg_size ready */ > set_pcie_thunderbolt(dev); > > + set_pcie_untrusted(dev); > + > /* "Unknown power state" */ > dev->current_state = PCI_UNKNOWN; > > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 11c71c4ecf75..c786a2f27bee 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -396,6 +396,14 @@ struct pci_dev { > unsigned int is_hotplug_bridge:1; > unsigned int shpc_managed:1; /* SHPC owned by shpchp */ > unsigned int is_thunderbolt:1; /* Thunderbolt controller */ > + /* > + * Devices marked being untrusted are the ones that can potentially > + * execute DMA attacks and similar. They are typically connected > + * through external ports such as Thunderbolt but not limited to > + * that. When an IOMMU is enabled they should be getting full > + * mappings to make sure they cannot access arbitrary memory. > + */ > + unsigned int untrusted:1; > unsigned int __aer_firmware_first_valid:1; > unsigned int __aer_firmware_first:1; > unsigned int broken_intx_masking:1; /* INTx masking can't be used */ > -- > 2.19.2 >
On Mon, Dec 03, 2018 at 06:28:00PM -0600, Bjorn Helgaas wrote: > On Thu, Nov 29, 2018 at 06:51:50PM +0300, Mika Westerberg wrote: > > A malicious PCI device may use DMA to attack the system. An external > > Thunderbolt port is a convenient point to attach such a device. The OS > > may use IOMMU to defend against DMA attacks. > > > > Recent BIOSes with Thunderbolt ports mark these externally facing root > > ports with this ACPI _DSD [1]: > > I'm not 100% comfortable with the "Recent BIOSes" wording because that > suggests that we can rely on the fact that *all* BIOSes newer than > some date X mark these ports. > > Since this _DSD usage is Microsoft-specific and not required by either > PCIe or ACPI specs, we can't rely on it. A BIOS that doesn't > implement it may not be Windows-certified, but it's perfectly > spec-compliant otherwise and we have to keep in mind the possibility > that ports without this _DSD may still be externally visible and may > still be attack vectors. OK. I will change it to "Some BIOSes .." following what you suggested earlier. That should make it clear not all BIOSes are required to implement this. > > Name (_DSD, Package () { > > ToUUID ("efcc06cc-73ac-4bc3-bff0-76143807c389"), > > Package () { > > Package () {"ExternalFacingPort", 1}, > > Package () {"UID", 0 } > > } > > }) > > > > If we find such a root port, mark it and all its children as untrusted. > > The rest of the OS may use this information to enable DMA protection > > against malicious devices. For instance the device may be put behind an > > IOMMU to keep it from accessing memory outside of what the driver has > > allocated for it. > > > > While at it, add a comment on top of prp_guids array explaining the > > possible caveat resulting when these GUIDs are treated equivalent. > > > > [1] https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports#identifying-externally-exposed-pcie-root-ports > > > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> Thanks!
diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c index 8c7c4583b52d..77abe0ec4043 100644 --- a/drivers/acpi/property.c +++ b/drivers/acpi/property.c @@ -24,6 +24,14 @@ static int acpi_data_get_property_array(const struct acpi_device_data *data, acpi_object_type type, const union acpi_object **obj); +/* + * The GUIDs here are made equivalent to each other in order to avoid extra + * complexity in the properties handling code, with the caveat that the + * kernel will accept certain combinations of GUID and properties that are + * not defined without a warning. For instance if any of the properties + * from different GUID appear in a property list of another, it will be + * accepted by the kernel. Firmware validation tools should catch these. + */ static const guid_t prp_guids[] = { /* ACPI _DSD device properties GUID: daffd814-6eba-4d8c-8a91-bc9bbf4aa301 */ GUID_INIT(0xdaffd814, 0x6eba, 0x4d8c, @@ -31,6 +39,9 @@ static const guid_t prp_guids[] = { /* Hotplug in D3 GUID: 6211e2c0-58a3-4af3-90e1-927a4e0c55a4 */ GUID_INIT(0x6211e2c0, 0x58a3, 0x4af3, 0x90, 0xe1, 0x92, 0x7a, 0x4e, 0x0c, 0x55, 0xa4), + /* External facing port GUID: efcc06cc-73ac-4bc3-bff0-76143807c389 */ + GUID_INIT(0xefcc06cc, 0x73ac, 0x4bc3, + 0xbf, 0xf0, 0x76, 0x14, 0x38, 0x07, 0xc3, 0x89), }; static const guid_t ads_guid = diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c index 921db6f80340..e1949f7efd9c 100644 --- a/drivers/pci/pci-acpi.c +++ b/drivers/pci/pci-acpi.c @@ -789,6 +789,24 @@ static void pci_acpi_optimize_delay(struct pci_dev *pdev, ACPI_FREE(obj); } +static void pci_acpi_set_untrusted(struct pci_dev *dev) +{ + u8 val; + + if (pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT) + return; + if (device_property_read_u8(&dev->dev, "ExternalFacingPort", &val)) + return; + + /* + * These root ports expose PCIe (including DMA) outside of the + * system so make sure we treat them and everything behind as + * untrusted. + */ + if (val) + dev->untrusted = 1; +} + static void pci_acpi_setup(struct device *dev) { struct pci_dev *pci_dev = to_pci_dev(dev); @@ -798,6 +816,7 @@ static void pci_acpi_setup(struct device *dev) return; pci_acpi_optimize_delay(pci_dev, adev->handle); + pci_acpi_set_untrusted(pci_dev); pci_acpi_add_pm_notifier(adev, pci_dev); if (!adev->wakeup.flags.valid) diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index b1c05b5054a0..257b9f6f2ebb 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -1378,6 +1378,19 @@ static void set_pcie_thunderbolt(struct pci_dev *dev) } } +static void set_pcie_untrusted(struct pci_dev *dev) +{ + struct pci_dev *parent; + + /* + * If the upstream bridge is untrusted we treat this device + * untrusted as well. + */ + parent = pci_upstream_bridge(dev); + if (parent && parent->untrusted) + dev->untrusted = true; +} + /** * pci_ext_cfg_is_aliased - Is ext config space just an alias of std config? * @dev: PCI device @@ -1638,6 +1651,8 @@ int pci_setup_device(struct pci_dev *dev) /* Need to have dev->cfg_size ready */ set_pcie_thunderbolt(dev); + set_pcie_untrusted(dev); + /* "Unknown power state" */ dev->current_state = PCI_UNKNOWN; diff --git a/include/linux/pci.h b/include/linux/pci.h index 11c71c4ecf75..c786a2f27bee 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -396,6 +396,14 @@ struct pci_dev { unsigned int is_hotplug_bridge:1; unsigned int shpc_managed:1; /* SHPC owned by shpchp */ unsigned int is_thunderbolt:1; /* Thunderbolt controller */ + /* + * Devices marked being untrusted are the ones that can potentially + * execute DMA attacks and similar. They are typically connected + * through external ports such as Thunderbolt but not limited to + * that. When an IOMMU is enabled they should be getting full + * mappings to make sure they cannot access arbitrary memory. + */ + unsigned int untrusted:1; unsigned int __aer_firmware_first_valid:1; unsigned int __aer_firmware_first:1; unsigned int broken_intx_masking:1; /* INTx masking can't be used */
A malicious PCI device may use DMA to attack the system. An external Thunderbolt port is a convenient point to attach such a device. The OS may use IOMMU to defend against DMA attacks. Recent BIOSes with Thunderbolt ports mark these externally facing root ports with this ACPI _DSD [1]: Name (_DSD, Package () { ToUUID ("efcc06cc-73ac-4bc3-bff0-76143807c389"), Package () { Package () {"ExternalFacingPort", 1}, Package () {"UID", 0 } } }) If we find such a root port, mark it and all its children as untrusted. The rest of the OS may use this information to enable DMA protection against malicious devices. For instance the device may be put behind an IOMMU to keep it from accessing memory outside of what the driver has allocated for it. While at it, add a comment on top of prp_guids array explaining the possible caveat resulting when these GUIDs are treated equivalent. [1] https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports#identifying-externally-exposed-pcie-root-ports Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- drivers/acpi/property.c | 11 +++++++++++ drivers/pci/pci-acpi.c | 19 +++++++++++++++++++ drivers/pci/probe.c | 15 +++++++++++++++ include/linux/pci.h | 8 ++++++++ 4 files changed, 53 insertions(+)