linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/4] PCI / iommu / thunderbolt: IOMMU based DMA protection
@ 2018-11-29 15:51 Mika Westerberg
  2018-11-29 15:51 ` [PATCH v3 1/4] PCI / ACPI: Identify untrusted PCI devices Mika Westerberg
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Mika Westerberg @ 2018-11-29 15:51 UTC (permalink / raw)
  To: iommu
  Cc: Joerg Roedel, David Woodhouse, Lu Baolu, Ashok Raj,
	Bjorn Helgaas, Rafael J. Wysocki, Jacob jun Pan, Andreas Noever,
	Michael Jamet, Yehezkel Bernat, Lukas Wunner, Christian Kellner,
	Mario.Limonciello, Anthony Wong, Lorenzo Pieralisi,
	Christoph Hellwig, Alex Williamson, Mika Westerberg, linux-acpi,
	linux-pci, linux-kernel

Recent systems with Thunderbolt ports may be utilizing IOMMU to prevent DMA
attacks. This is different from the previous security level based scheme
because the connected device cannot access system memory outside of the
regions allocated for it by the driver.

When enabled the BIOS makes sure no device can do DMA outside of RMRR
(Reserved Memory Region Record) regions. This means that during OS boot,
before it enables IOMMU, none of the connected devices can bypass DMA
protection for instance by overwriting the data structures used by the
IOMMU. The BIOS communicates support for this to the OS by setting a new
bit in ACPI DMAR table [1].

Because these systems utilize an IOMMU to block possible DMA attacks,
typically (but not always) the Thunderbolt security level is set to "none"
which means that all PCIe devices are immediately usable. This also means
that Linux needs to follow Windows 10 and enable IOMMU automatically when
running on such system otherwise connected devices can read/write system
memory pretty much without any restrictions.

Since there is a way to identify PCIe root ports that are "external facing"
we can put all internal devices to pass through (identity mapping) mode and
only external devices need to go through full IOMMU mappings. We add a new
flag "untrusted" that is supposed to cover various PCIe devices that may be
used to conduct DMA attacks.

We also make sure PCIe ATS (Address Translation Service) is not enabled for
devices flagged as untrusted because it could be used to bypass IOMMU
completely as explained in the changelog of patch 3/4.

Finally we expose this information to userspace so tools such as bolt can
do more accurate decision whether or not authorize the connected device.

[1] https://software.intel.com/sites/default/files/managed/c5/15/vt-directed-io-spec.pdf

Previous version of the patch series can be found here:

  v2: https://lkml.org/lkml/2018/11/26/638
  v1: https://www.spinics.net/lists/linux-pci/msg77751.html

Changes from v2:
  
  * Rename the flag to "untrusted"
  * Simplify setting the flag for root ports
  * Dropped loop in set_pcie_untrusted()
  * Add comment on top of prp_guids explaining the possible caveat
    resulting when the new GUIDs are treated as equivalent
  * Updated changelogs according to feedback

Changes from v1:

  * Reword Documentation/admin-guide/thunderbolt.rst to make the feature
    time frame/platform oriented as there will be systems shipping
    with Linux installed by default.

  * Rename the flag is_external to is_untrusted so that we could use the
    same flag to cover all kinds of "untrusted" PCI devices, not just
    Thunderbolt connected devices. I still parse the _DSD in PCI/ACPI core
    because that's where we currently handle "HotPlugSupportInD3" as well.
    Also updated comments in patch [1/4].

  * Added tags from Ashok, Joerg and Yehezkel. I'm assuming they still
    apply because I did not change the code with the exception of few
    comments and rename of the flag. Let me know if that's not the case
    anymore.

Lu Baolu (1):
  iommu/vt-d: Force IOMMU on for platform opt in hint

Mika Westerberg (3):
  PCI / ACPI: Identify untrusted PCI devices
  iommu/vt-d: Do not enable ATS for untrusted devices
  thunderbolt: Export IOMMU based DMA protection support to userspace

 .../ABI/testing/sysfs-bus-thunderbolt         |  9 +++
 Documentation/admin-guide/thunderbolt.rst     | 20 +++++++
 drivers/acpi/property.c                       | 11 ++++
 drivers/iommu/dmar.c                          | 25 +++++++++
 drivers/iommu/intel-iommu.c                   | 56 ++++++++++++++++++-
 drivers/pci/pci-acpi.c                        | 19 +++++++
 drivers/pci/probe.c                           | 15 +++++
 drivers/thunderbolt/domain.c                  | 17 ++++++
 include/linux/dmar.h                          |  8 +++
 include/linux/pci.h                           |  8 +++
 10 files changed, 185 insertions(+), 3 deletions(-)

-- 
2.19.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3 1/4] PCI / ACPI: Identify untrusted PCI devices
  2018-11-29 15:51 [PATCH v3 0/4] PCI / iommu / thunderbolt: IOMMU based DMA protection Mika Westerberg
@ 2018-11-29 15:51 ` Mika Westerberg
  2018-11-29 20:59   ` Rafael J. Wysocki
  2018-12-04  0:28   ` Bjorn Helgaas
  2018-11-29 15:51 ` [PATCH v3 2/4] iommu/vt-d: Force IOMMU on for platform opt in hint Mika Westerberg
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 9+ messages in thread
From: Mika Westerberg @ 2018-11-29 15:51 UTC (permalink / raw)
  To: iommu
  Cc: Joerg Roedel, David Woodhouse, Lu Baolu, Ashok Raj,
	Bjorn Helgaas, Rafael J. Wysocki, Jacob jun Pan, Andreas Noever,
	Michael Jamet, Yehezkel Bernat, Lukas Wunner, Christian Kellner,
	Mario.Limonciello, Anthony Wong, Lorenzo Pieralisi,
	Christoph Hellwig, Alex Williamson, Mika Westerberg, linux-acpi,
	linux-pci, linux-kernel

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(+)

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 2/4] iommu/vt-d: Force IOMMU on for platform opt in hint
  2018-11-29 15:51 [PATCH v3 0/4] PCI / iommu / thunderbolt: IOMMU based DMA protection Mika Westerberg
  2018-11-29 15:51 ` [PATCH v3 1/4] PCI / ACPI: Identify untrusted PCI devices Mika Westerberg
@ 2018-11-29 15:51 ` Mika Westerberg
  2018-11-29 15:51 ` [PATCH v3 3/4] iommu/vt-d: Do not enable ATS for untrusted devices Mika Westerberg
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Mika Westerberg @ 2018-11-29 15:51 UTC (permalink / raw)
  To: iommu
  Cc: Joerg Roedel, David Woodhouse, Lu Baolu, Ashok Raj,
	Bjorn Helgaas, Rafael J. Wysocki, Jacob jun Pan, Andreas Noever,
	Michael Jamet, Yehezkel Bernat, Lukas Wunner, Christian Kellner,
	Mario.Limonciello, Anthony Wong, Lorenzo Pieralisi,
	Christoph Hellwig, Alex Williamson, Mika Westerberg, linux-acpi,
	linux-pci, linux-kernel

From: Lu Baolu <baolu.lu@linux.intel.com>

Intel VT-d spec added a new DMA_CTRL_PLATFORM_OPT_IN_FLAG flag in DMAR
ACPI table [1] for BIOS to report compliance about platform initiated
DMA restricted to RMRR ranges when transferring control to the OS. This
means that during OS boot, before it enables IOMMU none of the connected
devices can bypass DMA protection for instance by overwriting the data
structures used by the IOMMU. The OS also treats this as a hint that the
IOMMU should be enabled to prevent DMA attacks from possible malicious
devices.

A use of this flag is Kernel DMA protection for Thunderbolt [2] which in
practice means that IOMMU should be enabled for PCIe devices connected
to the Thunderbolt ports. With IOMMU enabled for these devices, all DMA
operations are limited in the range reserved for it, thus the DMA
attacks are prevented. All these devices are enumerated in the PCI/PCIe
module and marked with an untrusted flag.

This forces IOMMU to be enabled if DMA_CTRL_PLATFORM_OPT_IN_FLAG is set
in DMAR ACPI table and there are PCIe devices marked as untrusted in the
system. This can be turned off by adding "intel_iommu=off" in the kernel
command line, if any problems are found.

[1] https://software.intel.com/sites/default/files/managed/c5/15/vt-directed-io-spec.pdf
[2] https://docs.microsoft.com/en-us/windows/security/information-protection/kernel-dma-protection-for-thunderbolt

Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Sohil Mehta <sohil.mehta@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Ashok Raj <ashok.raj@intel.com>
Reviewed-by: Joerg Roedel <jroedel@suse.de>
Acked-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/dmar.c        | 25 +++++++++++++++++
 drivers/iommu/intel-iommu.c | 53 +++++++++++++++++++++++++++++++++++--
 include/linux/dmar.h        |  8 ++++++
 3 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index d9c748b6f9e4..1edf2a251336 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -2042,3 +2042,28 @@ int dmar_device_remove(acpi_handle handle)
 {
 	return dmar_device_hotplug(handle, false);
 }
+
+/*
+ * dmar_platform_optin - Is %DMA_CTRL_PLATFORM_OPT_IN_FLAG set in DMAR table
+ *
+ * Returns true if the platform has %DMA_CTRL_PLATFORM_OPT_IN_FLAG set in
+ * the ACPI DMAR table. This means that the platform boot firmware has made
+ * sure no device can issue DMA outside of RMRR regions.
+ */
+bool dmar_platform_optin(void)
+{
+	struct acpi_table_dmar *dmar;
+	acpi_status status;
+	bool ret;
+
+	status = acpi_get_table(ACPI_SIG_DMAR, 0,
+				(struct acpi_table_header **)&dmar);
+	if (ACPI_FAILURE(status))
+		return false;
+
+	ret = !!(dmar->flags & DMAR_PLATFORM_OPT_IN);
+	acpi_put_table((struct acpi_table_header *)dmar);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(dmar_platform_optin);
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 41a4b8808802..30e8584137f5 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -184,6 +184,7 @@ static int rwbf_quirk;
  */
 static int force_on = 0;
 int intel_iommu_tboot_noforce;
+static int no_platform_optin;
 
 #define ROOT_ENTRY_NR (VTD_PAGE_SIZE/sizeof(struct root_entry))
 
@@ -503,6 +504,7 @@ static int __init intel_iommu_setup(char *str)
 			pr_info("IOMMU enabled\n");
 		} else if (!strncmp(str, "off", 3)) {
 			dmar_disabled = 1;
+			no_platform_optin = 1;
 			pr_info("IOMMU disabled\n");
 		} else if (!strncmp(str, "igfx_off", 8)) {
 			dmar_map_gfx = 0;
@@ -2895,6 +2897,13 @@ static int iommu_should_identity_map(struct device *dev, int startup)
 		if (device_is_rmrr_locked(dev))
 			return 0;
 
+		/*
+		 * Prevent any device marked as untrusted from getting
+		 * placed into the statically identity mapping domain.
+		 */
+		if (pdev->untrusted)
+			return 0;
+
 		if ((iommu_identity_mapping & IDENTMAP_AZALIA) && IS_AZALIA(pdev))
 			return 1;
 
@@ -4728,14 +4737,54 @@ const struct attribute_group *intel_iommu_groups[] = {
 	NULL,
 };
 
+static int __init platform_optin_force_iommu(void)
+{
+	struct pci_dev *pdev = NULL;
+	bool has_untrusted_dev = false;
+
+	if (!dmar_platform_optin() || no_platform_optin)
+		return 0;
+
+	for_each_pci_dev(pdev) {
+		if (pdev->untrusted) {
+			has_untrusted_dev = true;
+			break;
+		}
+	}
+
+	if (!has_untrusted_dev)
+		return 0;
+
+	if (no_iommu || dmar_disabled)
+		pr_info("Intel-IOMMU force enabled due to platform opt in\n");
+
+	/*
+	 * If Intel-IOMMU is disabled by default, we will apply identity
+	 * map for all devices except those marked as being untrusted.
+	 */
+	if (dmar_disabled)
+		iommu_identity_mapping |= IDENTMAP_ALL;
+
+	dmar_disabled = 0;
+#if defined(CONFIG_X86) && defined(CONFIG_SWIOTLB)
+	swiotlb = 0;
+#endif
+	no_iommu = 0;
+
+	return 1;
+}
+
 int __init intel_iommu_init(void)
 {
 	int ret = -ENODEV;
 	struct dmar_drhd_unit *drhd;
 	struct intel_iommu *iommu;
 
-	/* VT-d is required for a TXT/tboot launch, so enforce that */
-	force_on = tboot_force_iommu();
+	/*
+	 * Intel IOMMU is required for a TXT/tboot launch or platform
+	 * opt in, so enforce that.
+	 */
+	force_on = tboot_force_iommu() || platform_optin_force_iommu();
 
 	if (iommu_init_mempool()) {
 		if (force_on)
diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index 843a41ba7e28..f8af1d770520 100644
--- a/include/linux/dmar.h
+++ b/include/linux/dmar.h
@@ -39,6 +39,7 @@ struct acpi_dmar_header;
 /* DMAR Flags */
 #define DMAR_INTR_REMAP		0x1
 #define DMAR_X2APIC_OPT_OUT	0x2
+#define DMAR_PLATFORM_OPT_IN	0x4
 
 struct intel_iommu;
 
@@ -170,6 +171,8 @@ static inline int dmar_ir_hotplug(struct dmar_drhd_unit *dmaru, bool insert)
 { return 0; }
 #endif /* CONFIG_IRQ_REMAP */
 
+extern bool dmar_platform_optin(void);
+
 #else /* CONFIG_DMAR_TABLE */
 
 static inline int dmar_device_add(void *handle)
@@ -182,6 +185,11 @@ static inline int dmar_device_remove(void *handle)
 	return 0;
 }
 
+static inline bool dmar_platform_optin(void)
+{
+	return false;
+}
+
 #endif /* CONFIG_DMAR_TABLE */
 
 struct irte {
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 3/4] iommu/vt-d: Do not enable ATS for untrusted devices
  2018-11-29 15:51 [PATCH v3 0/4] PCI / iommu / thunderbolt: IOMMU based DMA protection Mika Westerberg
  2018-11-29 15:51 ` [PATCH v3 1/4] PCI / ACPI: Identify untrusted PCI devices Mika Westerberg
  2018-11-29 15:51 ` [PATCH v3 2/4] iommu/vt-d: Force IOMMU on for platform opt in hint Mika Westerberg
@ 2018-11-29 15:51 ` Mika Westerberg
  2018-11-29 15:51 ` [PATCH v3 4/4] thunderbolt: Export IOMMU based DMA protection support to userspace Mika Westerberg
  2018-12-05  9:11 ` [PATCH v3 0/4] PCI / iommu / thunderbolt: IOMMU based DMA protection Mika Westerberg
  4 siblings, 0 replies; 9+ messages in thread
From: Mika Westerberg @ 2018-11-29 15:51 UTC (permalink / raw)
  To: iommu
  Cc: Joerg Roedel, David Woodhouse, Lu Baolu, Ashok Raj,
	Bjorn Helgaas, Rafael J. Wysocki, Jacob jun Pan, Andreas Noever,
	Michael Jamet, Yehezkel Bernat, Lukas Wunner, Christian Kellner,
	Mario.Limonciello, Anthony Wong, Lorenzo Pieralisi,
	Christoph Hellwig, Alex Williamson, Mika Westerberg, linux-acpi,
	linux-pci, linux-kernel

Currently Linux automatically enables ATS (Address Translation Service)
for any device that supports it (and IOMMU is turned on). ATS is used to
accelerate DMA access as the device can cache translations locally so
there is no need to do full translation on IOMMU side. However, as
pointed out in [1] ATS can be used to bypass IOMMU based security
completely by simply sending PCIe read/write transaction with AT
(Address Translation) field set to "translated".

To mitigate this modify the Intel IOMMU code so that it does not enable
ATS for any device that is marked as being untrusted. In case this turns
out to cause performance issues we may selectively allow ATS based on
user decision but currently use big hammer and disable it completely to
be on the safe side.

[1] https://www.repository.cam.ac.uk/handle/1810/274352

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Ashok Raj <ashok.raj@intel.com>
Reviewed-by: Joerg Roedel <jroedel@suse.de>
Acked-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 30e8584137f5..adc2c9619e56 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1473,7 +1473,8 @@ static void iommu_enable_dev_iotlb(struct device_domain_info *info)
 	if (info->pri_supported && !pci_reset_pri(pdev) && !pci_enable_pri(pdev, 32))
 		info->pri_enabled = 1;
 #endif
-	if (info->ats_supported && !pci_enable_ats(pdev, VTD_PAGE_SHIFT)) {
+	if (!pdev->untrusted && info->ats_supported &&
+	    !pci_enable_ats(pdev, VTD_PAGE_SHIFT)) {
 		info->ats_enabled = 1;
 		domain_update_iotlb(info->domain);
 		info->ats_qdep = pci_ats_queue_depth(pdev);
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 4/4] thunderbolt: Export IOMMU based DMA protection support to userspace
  2018-11-29 15:51 [PATCH v3 0/4] PCI / iommu / thunderbolt: IOMMU based DMA protection Mika Westerberg
                   ` (2 preceding siblings ...)
  2018-11-29 15:51 ` [PATCH v3 3/4] iommu/vt-d: Do not enable ATS for untrusted devices Mika Westerberg
@ 2018-11-29 15:51 ` Mika Westerberg
  2018-12-05  9:11 ` [PATCH v3 0/4] PCI / iommu / thunderbolt: IOMMU based DMA protection Mika Westerberg
  4 siblings, 0 replies; 9+ messages in thread
From: Mika Westerberg @ 2018-11-29 15:51 UTC (permalink / raw)
  To: iommu
  Cc: Joerg Roedel, David Woodhouse, Lu Baolu, Ashok Raj,
	Bjorn Helgaas, Rafael J. Wysocki, Jacob jun Pan, Andreas Noever,
	Michael Jamet, Yehezkel Bernat, Lukas Wunner, Christian Kellner,
	Mario.Limonciello, Anthony Wong, Lorenzo Pieralisi,
	Christoph Hellwig, Alex Williamson, Mika Westerberg, linux-acpi,
	linux-pci, linux-kernel

Recent systems with Thunderbolt ports may support IOMMU natively. In
practice this means that Thunderbolt connected devices are placed behind
an IOMMU during the whole time it is connected (including during boot)
making Thunderbolt security levels redundant. This is called Kernel DMA
protection [1] by Microsoft.

Some of these systems still have Thunderbolt security level set to
"user" in order to support OS downgrade (the older version of the OS
might not support IOMMU based DMA protection so connecting a device
still relies on user approval).

Export this information to userspace by introducing a new sysfs
attribute (iommu_dma_protection). Based on it userspace tools can make
more accurate decision whether or not authorize the connected device.

In addition update Thunderbolt documentation regarding IOMMU based DMA
protection.

[1] https://docs.microsoft.com/en-us/windows/security/information-protection/kernel-dma-protection-for-thunderbolt

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Yehezkel Bernat <YehezkelShB@gmail.com>
---
 .../ABI/testing/sysfs-bus-thunderbolt         |  9 +++++++++
 Documentation/admin-guide/thunderbolt.rst     | 20 +++++++++++++++++++
 drivers/thunderbolt/domain.c                  | 17 ++++++++++++++++
 3 files changed, 46 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-thunderbolt b/Documentation/ABI/testing/sysfs-bus-thunderbolt
index 151584a1f950..b21fba14689b 100644
--- a/Documentation/ABI/testing/sysfs-bus-thunderbolt
+++ b/Documentation/ABI/testing/sysfs-bus-thunderbolt
@@ -21,6 +21,15 @@ Description:	Holds a comma separated list of device unique_ids that
 		If a device is authorized automatically during boot its
 		boot attribute is set to 1.
 
+What: /sys/bus/thunderbolt/devices/.../domainX/iommu_dma_protection
+Date:		Mar 2019
+KernelVersion:	4.21
+Contact:	thunderbolt-software@lists.01.org
+Description:	This attribute tells whether the system uses IOMMU
+		for DMA protection. Value of 1 means IOMMU is used 0 means
+		it is not (DMA protection is solely based on Thunderbolt
+		security levels).
+
 What: /sys/bus/thunderbolt/devices/.../domainX/security
 Date:		Sep 2017
 KernelVersion:	4.13
diff --git a/Documentation/admin-guide/thunderbolt.rst b/Documentation/admin-guide/thunderbolt.rst
index 35fccba6a9a6..898ad78f3cc7 100644
--- a/Documentation/admin-guide/thunderbolt.rst
+++ b/Documentation/admin-guide/thunderbolt.rst
@@ -133,6 +133,26 @@ If the user still wants to connect the device they can either approve
 the device without a key or write a new key and write 1 to the
 ``authorized`` file to get the new key stored on the device NVM.
 
+DMA protection utilizing IOMMU
+------------------------------
+Recent systems from 2018 and forward with Thunderbolt ports may natively
+support IOMMU. This means that Thunderbolt security is handled by an IOMMU
+so connected devices cannot access memory regions outside of what is
+allocated for them by drivers. When Linux is running on such system it
+automatically enables IOMMU if not enabled by the user already. These
+systems can be identified by reading ``1`` from
+``/sys/bus/thunderbolt/devices/domainX/iommu_dma_protection`` attribute.
+
+The driver does not do anything special in this case but because DMA
+protection is handled by the IOMMU, security levels (if set) are
+redundant. For this reason some systems ship with security level set to
+``none``. Other systems have security level set to ``user`` in order to
+support downgrade to older OS, so users who want to automatically
+authorize devices when IOMMU DMA protection is enabled can use the
+following ``udev`` rule::
+
+  ACTION=="add", SUBSYSTEM=="thunderbolt", ATTRS{iommu_dma_protection}=="1", ATTR{authorized}=="0", ATTR{authorized}="1"
+
 Upgrading NVM on Thunderbolt device or host
 -------------------------------------------
 Since most of the functionality is handled in firmware running on a
diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c
index 93e562f18d40..7416bdbd8576 100644
--- a/drivers/thunderbolt/domain.c
+++ b/drivers/thunderbolt/domain.c
@@ -7,7 +7,9 @@
  */
 
 #include <linux/device.h>
+#include <linux/dmar.h>
 #include <linux/idr.h>
+#include <linux/iommu.h>
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
 #include <linux/slab.h>
@@ -236,6 +238,20 @@ static ssize_t boot_acl_store(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RW(boot_acl);
 
+static ssize_t iommu_dma_protection_show(struct device *dev,
+					 struct device_attribute *attr,
+					 char *buf)
+{
+	/*
+	 * Kernel DMA protection is a feature where Thunderbolt security is
+	 * handled natively using IOMMU. It is enabled when IOMMU is
+	 * enabled and ACPI DMAR table has DMAR_PLATFORM_OPT_IN set.
+	 */
+	return sprintf(buf, "%d\n",
+		       iommu_present(&pci_bus_type) && dmar_platform_optin());
+}
+static DEVICE_ATTR_RO(iommu_dma_protection);
+
 static ssize_t security_show(struct device *dev, struct device_attribute *attr,
 			     char *buf)
 {
@@ -251,6 +267,7 @@ static DEVICE_ATTR_RO(security);
 
 static struct attribute *domain_attrs[] = {
 	&dev_attr_boot_acl.attr,
+	&dev_attr_iommu_dma_protection.attr,
 	&dev_attr_security.attr,
 	NULL,
 };
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 1/4] PCI / ACPI: Identify untrusted PCI devices
  2018-11-29 15:51 ` [PATCH v3 1/4] PCI / ACPI: Identify untrusted PCI devices Mika Westerberg
@ 2018-11-29 20:59   ` Rafael J. Wysocki
  2018-12-04  0:28   ` Bjorn Helgaas
  1 sibling, 0 replies; 9+ messages in thread
From: Rafael J. Wysocki @ 2018-11-29 20:59 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: open list:AMD IOMMU (AMD-VI),
	Joerg Roedel, David Woodhouse, Lu Baolu, Raj, Ashok,
	Bjorn Helgaas, Rafael J. Wysocki, Pan, Jacob jun, Andreas Noever,
	Michael Jamet, Yehezkel Bernat, Lukas Wunner, ckellner,
	Mario Limonciello, Anthony Wong, Lorenzo Pieralisi,
	Christoph Hellwig, Alex Williamson, ACPI Devel Maling List,
	Linux PCI, Linux Kernel Mailing List

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
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 1/4] PCI / ACPI: Identify untrusted PCI devices
  2018-11-29 15:51 ` [PATCH v3 1/4] PCI / ACPI: Identify untrusted PCI devices Mika Westerberg
  2018-11-29 20:59   ` Rafael J. Wysocki
@ 2018-12-04  0:28   ` Bjorn Helgaas
  2018-12-04  8:57     ` Mika Westerberg
  1 sibling, 1 reply; 9+ messages in thread
From: Bjorn Helgaas @ 2018-12-04  0:28 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: iommu, Joerg Roedel, David Woodhouse, Lu Baolu, Ashok Raj,
	Rafael J. Wysocki, Jacob jun Pan, Andreas Noever, Michael Jamet,
	Yehezkel Bernat, Lukas Wunner, Christian Kellner,
	Mario.Limonciello, Anthony Wong, Lorenzo Pieralisi,
	Christoph Hellwig, Alex Williamson, linux-acpi, linux-pci,
	linux-kernel

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
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 1/4] PCI / ACPI: Identify untrusted PCI devices
  2018-12-04  0:28   ` Bjorn Helgaas
@ 2018-12-04  8:57     ` Mika Westerberg
  0 siblings, 0 replies; 9+ messages in thread
From: Mika Westerberg @ 2018-12-04  8:57 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: iommu, Joerg Roedel, David Woodhouse, Lu Baolu, Ashok Raj,
	Rafael J. Wysocki, Jacob jun Pan, Andreas Noever, Michael Jamet,
	Yehezkel Bernat, Lukas Wunner, Christian Kellner,
	Mario.Limonciello, Anthony Wong, Lorenzo Pieralisi,
	Christoph Hellwig, Alex Williamson, linux-acpi, linux-pci,
	linux-kernel

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!

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 0/4] PCI / iommu / thunderbolt: IOMMU based DMA protection
  2018-11-29 15:51 [PATCH v3 0/4] PCI / iommu / thunderbolt: IOMMU based DMA protection Mika Westerberg
                   ` (3 preceding siblings ...)
  2018-11-29 15:51 ` [PATCH v3 4/4] thunderbolt: Export IOMMU based DMA protection support to userspace Mika Westerberg
@ 2018-12-05  9:11 ` Mika Westerberg
  4 siblings, 0 replies; 9+ messages in thread
From: Mika Westerberg @ 2018-12-05  9:11 UTC (permalink / raw)
  To: iommu
  Cc: Joerg Roedel, David Woodhouse, Lu Baolu, Ashok Raj,
	Bjorn Helgaas, Rafael J. Wysocki, Jacob jun Pan, Andreas Noever,
	Michael Jamet, Yehezkel Bernat, Lukas Wunner, Christian Kellner,
	Mario.Limonciello, Anthony Wong, Lorenzo Pieralisi,
	Christoph Hellwig, Alex Williamson, linux-acpi, linux-pci,
	linux-kernel

On Thu, Nov 29, 2018 at 06:51:49PM +0300, Mika Westerberg wrote:
> Recent systems with Thunderbolt ports may be utilizing IOMMU to prevent DMA
> attacks. This is different from the previous security level based scheme
> because the connected device cannot access system memory outside of the
> regions allocated for it by the driver.

Applied all to thunderbolt.git/next with acks from Bjorn and Rafael.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-12-05  9:11 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-29 15:51 [PATCH v3 0/4] PCI / iommu / thunderbolt: IOMMU based DMA protection Mika Westerberg
2018-11-29 15:51 ` [PATCH v3 1/4] PCI / ACPI: Identify untrusted PCI devices Mika Westerberg
2018-11-29 20:59   ` Rafael J. Wysocki
2018-12-04  0:28   ` Bjorn Helgaas
2018-12-04  8:57     ` Mika Westerberg
2018-11-29 15:51 ` [PATCH v3 2/4] iommu/vt-d: Force IOMMU on for platform opt in hint Mika Westerberg
2018-11-29 15:51 ` [PATCH v3 3/4] iommu/vt-d: Do not enable ATS for untrusted devices Mika Westerberg
2018-11-29 15:51 ` [PATCH v3 4/4] thunderbolt: Export IOMMU based DMA protection support to userspace Mika Westerberg
2018-12-05  9:11 ` [PATCH v3 0/4] PCI / iommu / thunderbolt: IOMMU based DMA protection Mika Westerberg

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).