linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/9] Bounce buffer for untrusted devices
@ 2019-03-12  5:59 Lu Baolu
  2019-03-12  5:59 ` [PATCH v1 1/9] iommu/vt-d: Add trace events for domain map/unmap Lu Baolu
                   ` (9 more replies)
  0 siblings, 10 replies; 18+ messages in thread
From: Lu Baolu @ 2019-03-12  5:59 UTC (permalink / raw)
  To: David Woodhouse, Joerg Roedel, ashok.raj, jacob.jun.pan,
	alan.cox, kevin.tian, mika.westerberg, pengfei.xu
  Cc: iommu, linux-kernel, Lu Baolu

An external PCI device is a PCI peripheral device connected
to the system through an external bus, such as Thunderbolt.
What makes it different is that it can't be trusted to the
same degree as the devices build into the system. Generally,
a trusted PCIe device will DMA into the designated buffers
and not overrun or otherwise write outside the specified
bounds. But it's different for an external device. The minimum
IOMMU mapping granularity is one page (4k), so for DMA transfers
smaller than that a malicious PCIe device can access the whole
page of memory even if it does not belong to the driver in
question. This opens a possibility for DMA attack. For more
information about DMA attacks imposed by an untrusted PCI/PCIe
device, please refer to [2].

This implements bounce buffer for the untrusted external
devices. The transfers should be limited in isolated pages
so the IOMMU window does not cover memory outside of what
the driver expects. Full pages within a buffer could be
directly mapped in IOMMU page table, but for partial pages
we use bounce page instead.

In addition, the IOMMU mappings cached in the IOTLB for
untrusted devices should be invalidated immediately after
the unmap operation. Otherwise, the IOMMU window is left
open to attacks.

The implementation of bounce buffers for untrusted devices
will cause a little performance overhead, but we didn't see
any user experience problems. The users could use the kernel
parameter of "intel_iommu=nobounce" to remove the performance
overhead if they trust their devices enough.

The Thunderbolt vulnerabiltiies is public and has a nice
name as Thunderclap nowadays. Please refer to [1] [3] for
more information. This patch series aims to mitigate the
concerns.

The bounce buffer idea:

Based-on-idea-by: Mika Westerberg <mika.westerberg@intel.com>
Based-on-idea-by: Ashok Raj <ashok.raj@intel.com>
Based-on-idea-by: Alan Cox <alan.cox@intel.com>

The patch series has been tested by:

Tested-by: Xu Pengfei <pengfei.xu@intel.com>
Tested-by: Mika Westerberg <mika.westerberg@intel.com>

[1] https://thunderclap.io/
[2] https://thunderclap.io/thunderclap-paper-ndss2019.pdf
[3] https://christian.kellner.me/2019/02/27/thunderclap-and-linux/

Lu Baolu (9):
  iommu/vt-d: Add trace events for domain map/unmap
  iommu/vt-d: Add helpers for domain mapping/unmapping
  iommu/vt-d: Add address walk helper
  iommu/vt-d: Add bounce buffer API for domain map/unmap
  iommu/vt-d: Add bounce buffer API for dma sync
  iommu/vt-d: Check whether device requires bounce buffer
  iommu/vt-d: Add dma sync ops for untrusted devices
  iommu/vt-d: Flush IOTLB for untrusted device in time
  iommu/vt-d: Use bounce buffer for untrusted devices

 .../admin-guide/kernel-parameters.txt         |   5 +
 drivers/iommu/Makefile                        |   1 +
 drivers/iommu/intel-iommu.c                   | 360 ++++++++++--
 drivers/iommu/intel-pgtable.c                 | 518 ++++++++++++++++++
 drivers/iommu/intel-trace.c                   |  14 +
 include/linux/intel-iommu.h                   |  24 +
 include/trace/events/intel_iommu.h            | 132 +++++
 7 files changed, 1010 insertions(+), 44 deletions(-)
 create mode 100644 drivers/iommu/intel-pgtable.c
 create mode 100644 drivers/iommu/intel-trace.c
 create mode 100644 include/trace/events/intel_iommu.h

-- 
2.17.1


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

* [PATCH v1 1/9] iommu/vt-d: Add trace events for domain map/unmap
  2019-03-12  5:59 [PATCH v1 0/9] Bounce buffer for untrusted devices Lu Baolu
@ 2019-03-12  5:59 ` Lu Baolu
  2019-03-12  5:59 ` [PATCH v1 2/9] iommu/vt-d: Add helpers for domain mapping/unmapping Lu Baolu
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Lu Baolu @ 2019-03-12  5:59 UTC (permalink / raw)
  To: David Woodhouse, Joerg Roedel, ashok.raj, jacob.jun.pan,
	alan.cox, kevin.tian, mika.westerberg, pengfei.xu
  Cc: iommu, linux-kernel, Lu Baolu, Jacob Pan

This adds trace support for the Intel IOMMU driver. It
also declares some events which could be used to trace
the events when an IOVA is being mapped or unmapped in
a domain.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/Makefile             |   1 +
 drivers/iommu/intel-trace.c        |  14 +++
 include/trace/events/intel_iommu.h | 132 +++++++++++++++++++++++++++++
 3 files changed, 147 insertions(+)
 create mode 100644 drivers/iommu/intel-trace.c
 create mode 100644 include/trace/events/intel_iommu.h

diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 8c71a15e986b..8b5fb8051281 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_ARM_SMMU) += arm-smmu.o
 obj-$(CONFIG_ARM_SMMU_V3) += arm-smmu-v3.o
 obj-$(CONFIG_DMAR_TABLE) += dmar.o
 obj-$(CONFIG_INTEL_IOMMU) += intel-iommu.o intel-pasid.o
+obj-$(CONFIG_INTEL_IOMMU) += intel-trace.o
 obj-$(CONFIG_INTEL_IOMMU_DEBUGFS) += intel-iommu-debugfs.o
 obj-$(CONFIG_INTEL_IOMMU_SVM) += intel-svm.o
 obj-$(CONFIG_IPMMU_VMSA) += ipmmu-vmsa.o
diff --git a/drivers/iommu/intel-trace.c b/drivers/iommu/intel-trace.c
new file mode 100644
index 000000000000..bfb6a6e37a88
--- /dev/null
+++ b/drivers/iommu/intel-trace.c
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Intel IOMMU trace support
+ *
+ * Copyright (C) 2019 Intel Corporation
+ *
+ * Author: Lu Baolu <baolu.lu@linux.intel.com>
+ */
+
+#include <linux/string.h>
+#include <linux/types.h>
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/intel_iommu.h>
diff --git a/include/trace/events/intel_iommu.h b/include/trace/events/intel_iommu.h
new file mode 100644
index 000000000000..49ca57a90079
--- /dev/null
+++ b/include/trace/events/intel_iommu.h
@@ -0,0 +1,132 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Intel IOMMU trace support
+ *
+ * Copyright (C) 2019 Intel Corporation
+ *
+ * Author: Lu Baolu <baolu.lu@linux.intel.com>
+ */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM intel_iommu
+
+#if !defined(_TRACE_INTEL_IOMMU_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_INTEL_IOMMU_H
+
+#include <linux/tracepoint.h>
+#include <linux/intel-iommu.h>
+
+TRACE_EVENT(bounce_map_single,
+	TP_PROTO(struct device *dev, dma_addr_t dev_addr, phys_addr_t phys_addr,
+		 size_t size),
+
+	TP_ARGS(dev, dev_addr, phys_addr, size),
+
+	TP_STRUCT__entry(
+		__string(dev_name, dev_name(dev))
+		__field(dma_addr_t, dev_addr)
+		__field(phys_addr_t, phys_addr)
+		__field(size_t,	size)
+	),
+
+	TP_fast_assign(
+		__assign_str(dev_name, dev_name(dev));
+		__entry->dev_addr = dev_addr;
+		__entry->phys_addr = phys_addr;
+		__entry->size = size;
+	),
+
+	TP_printk("dev=%s dev_addr=0x%llx phys_addr=0x%llx size=%zu",
+		  __get_str(dev_name),
+		  (unsigned long long)__entry->dev_addr,
+		  (unsigned long long)__entry->phys_addr,
+		  __entry->size)
+);
+
+TRACE_EVENT(bounce_unmap_single,
+	TP_PROTO(struct device *dev, dma_addr_t dev_addr, size_t size),
+
+	TP_ARGS(dev, dev_addr, size),
+
+	TP_STRUCT__entry(
+		__string(dev_name, dev_name(dev))
+		__field(dma_addr_t, dev_addr)
+		__field(size_t,	size)
+	),
+
+	TP_fast_assign(
+		__assign_str(dev_name, dev_name(dev));
+		__entry->dev_addr = dev_addr;
+		__entry->size = size;
+	),
+
+	TP_printk("dev=%s dev_addr=0x%llx size=%zu",
+		  __get_str(dev_name),
+		  (unsigned long long)__entry->dev_addr,
+		  __entry->size)
+);
+
+TRACE_EVENT(bounce_map_sg,
+	TP_PROTO(struct device *dev, unsigned int i, unsigned int nelems,
+		 dma_addr_t dev_addr, phys_addr_t phys_addr, size_t size),
+
+	TP_ARGS(dev, i, nelems, dev_addr, phys_addr, size),
+
+	TP_STRUCT__entry(
+		__string(dev_name, dev_name(dev))
+		__field(unsigned int, i)
+		__field(unsigned int, last)
+		__field(dma_addr_t, dev_addr)
+		__field(phys_addr_t, phys_addr)
+		__field(size_t,	size)
+	),
+
+	TP_fast_assign(
+		__assign_str(dev_name, dev_name(dev));
+		__entry->i = i;
+		__entry->last = nelems - 1;
+		__entry->dev_addr = dev_addr;
+		__entry->phys_addr = phys_addr;
+		__entry->size = size;
+	),
+
+	TP_printk("dev=%s elem=%u/%u dev_addr=0x%llx phys_addr=0x%llx size=%zu",
+		  __get_str(dev_name), __entry->i, __entry->last,
+		  (unsigned long long)__entry->dev_addr,
+		  (unsigned long long)__entry->phys_addr,
+		  __entry->size)
+);
+
+TRACE_EVENT(bounce_unmap_sg,
+	TP_PROTO(struct device *dev, unsigned int i, unsigned int nelems,
+		 dma_addr_t dev_addr, phys_addr_t phys_addr, size_t size),
+
+	TP_ARGS(dev, i, nelems, dev_addr, phys_addr, size),
+
+	TP_STRUCT__entry(
+		__string(dev_name, dev_name(dev))
+		__field(unsigned int, i)
+		__field(unsigned int, last)
+		__field(dma_addr_t, dev_addr)
+		__field(phys_addr_t, phys_addr)
+		__field(size_t,	size)
+	),
+
+	TP_fast_assign(
+		__assign_str(dev_name, dev_name(dev));
+		__entry->i = i;
+		__entry->last = nelems - 1;
+		__entry->dev_addr = dev_addr;
+		__entry->phys_addr = phys_addr;
+		__entry->size = size;
+	),
+
+	TP_printk("dev=%s elem=%u/%u dev_addr=0x%llx phys_addr=0x%llx size=%zu",
+		  __get_str(dev_name), __entry->i, __entry->last,
+		  (unsigned long long)__entry->dev_addr,
+		  (unsigned long long)__entry->phys_addr,
+		  __entry->size)
+);
+#endif /* _TRACE_INTEL_IOMMU_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
-- 
2.17.1


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

* [PATCH v1 2/9] iommu/vt-d: Add helpers for domain mapping/unmapping
  2019-03-12  5:59 [PATCH v1 0/9] Bounce buffer for untrusted devices Lu Baolu
  2019-03-12  5:59 ` [PATCH v1 1/9] iommu/vt-d: Add trace events for domain map/unmap Lu Baolu
@ 2019-03-12  5:59 ` Lu Baolu
  2019-03-12  5:59 ` [PATCH v1 3/9] iommu/vt-d: Add address walk helper Lu Baolu
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Lu Baolu @ 2019-03-12  5:59 UTC (permalink / raw)
  To: David Woodhouse, Joerg Roedel, ashok.raj, jacob.jun.pan,
	alan.cox, kevin.tian, mika.westerberg, pengfei.xu
  Cc: iommu, linux-kernel, Lu Baolu, Jacob Pan

This adds two helpers to map or unmap a physically
contiguous memory region in the page table of an
iommu domain.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Tested-by: Xu Pengfei <pengfei.xu@intel.com>
Tested-by: Mika Westerberg <mika.westerberg@intel.com>
---
 drivers/iommu/intel-iommu.c | 35 +++++++++++++++++++++++++++++++++++
 include/linux/intel-iommu.h |  5 +++++
 2 files changed, 40 insertions(+)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 7b433772bce0..791261afb4a9 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5847,3 +5847,38 @@ static void __init check_tylersburg_isoch(void)
 	pr_warn("Recommended TLB entries for ISOCH unit is 16; your BIOS set %d\n",
 	       vtisochctrl);
 }
+
+/*
+ * Map a physically contiguous memory region in the dma page table
+ * of an iommu domain.
+ */
+int
+domain_iomap_range(struct dmar_domain *domain, unsigned long addr,
+		   phys_addr_t paddr, size_t size, int prot)
+{
+	return domain_mapping(domain,
+			      mm_to_dma_pfn(IOVA_PFN(addr)),
+			      NULL,
+			      mm_to_dma_pfn(IOVA_PFN(paddr)),
+			      aligned_nrpages(addr, size),
+			      prot);
+}
+
+/*
+ * Unmap a physically contiguous memory region in the dma page table
+ * of an iommu domain. Return the free page list that should be freed
+ * after iotlb flush. We can't free these pages here since it possibly
+ * is still used by iotlb cache for a DMA translation.
+ */
+struct page *
+domain_iounmap_range(struct dmar_domain *domain, unsigned long addr,
+		     size_t size)
+{
+	unsigned long nrpages, start_pfn, last_pfn;
+
+	nrpages = aligned_nrpages(addr, size);
+	start_pfn = mm_to_dma_pfn(IOVA_PFN(addr));
+	last_pfn = start_pfn + nrpages - 1;
+
+	return domain_unmap(domain, start_pfn, last_pfn);
+}
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 6925a18a5ca3..74afedfe193b 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -663,6 +663,11 @@ int for_each_device_domain(int (*fn)(struct device_domain_info *info,
 void iommu_flush_write_buffer(struct intel_iommu *iommu);
 int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct device *dev);
 
+int domain_iomap_range(struct dmar_domain *domain, unsigned long addr,
+		       phys_addr_t paddr, size_t size, int prot);
+struct page *domain_iounmap_range(struct dmar_domain *domain,
+				  unsigned long addr, size_t size);
+
 #ifdef CONFIG_INTEL_IOMMU_SVM
 int intel_svm_init(struct intel_iommu *iommu);
 extern int intel_svm_enable_prq(struct intel_iommu *iommu);
-- 
2.17.1


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

* [PATCH v1 3/9] iommu/vt-d: Add address walk helper
  2019-03-12  5:59 [PATCH v1 0/9] Bounce buffer for untrusted devices Lu Baolu
  2019-03-12  5:59 ` [PATCH v1 1/9] iommu/vt-d: Add trace events for domain map/unmap Lu Baolu
  2019-03-12  5:59 ` [PATCH v1 2/9] iommu/vt-d: Add helpers for domain mapping/unmapping Lu Baolu
@ 2019-03-12  5:59 ` Lu Baolu
  2019-03-12  6:00 ` [PATCH v1 4/9] iommu/vt-d: Add bounce buffer API for domain map/unmap Lu Baolu
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Lu Baolu @ 2019-03-12  5:59 UTC (permalink / raw)
  To: David Woodhouse, Joerg Roedel, ashok.raj, jacob.jun.pan,
	alan.cox, kevin.tian, mika.westerberg, pengfei.xu
  Cc: iommu, linux-kernel, Lu Baolu, Jacob Pan

This adds a helper to walk a contiguous dma address
and divide the address space into possiblely three
parts: a start partial page, middle full pages and
an end partial page, and call the callback for each
part of the address.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Tested-by: Xu Pengfei <pengfei.xu@intel.com>
Tested-by: Mika Westerberg <mika.westerberg@intel.com>
---
 drivers/iommu/Makefile        |   2 +-
 drivers/iommu/intel-pgtable.c | 109 ++++++++++++++++++++++++++++++++++
 include/linux/intel-iommu.h   |   6 ++
 3 files changed, 116 insertions(+), 1 deletion(-)
 create mode 100644 drivers/iommu/intel-pgtable.c

diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 8b5fb8051281..562c6a526d63 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -17,7 +17,7 @@ obj-$(CONFIG_ARM_SMMU) += arm-smmu.o
 obj-$(CONFIG_ARM_SMMU_V3) += arm-smmu-v3.o
 obj-$(CONFIG_DMAR_TABLE) += dmar.o
 obj-$(CONFIG_INTEL_IOMMU) += intel-iommu.o intel-pasid.o
-obj-$(CONFIG_INTEL_IOMMU) += intel-trace.o
+obj-$(CONFIG_INTEL_IOMMU) += intel-trace.o intel-pgtable.o
 obj-$(CONFIG_INTEL_IOMMU_DEBUGFS) += intel-iommu-debugfs.o
 obj-$(CONFIG_INTEL_IOMMU_SVM) += intel-svm.o
 obj-$(CONFIG_IPMMU_VMSA) += ipmmu-vmsa.o
diff --git a/drivers/iommu/intel-pgtable.c b/drivers/iommu/intel-pgtable.c
new file mode 100644
index 000000000000..ad3347d7ac1d
--- /dev/null
+++ b/drivers/iommu/intel-pgtable.c
@@ -0,0 +1,109 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * intel-pgtable.c - Utilities for page table manipulation
+ *
+ * Copyright (C) 2019 Intel Corporation
+ *
+ * Author: Lu Baolu <baolu.lu@linux.intel.com>
+ */
+
+#define pr_fmt(fmt)	"DMAR: " fmt
+
+#include <linux/dmar.h>
+#include <linux/highmem.h>
+#include <linux/intel-iommu.h>
+#include <linux/iommu.h>
+#include <trace/events/intel_iommu.h>
+
+struct addr_walk {
+	int (*low)(struct dmar_domain *domain, dma_addr_t addr,
+			phys_addr_t paddr, size_t size,
+			struct bounce_param *param);
+	int (*middle)(struct dmar_domain *domain, dma_addr_t addr,
+			phys_addr_t paddr, size_t size,
+			struct bounce_param *param);
+	int (*high)(struct dmar_domain *domain, dma_addr_t addr,
+			phys_addr_t paddr, size_t size,
+			struct bounce_param *param);
+};
+
+/*
+ * Bounce buffer support for external devices:
+ *
+ * Intel VT-d hardware uses paging for DMA remapping. The minimum mapped
+ * window is a page size. The device drivers may map buffers not filling
+ * whole IOMMU window. This allows device to access to possibly unrelated
+ * memory and malicious device can exploit this to perform a DMA attack.
+ * Use a bounce page for the buffer which doesn't fill a whole IOMU page.
+ */
+
+static inline unsigned long domain_page_size(struct dmar_domain *domain)
+{
+	return 1UL << __ffs(domain->domain.pgsize_bitmap);
+}
+
+/* Calculate how many pages does a range of [addr, addr + size) cross. */
+static inline unsigned long
+range_nrpages(dma_addr_t addr, size_t size, unsigned long page_size)
+{
+	unsigned long offset = page_size - 1;
+
+	return ALIGN((addr & offset) + size, page_size) >> __ffs(page_size);
+}
+
+int domain_walk_addr_range(const struct addr_walk *walk,
+			   struct dmar_domain *domain,
+			   dma_addr_t addr, phys_addr_t paddr,
+			   size_t size, struct bounce_param *param)
+{
+	u64 page_size = domain_page_size(domain);
+	u64 page_offset = page_size - 1;
+	u64 page_mask = ~page_offset;
+	u64 length = 0;
+	int ret;
+
+	/*
+	 * The first vt-d page is partial. Use bounce buffer for
+	 * security concern if necessary.
+	 */
+	if (addr & page_offset) {
+		length = ALIGN(addr, page_size) - addr;
+		if (length > size)
+			length = size;
+		ret = walk->low(domain, addr, paddr, length, param);
+		if (ret)
+			return ret;
+
+		/* The buffer only covers on page. */
+		if (range_nrpages(addr, size, page_size) <= 1)
+			return 0;
+
+		size -= length;
+		addr = ALIGN(addr, page_size);
+		paddr = ALIGN(paddr, page_size);
+	}
+
+	/*
+	 * There might be several pages which could totally accessed
+	 * by a device in the middle. It's unnecessary to use bounce
+	 * buffer against these pages.
+	 */
+	if (size & page_mask) {
+		length = size & page_mask;
+		ret = walk->middle(domain, addr, paddr, length, param);
+		if (ret)
+			return ret;
+
+		addr += size & page_mask;
+		paddr += size & page_mask;
+		size &= page_offset;
+	}
+
+	/*
+	 * Okay, last page might be partial. Use bounce buffer if necessary.
+	 */
+	if (size)
+		return walk->high(domain, addr, paddr, size, param);
+
+	return 0;
+}
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 74afedfe193b..f74aed6ecc33 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -668,6 +668,12 @@ int domain_iomap_range(struct dmar_domain *domain, unsigned long addr,
 struct page *domain_iounmap_range(struct dmar_domain *domain,
 				  unsigned long addr, size_t size);
 
+struct bounce_param {
+	int			prot;
+	enum dma_data_direction	dir;
+	struct page		**freelist;
+};
+
 #ifdef CONFIG_INTEL_IOMMU_SVM
 int intel_svm_init(struct intel_iommu *iommu);
 extern int intel_svm_enable_prq(struct intel_iommu *iommu);
-- 
2.17.1


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

* [PATCH v1 4/9] iommu/vt-d: Add bounce buffer API for domain map/unmap
  2019-03-12  5:59 [PATCH v1 0/9] Bounce buffer for untrusted devices Lu Baolu
                   ` (2 preceding siblings ...)
  2019-03-12  5:59 ` [PATCH v1 3/9] iommu/vt-d: Add address walk helper Lu Baolu
@ 2019-03-12  6:00 ` Lu Baolu
  2019-03-12 16:38   ` Christoph Hellwig
  2019-03-12  6:00 ` [PATCH v1 5/9] iommu/vt-d: Add bounce buffer API for dma sync Lu Baolu
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Lu Baolu @ 2019-03-12  6:00 UTC (permalink / raw)
  To: David Woodhouse, Joerg Roedel, ashok.raj, jacob.jun.pan,
	alan.cox, kevin.tian, mika.westerberg, pengfei.xu
  Cc: iommu, linux-kernel, Lu Baolu, Jacob Pan

This adds the APIs for bounce buffer specified domain
map() and unmap(). The start and end partial pages will
be mapped with bounce buffered pages instead. This will
enhance the security of DMA buffer by isolating the DMA
attacks from malicious devices.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Tested-by: Xu Pengfei <pengfei.xu@intel.com>
Tested-by: Mika Westerberg <mika.westerberg@intel.com>
---
 drivers/iommu/intel-iommu.c   |   3 +
 drivers/iommu/intel-pgtable.c | 305 +++++++++++++++++++++++++++++++++-
 include/linux/intel-iommu.h   |   7 +
 3 files changed, 311 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 791261afb4a9..305731ec142e 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1724,6 +1724,7 @@ static struct dmar_domain *alloc_domain(int flags)
 	domain->flags = flags;
 	domain->has_iotlb_device = false;
 	INIT_LIST_HEAD(&domain->devices);
+	idr_init(&domain->bounce_idr);
 
 	return domain;
 }
@@ -1919,6 +1920,8 @@ static void domain_exit(struct dmar_domain *domain)
 
 	dma_free_pagelist(freelist);
 
+	idr_destroy(&domain->bounce_idr);
+
 	free_domain_mem(domain);
 }
 
diff --git a/drivers/iommu/intel-pgtable.c b/drivers/iommu/intel-pgtable.c
index ad3347d7ac1d..e8317982c5ab 100644
--- a/drivers/iommu/intel-pgtable.c
+++ b/drivers/iommu/intel-pgtable.c
@@ -15,6 +15,8 @@
 #include <linux/iommu.h>
 #include <trace/events/intel_iommu.h>
 
+#define	MAX_BOUNCE_LIST_ENTRIES		32
+
 struct addr_walk {
 	int (*low)(struct dmar_domain *domain, dma_addr_t addr,
 			phys_addr_t paddr, size_t size,
@@ -27,6 +29,13 @@ struct addr_walk {
 			struct bounce_param *param);
 };
 
+struct bounce_cookie {
+	struct page		*bounce_page;
+	phys_addr_t		original_phys;
+	phys_addr_t		bounce_phys;
+	struct list_head	list;
+};
+
 /*
  * Bounce buffer support for external devices:
  *
@@ -42,6 +51,14 @@ static inline unsigned long domain_page_size(struct dmar_domain *domain)
 	return 1UL << __ffs(domain->domain.pgsize_bitmap);
 }
 
+/*
+ * Bounce buffer cookie lazy allocation. A list to keep the unused
+ * bounce buffer cookies with a spin lock to protect the access.
+ */
+static LIST_HEAD(bounce_list);
+static DEFINE_SPINLOCK(bounce_lock);
+static int bounce_list_entries;
+
 /* Calculate how many pages does a range of [addr, addr + size) cross. */
 static inline unsigned long
 range_nrpages(dma_addr_t addr, size_t size, unsigned long page_size)
@@ -51,10 +68,274 @@ range_nrpages(dma_addr_t addr, size_t size, unsigned long page_size)
 	return ALIGN((addr & offset) + size, page_size) >> __ffs(page_size);
 }
 
-int domain_walk_addr_range(const struct addr_walk *walk,
-			   struct dmar_domain *domain,
-			   dma_addr_t addr, phys_addr_t paddr,
-			   size_t size, struct bounce_param *param)
+static int nobounce_map_middle(struct dmar_domain *domain, dma_addr_t addr,
+			       phys_addr_t paddr, size_t size,
+			       struct bounce_param *param)
+{
+	return domain_iomap_range(domain, addr, paddr, size, param->prot);
+}
+
+static int nobounce_unmap_middle(struct dmar_domain *domain, dma_addr_t addr,
+				 phys_addr_t paddr, size_t size,
+				 struct bounce_param *param)
+{
+	struct page **freelist = param->freelist, *new;
+
+	new = domain_iounmap_range(domain, addr, size);
+	if (new) {
+		new->freelist = *freelist;
+		*freelist = new;
+	}
+
+	return 0;
+}
+
+static inline void free_bounce_cookie(struct bounce_cookie *cookie)
+{
+	if (!cookie)
+		return;
+
+	free_page((unsigned long)page_address(cookie->bounce_page));
+	kfree(cookie);
+}
+
+static struct bounce_cookie *
+domain_get_bounce_buffer(struct dmar_domain *domain, unsigned long iova_pfn)
+{
+	struct bounce_cookie *cookie;
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&bounce_lock, flags);
+	cookie = idr_find(&domain->bounce_idr, iova_pfn);
+	if (WARN_ON(cookie)) {
+		spin_unlock_irqrestore(&bounce_lock, flags);
+		pr_warn("bounce cookie for iova_pfn 0x%lx exists\n", iova_pfn);
+
+		return NULL;
+	}
+
+	/* Check the bounce list. */
+	cookie = list_first_entry_or_null(&bounce_list,
+					  struct bounce_cookie, list);
+	if (cookie) {
+		list_del_init(&cookie->list);
+		bounce_list_entries--;
+		spin_unlock_irqrestore(&bounce_lock, flags);
+		goto skip_alloc;
+	}
+	spin_unlock_irqrestore(&bounce_lock, flags);
+
+	/* We have to allocate a new cookie. */
+	cookie = kzalloc(sizeof(*cookie), GFP_ATOMIC);
+	if (!cookie)
+		return NULL;
+
+	cookie->bounce_page = alloc_pages_node(domain->nid,
+					       GFP_ATOMIC | __GFP_ZERO, 0);
+	if (!cookie->bounce_page) {
+		kfree(cookie);
+		return NULL;
+	}
+
+skip_alloc:
+	/* Map the cookie with the iova pfn. */
+	spin_lock_irqsave(&bounce_lock, flags);
+	ret = idr_alloc(&domain->bounce_idr, cookie, iova_pfn,
+			iova_pfn + 1, GFP_ATOMIC);
+	spin_unlock_irqrestore(&bounce_lock, flags);
+	if (ret < 0) {
+		free_bounce_cookie(cookie);
+		pr_warn("failed to reserve idr for iova_pfn 0x%lx\n", iova_pfn);
+
+		return NULL;
+	}
+
+	return cookie;
+}
+
+static void
+domain_put_bounce_buffer(struct dmar_domain *domain, unsigned long iova_pfn)
+{
+	struct bounce_cookie *cookie;
+	unsigned long flags;
+
+	spin_lock_irqsave(&bounce_lock, flags);
+	cookie = idr_remove(&domain->bounce_idr, iova_pfn);
+	if (!cookie) {
+		spin_unlock_irqrestore(&bounce_lock, flags);
+		pr_warn("no idr for iova_pfn 0x%lx\n", iova_pfn);
+
+		return;
+	}
+
+	if (bounce_list_entries >= MAX_BOUNCE_LIST_ENTRIES) {
+		spin_unlock_irqrestore(&bounce_lock, flags);
+		free_bounce_cookie(cookie);
+
+		return;
+	}
+	list_add_tail(&cookie->list, &bounce_list);
+	bounce_list_entries++;
+	spin_unlock_irqrestore(&bounce_lock, flags);
+}
+
+static inline int
+bounce_sync(phys_addr_t orig_addr, phys_addr_t bounce_addr,
+	    size_t size, enum dma_data_direction dir)
+{
+	unsigned long pfn = PFN_DOWN(orig_addr);
+	unsigned char *vaddr = phys_to_virt(bounce_addr);
+
+	if (PageHighMem(pfn_to_page(pfn))) {
+		/* The buffer does not have a mapping. Map it in and copy */
+		unsigned int offset = offset_in_page(orig_addr);
+		unsigned int sz = 0;
+		unsigned long flags;
+		char *buffer;
+
+		while (size) {
+			sz = min_t(size_t, PAGE_SIZE - offset, size);
+
+			local_irq_save(flags);
+			buffer = kmap_atomic(pfn_to_page(pfn));
+			if (dir == DMA_TO_DEVICE)
+				memcpy(vaddr, buffer + offset, sz);
+			else
+				memcpy(buffer + offset, vaddr, sz);
+			kunmap_atomic(buffer);
+			local_irq_restore(flags);
+
+			size -= sz;
+			pfn++;
+			vaddr += sz;
+			offset = 0;
+		}
+	} else if (dir == DMA_TO_DEVICE) {
+		memcpy(vaddr, phys_to_virt(orig_addr), size);
+	} else {
+		memcpy(phys_to_virt(orig_addr), vaddr, size);
+	}
+
+	return 0;
+}
+
+static int
+bounce_sync_for_cpu(phys_addr_t orig_addr, phys_addr_t bounce_addr, size_t size)
+{
+	return bounce_sync(orig_addr, bounce_addr, size, DMA_FROM_DEVICE);
+}
+
+static int
+bounce_sync_for_dev(phys_addr_t orig_addr, phys_addr_t bounce_addr, size_t size)
+{
+	return bounce_sync(orig_addr, bounce_addr, size, DMA_TO_DEVICE);
+}
+
+static int bounce_map(struct dmar_domain *domain, dma_addr_t addr,
+		      phys_addr_t paddr, size_t size,
+		      struct bounce_param *param)
+{
+	unsigned long page_size = domain_page_size(domain);
+	enum dma_data_direction dir = param->dir;
+	struct bounce_cookie *cookie;
+	phys_addr_t bounce_addr;
+	int prot = param->prot;
+	unsigned long offset;
+	int ret = 0;
+
+	offset = addr & (page_size - 1);
+	cookie = domain_get_bounce_buffer(domain, addr >> PAGE_SHIFT);
+	if (!cookie)
+		return -ENOMEM;
+
+	bounce_addr = page_to_phys(cookie->bounce_page) + offset;
+	cookie->original_phys = paddr;
+	cookie->bounce_phys = bounce_addr;
+	if (dir == DMA_BIDIRECTIONAL || dir == DMA_TO_DEVICE) {
+		ret = bounce_sync_for_dev(paddr, bounce_addr, size);
+		if (ret)
+			return ret;
+	}
+
+	return domain_iomap_range(domain, addr, bounce_addr, size, prot);
+}
+
+static int bounce_map_low(struct dmar_domain *domain, dma_addr_t addr,
+			  phys_addr_t paddr, size_t size,
+			  struct bounce_param *param)
+{
+	return bounce_map(domain, addr, paddr, size, param);
+}
+
+static int bounce_map_high(struct dmar_domain *domain, dma_addr_t addr,
+			   phys_addr_t paddr, size_t size,
+			   struct bounce_param *param)
+{
+	return bounce_map(domain, addr, paddr, size, param);
+}
+
+static const struct addr_walk walk_bounce_map = {
+	.low = bounce_map_low,
+	.middle = nobounce_map_middle,
+	.high = bounce_map_high,
+};
+
+static int bounce_unmap(struct dmar_domain *domain, dma_addr_t addr,
+			phys_addr_t paddr, size_t size,
+			struct bounce_param *param)
+{
+	struct page **freelist = param->freelist, *new;
+	enum dma_data_direction dir = param->dir;
+	struct bounce_cookie *cookie;
+	unsigned long flags;
+
+	spin_lock_irqsave(&bounce_lock, flags);
+	cookie = idr_find(&domain->bounce_idr, addr >> PAGE_SHIFT);
+	spin_unlock_irqrestore(&bounce_lock, flags);
+	if (WARN_ON(!cookie))
+		return -ENODEV;
+
+	new = domain_iounmap_range(domain, addr, size);
+	if (new) {
+		new->freelist = *freelist;
+		*freelist = new;
+	}
+
+	if (dir == DMA_BIDIRECTIONAL || dir == DMA_FROM_DEVICE)
+		bounce_sync_for_cpu(cookie->original_phys,
+				    cookie->bounce_phys, size);
+
+	domain_put_bounce_buffer(domain, addr >> PAGE_SHIFT);
+
+	return 0;
+}
+
+static int bounce_unmap_low(struct dmar_domain *domain, dma_addr_t addr,
+			    phys_addr_t paddr, size_t size,
+			    struct bounce_param *param)
+{
+	return bounce_unmap(domain, addr, paddr, size, param);
+}
+
+static int bounce_unmap_high(struct dmar_domain *domain, dma_addr_t addr,
+			     phys_addr_t paddr, size_t size,
+			     struct bounce_param *param)
+{
+	return bounce_unmap(domain, addr, paddr, size, param);
+}
+
+static const struct addr_walk walk_bounce_unmap = {
+	.low = bounce_unmap_low,
+	.middle = nobounce_unmap_middle,
+	.high = bounce_unmap_high,
+};
+
+static int
+domain_walk_addr_range(const struct addr_walk *walk,
+		       struct dmar_domain *domain,
+		       dma_addr_t addr, phys_addr_t paddr,
+		       size_t size, struct bounce_param *param)
 {
 	u64 page_size = domain_page_size(domain);
 	u64 page_offset = page_size - 1;
@@ -107,3 +388,19 @@ int domain_walk_addr_range(const struct addr_walk *walk,
 
 	return 0;
 }
+
+int
+domain_bounce_map(struct dmar_domain *domain, dma_addr_t addr,
+		  phys_addr_t paddr, size_t size, struct bounce_param *param)
+{
+	return domain_walk_addr_range(&walk_bounce_map, domain,
+				      addr, paddr, size, param);
+}
+
+int
+domain_bounce_unmap(struct dmar_domain *domain, dma_addr_t addr,
+		    phys_addr_t paddr, size_t size, struct bounce_param *param)
+{
+	return domain_walk_addr_range(&walk_bounce_unmap, domain,
+				      addr, paddr, size, param);
+}
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index f74aed6ecc33..8b5ba91ab606 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -498,6 +498,7 @@ struct dmar_domain {
 
 	struct dma_pte	*pgd;		/* virtual address */
 	int		gaw;		/* max guest address width */
+	struct idr	bounce_idr;	/* IDR for iova_pfn to bounce page */
 
 	/* adjusted guest address width, 0 is level 2 30-bit */
 	int		agaw;
@@ -674,6 +675,12 @@ struct bounce_param {
 	struct page		**freelist;
 };
 
+int domain_bounce_map(struct dmar_domain *domain, dma_addr_t addr,
+		      phys_addr_t paddr, size_t size,
+		      struct bounce_param *param);
+int domain_bounce_unmap(struct dmar_domain *domain, dma_addr_t addr,
+			phys_addr_t paddr, size_t size,
+			struct bounce_param *param);
 #ifdef CONFIG_INTEL_IOMMU_SVM
 int intel_svm_init(struct intel_iommu *iommu);
 extern int intel_svm_enable_prq(struct intel_iommu *iommu);
-- 
2.17.1


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

* [PATCH v1 5/9] iommu/vt-d: Add bounce buffer API for dma sync
  2019-03-12  5:59 [PATCH v1 0/9] Bounce buffer for untrusted devices Lu Baolu
                   ` (3 preceding siblings ...)
  2019-03-12  6:00 ` [PATCH v1 4/9] iommu/vt-d: Add bounce buffer API for domain map/unmap Lu Baolu
@ 2019-03-12  6:00 ` Lu Baolu
  2019-03-12  6:00 ` [PATCH v1 6/9] iommu/vt-d: Check whether device requires bounce buffer Lu Baolu
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Lu Baolu @ 2019-03-12  6:00 UTC (permalink / raw)
  To: David Woodhouse, Joerg Roedel, ashok.raj, jacob.jun.pan,
	alan.cox, kevin.tian, mika.westerberg, pengfei.xu
  Cc: iommu, linux-kernel, Lu Baolu, Jacob Pan

This adds the APIs for bounce buffer specified dma sync
ops.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Tested-by: Xu Pengfei <pengfei.xu@intel.com>
Tested-by: Mika Westerberg <mika.westerberg@intel.com>
---
 drivers/iommu/intel-pgtable.c | 112 ++++++++++++++++++++++++++++++++++
 include/linux/intel-iommu.h   |   6 ++
 2 files changed, 118 insertions(+)

diff --git a/drivers/iommu/intel-pgtable.c b/drivers/iommu/intel-pgtable.c
index e8317982c5ab..d175045fe236 100644
--- a/drivers/iommu/intel-pgtable.c
+++ b/drivers/iommu/intel-pgtable.c
@@ -331,6 +331,100 @@ static const struct addr_walk walk_bounce_unmap = {
 	.high = bounce_unmap_high,
 };
 
+static int
+bounce_sync_iova_pfn(struct dmar_domain *domain, dma_addr_t addr,
+		     size_t size, struct bounce_param *param,
+		     enum dma_data_direction dir)
+{
+	struct bounce_cookie *cookie;
+	unsigned long flags;
+
+	spin_lock_irqsave(&bounce_lock, flags);
+	cookie = idr_find(&domain->bounce_idr, addr >> PAGE_SHIFT);
+	spin_unlock_irqrestore(&bounce_lock, flags);
+	if (!cookie)
+		return 0;
+
+	return bounce_sync(cookie->original_phys, cookie->bounce_phys,
+			   size, dir);
+}
+
+static int
+bounce_sync_for_device_low(struct dmar_domain *domain, dma_addr_t addr,
+			   phys_addr_t paddr, size_t size,
+			   struct bounce_param *param)
+{
+	if (param->dir == DMA_BIDIRECTIONAL || param->dir == DMA_TO_DEVICE)
+		return bounce_sync_iova_pfn(domain, addr, size,
+					    param, DMA_TO_DEVICE);
+
+	return 0;
+}
+
+static int
+bounce_sync_for_device_middle(struct dmar_domain *domain, dma_addr_t addr,
+			      phys_addr_t paddr, size_t size,
+			      struct bounce_param *param)
+{
+	return 0;
+}
+
+static int
+bounce_sync_for_device_high(struct dmar_domain *domain, dma_addr_t addr,
+			    phys_addr_t paddr, size_t size,
+			    struct bounce_param *param)
+{
+	if (param->dir == DMA_BIDIRECTIONAL || param->dir == DMA_TO_DEVICE)
+		return bounce_sync_iova_pfn(domain, addr, size,
+					    param, DMA_TO_DEVICE);
+
+	return 0;
+}
+
+const struct addr_walk walk_bounce_sync_for_device = {
+	.low = bounce_sync_for_device_low,
+	.middle = bounce_sync_for_device_middle,
+	.high = bounce_sync_for_device_high,
+};
+
+static int
+bounce_sync_for_cpu_low(struct dmar_domain *domain, dma_addr_t addr,
+			phys_addr_t paddr, size_t size,
+			struct bounce_param *param)
+{
+	if (param->dir == DMA_BIDIRECTIONAL || param->dir == DMA_FROM_DEVICE)
+		return bounce_sync_iova_pfn(domain, addr, size,
+					    param, DMA_FROM_DEVICE);
+
+	return 0;
+}
+
+static int
+bounce_sync_for_cpu_middle(struct dmar_domain *domain, dma_addr_t addr,
+			   phys_addr_t paddr, size_t size,
+			   struct bounce_param *param)
+{
+	return 0;
+}
+
+static int
+bounce_sync_for_cpu_high(struct dmar_domain *domain, dma_addr_t addr,
+			 phys_addr_t paddr, size_t size,
+			 struct bounce_param *param)
+{
+	if (param->dir == DMA_BIDIRECTIONAL || param->dir == DMA_FROM_DEVICE)
+		return bounce_sync_iova_pfn(domain, addr, size,
+					    param, DMA_FROM_DEVICE);
+
+	return 0;
+}
+
+const struct addr_walk walk_bounce_sync_for_cpu = {
+	.low = bounce_sync_for_cpu_low,
+	.middle = bounce_sync_for_cpu_middle,
+	.high = bounce_sync_for_cpu_high,
+};
+
 static int
 domain_walk_addr_range(const struct addr_walk *walk,
 		       struct dmar_domain *domain,
@@ -404,3 +498,21 @@ domain_bounce_unmap(struct dmar_domain *domain, dma_addr_t addr,
 	return domain_walk_addr_range(&walk_bounce_unmap, domain,
 				      addr, paddr, size, param);
 }
+
+int
+domain_bounce_sync_for_device(struct dmar_domain *domain, dma_addr_t addr,
+			      phys_addr_t paddr, size_t size,
+			      struct bounce_param *param)
+{
+	return domain_walk_addr_range(&walk_bounce_sync_for_device, domain,
+				      addr, paddr, size, param);
+}
+
+int
+domain_bounce_sync_for_cpu(struct dmar_domain *domain, dma_addr_t addr,
+			   phys_addr_t paddr, size_t size,
+			   struct bounce_param *param)
+{
+	return domain_walk_addr_range(&walk_bounce_sync_for_cpu, domain,
+				      addr, paddr, size, param);
+}
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 8b5ba91ab606..f4f313df7249 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -681,6 +681,12 @@ int domain_bounce_map(struct dmar_domain *domain, dma_addr_t addr,
 int domain_bounce_unmap(struct dmar_domain *domain, dma_addr_t addr,
 			phys_addr_t paddr, size_t size,
 			struct bounce_param *param);
+int domain_bounce_sync_for_device(struct dmar_domain *domain, dma_addr_t addr,
+				  phys_addr_t paddr, size_t size,
+				  struct bounce_param *param);
+int domain_bounce_sync_for_cpu(struct dmar_domain *domain, dma_addr_t addr,
+			       phys_addr_t paddr, size_t size,
+			       struct bounce_param *param);
 #ifdef CONFIG_INTEL_IOMMU_SVM
 int intel_svm_init(struct intel_iommu *iommu);
 extern int intel_svm_enable_prq(struct intel_iommu *iommu);
-- 
2.17.1


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

* [PATCH v1 6/9] iommu/vt-d: Check whether device requires bounce buffer
  2019-03-12  5:59 [PATCH v1 0/9] Bounce buffer for untrusted devices Lu Baolu
                   ` (4 preceding siblings ...)
  2019-03-12  6:00 ` [PATCH v1 5/9] iommu/vt-d: Add bounce buffer API for dma sync Lu Baolu
@ 2019-03-12  6:00 ` Lu Baolu
  2019-03-12  6:00 ` [PATCH v1 7/9] iommu/vt-d: Add dma sync ops for untrusted devices Lu Baolu
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Lu Baolu @ 2019-03-12  6:00 UTC (permalink / raw)
  To: David Woodhouse, Joerg Roedel, ashok.raj, jacob.jun.pan,
	alan.cox, kevin.tian, mika.westerberg, pengfei.xu
  Cc: iommu, linux-kernel, Lu Baolu, Jacob Pan

This adds a helper to check whether a device needs to
use bounce buffer. It also provides a boot time option
to disable the bounce buffer. Users can use this to
prevent the iommu driver from using the bounce buffer
for performance gain.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Tested-by: Xu Pengfei <pengfei.xu@intel.com>
Tested-by: Mika Westerberg <mika.westerberg@intel.com>
---
 Documentation/admin-guide/kernel-parameters.txt |  5 +++++
 drivers/iommu/intel-iommu.c                     | 17 +++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 2b8ee90bb644..86880eb3fc73 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1726,6 +1726,11 @@
 			Note that using this option lowers the security
 			provided by tboot because it makes the system
 			vulnerable to DMA attacks.
+		nobounce [Default off]
+			Do not use the bounce buffer for untrusted devices like
+			the Thunderbolt devices. This will treat the untrusted
+			devices as the trusted ones, hence might expose security
+			risks of DMA attacks.
 
 	intel_idle.max_cstate=	[KNL,HW,ACPI,X86]
 			0	disables intel_idle and fall back on acpi_idle.
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 305731ec142e..cc7609a17d6a 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -366,6 +366,7 @@ static int intel_iommu_strict;
 static int intel_iommu_superpage = 1;
 static int intel_iommu_sm;
 static int iommu_identity_mapping;
+static int intel_no_bounce;
 
 #define IDENTMAP_ALL		1
 #define IDENTMAP_GFX		2
@@ -382,6 +383,19 @@ EXPORT_SYMBOL_GPL(intel_iommu_gfx_mapped);
 static DEFINE_SPINLOCK(device_domain_lock);
 static LIST_HEAD(device_domain_list);
 
+static inline bool device_needs_bounce(struct device *dev)
+{
+	struct pci_dev *pdev = NULL;
+
+	if (intel_no_bounce)
+		return false;
+
+	if (dev_is_pci(dev))
+		pdev = to_pci_dev(dev);
+
+	return pdev ? pdev->untrusted : false;
+}
+
 /*
  * Iterate over elements in device_domain_list and call the specified
  * callback @fn against each element.
@@ -464,6 +478,9 @@ static int __init intel_iommu_setup(char *str)
 			printk(KERN_INFO
 				"Intel-IOMMU: not forcing on after tboot. This could expose security risk for tboot\n");
 			intel_iommu_tboot_noforce = 1;
+		} else if (!strncmp(str, "nobounce", 8)) {
+			pr_info("Intel-IOMMU: No bounce buffer. This could expose security risks of DMA attacks\n");
+			intel_no_bounce = 1;
 		}
 
 		str += strcspn(str, ",");
-- 
2.17.1


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

* [PATCH v1 7/9] iommu/vt-d: Add dma sync ops for untrusted devices
  2019-03-12  5:59 [PATCH v1 0/9] Bounce buffer for untrusted devices Lu Baolu
                   ` (5 preceding siblings ...)
  2019-03-12  6:00 ` [PATCH v1 6/9] iommu/vt-d: Check whether device requires bounce buffer Lu Baolu
@ 2019-03-12  6:00 ` Lu Baolu
  2019-03-12  6:00 ` [PATCH v1 8/9] iommu/vt-d: Flush IOTLB for untrusted device in time Lu Baolu
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Lu Baolu @ 2019-03-12  6:00 UTC (permalink / raw)
  To: David Woodhouse, Joerg Roedel, ashok.raj, jacob.jun.pan,
	alan.cox, kevin.tian, mika.westerberg, pengfei.xu
  Cc: iommu, linux-kernel, Lu Baolu, Jacob Pan

This adds the dma sync ops for dma buffers used by any
untrusted device. We need to sync such buffers because
they might have been mapped with bounce pages.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Tested-by: Xu Pengfei <pengfei.xu@intel.com>
Tested-by: Mika Westerberg <mika.westerberg@intel.com>
---
 drivers/iommu/intel-iommu.c | 154 +++++++++++++++++++++++++++++++++---
 1 file changed, 145 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index cc7609a17d6a..36909f8e7788 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3940,16 +3940,152 @@ static int intel_map_sg(struct device *dev, struct scatterlist *sglist, int nele
 	return nelems;
 }
 
+static void
+sync_dma_for_device(struct device *dev, dma_addr_t dev_addr, size_t size,
+		    enum dma_data_direction dir)
+{
+	struct dmar_domain *domain;
+	struct bounce_param param;
+
+	domain = find_domain(dev);
+	if (WARN_ON(!domain))
+		return;
+
+	memset(&param, 0, sizeof(param));
+	param.dir = dir;
+	if (dir == DMA_BIDIRECTIONAL || dir == DMA_TO_DEVICE)
+		domain_bounce_sync_for_device(domain, dev_addr,
+					      0, size, &param);
+}
+
+static void
+sync_dma_for_cpu(struct device *dev, dma_addr_t dev_addr, size_t size,
+		 enum dma_data_direction dir)
+{
+	struct dmar_domain *domain;
+	struct bounce_param param;
+
+	domain = find_domain(dev);
+	if (WARN_ON(!domain))
+		return;
+
+	memset(&param, 0, sizeof(param));
+	param.dir = dir;
+	if (dir == DMA_BIDIRECTIONAL || dir == DMA_FROM_DEVICE)
+		domain_bounce_sync_for_cpu(domain, dev_addr,
+					   0, size, &param);
+}
+
+static void
+intel_sync_single_for_cpu(struct device *dev, dma_addr_t addr,
+			  size_t size, enum dma_data_direction dir)
+{
+	struct dmar_domain *domain;
+
+	if (WARN_ON(dir == DMA_NONE))
+		return;
+
+	if (!device_needs_bounce(dev))
+		return;
+
+	if (iommu_no_mapping(dev))
+		return;
+
+	domain = get_valid_domain_for_dev(dev);
+	if (!domain)
+		return;
+
+	sync_dma_for_cpu(dev, addr, size, dir);
+}
+
+static void
+intel_sync_single_for_device(struct device *dev, dma_addr_t addr,
+			     size_t size, enum dma_data_direction dir)
+{
+	struct dmar_domain *domain;
+
+	if (WARN_ON(dir == DMA_NONE))
+		return;
+
+	if (!device_needs_bounce(dev))
+		return;
+
+	if (iommu_no_mapping(dev))
+		return;
+
+	domain = get_valid_domain_for_dev(dev);
+	if (!domain)
+		return;
+
+	sync_dma_for_device(dev, addr, size, dir);
+}
+
+static void
+intel_sync_sg_for_cpu(struct device *dev, struct scatterlist *sglist,
+		      int nelems, enum dma_data_direction dir)
+{
+	struct dmar_domain *domain;
+	struct scatterlist *sg;
+	int i;
+
+	if (WARN_ON(dir == DMA_NONE))
+		return;
+
+	if (!device_needs_bounce(dev))
+		return;
+
+	if (iommu_no_mapping(dev))
+		return;
+
+	domain = get_valid_domain_for_dev(dev);
+	if (!domain)
+		return;
+
+	for_each_sg(sglist, sg, nelems, i)
+		sync_dma_for_cpu(dev, sg_dma_address(sg),
+				 sg_dma_len(sg), dir);
+}
+
+static void
+intel_sync_sg_for_device(struct device *dev, struct scatterlist *sglist,
+			 int nelems, enum dma_data_direction dir)
+{
+	struct dmar_domain *domain;
+	struct scatterlist *sg;
+	int i;
+
+	if (WARN_ON(dir == DMA_NONE))
+		return;
+
+	if (!device_needs_bounce(dev))
+		return;
+
+	if (iommu_no_mapping(dev))
+		return;
+
+	domain = get_valid_domain_for_dev(dev);
+	if (!domain)
+		return;
+
+	for_each_sg(sglist, sg, nelems, i)
+		sync_dma_for_device(dev, sg_dma_address(sg),
+				    sg_dma_len(sg), dir);
+}
+
 static const struct dma_map_ops intel_dma_ops = {
-	.alloc = intel_alloc_coherent,
-	.free = intel_free_coherent,
-	.map_sg = intel_map_sg,
-	.unmap_sg = intel_unmap_sg,
-	.map_page = intel_map_page,
-	.unmap_page = intel_unmap_page,
-	.map_resource = intel_map_resource,
-	.unmap_resource = intel_unmap_page,
-	.dma_supported = dma_direct_supported,
+	.alloc			= intel_alloc_coherent,
+	.free			= intel_free_coherent,
+	.map_sg			= intel_map_sg,
+	.unmap_sg		= intel_unmap_sg,
+	.map_page		= intel_map_page,
+	.unmap_page		= intel_unmap_page,
+	.sync_single_for_cpu	= intel_sync_single_for_cpu,
+	.sync_single_for_device	= intel_sync_single_for_device,
+	.sync_sg_for_cpu	= intel_sync_sg_for_cpu,
+	.sync_sg_for_device	= intel_sync_sg_for_device,
+	.map_resource		= intel_map_resource,
+	.unmap_resource		= intel_unmap_page,
+	.dma_supported		= dma_direct_supported,
 };
 
 static inline int iommu_domain_cache_init(void)
-- 
2.17.1


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

* [PATCH v1 8/9] iommu/vt-d: Flush IOTLB for untrusted device in time
  2019-03-12  5:59 [PATCH v1 0/9] Bounce buffer for untrusted devices Lu Baolu
                   ` (6 preceding siblings ...)
  2019-03-12  6:00 ` [PATCH v1 7/9] iommu/vt-d: Add dma sync ops for untrusted devices Lu Baolu
@ 2019-03-12  6:00 ` Lu Baolu
  2019-03-12  6:00 ` [PATCH v1 9/9] iommu/vt-d: Use bounce buffer for untrusted devices Lu Baolu
  2019-03-12  6:07 ` [PATCH v1 0/9] Bounce " Lu Baolu
  9 siblings, 0 replies; 18+ messages in thread
From: Lu Baolu @ 2019-03-12  6:00 UTC (permalink / raw)
  To: David Woodhouse, Joerg Roedel, ashok.raj, jacob.jun.pan,
	alan.cox, kevin.tian, mika.westerberg, pengfei.xu
  Cc: iommu, linux-kernel, Lu Baolu, Jacob Pan

By default, for performance consideration, Intel IOMMU
driver won't flush IOTLB immediately after a buffer is
unmapped. It schedules a thread and flushes IOTLB in a
batched mode. This isn't suitable for untrusted device
since it still can access the memory even if it isn't
supposed to do so.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Tested-by: Xu Pengfei <pengfei.xu@intel.com>
Tested-by: Mika Westerberg <mika.westerberg@intel.com>
---
 drivers/iommu/intel-iommu.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 36909f8e7788..4f2fdd68658c 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3749,10 +3749,14 @@ static void intel_unmap(struct device *dev, dma_addr_t dev_addr, size_t size)
 	unsigned long iova_pfn;
 	struct intel_iommu *iommu;
 	struct page *freelist;
+	struct pci_dev *pdev = NULL;
 
 	if (iommu_no_mapping(dev))
 		return;
 
+	if (dev_is_pci(dev))
+		pdev = to_pci_dev(dev);
+
 	domain = find_domain(dev);
 	BUG_ON(!domain);
 
@@ -3768,7 +3772,7 @@ static void intel_unmap(struct device *dev, dma_addr_t dev_addr, size_t size)
 
 	freelist = domain_unmap(domain, start_pfn, last_pfn);
 
-	if (intel_iommu_strict) {
+	if (intel_iommu_strict || (pdev && pdev->untrusted)) {
 		iommu_flush_iotlb_psi(iommu, domain, start_pfn,
 				      nrpages, !freelist, 0);
 		/* free iova */
-- 
2.17.1


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

* [PATCH v1 9/9] iommu/vt-d: Use bounce buffer for untrusted devices
  2019-03-12  5:59 [PATCH v1 0/9] Bounce buffer for untrusted devices Lu Baolu
                   ` (7 preceding siblings ...)
  2019-03-12  6:00 ` [PATCH v1 8/9] iommu/vt-d: Flush IOTLB for untrusted device in time Lu Baolu
@ 2019-03-12  6:00 ` Lu Baolu
  2019-03-12  6:07 ` [PATCH v1 0/9] Bounce " Lu Baolu
  9 siblings, 0 replies; 18+ messages in thread
From: Lu Baolu @ 2019-03-12  6:00 UTC (permalink / raw)
  To: David Woodhouse, Joerg Roedel, ashok.raj, jacob.jun.pan,
	alan.cox, kevin.tian, mika.westerberg, pengfei.xu
  Cc: iommu, linux-kernel, Lu Baolu, Jacob Pan

The Intel VT-d hardware uses paging for DMA remapping.
The minimum mapped window is a page size. The device
drivers may map buffers not filling the whole IOMMU
window. This allows the device to access to possibly
unrelated memory and a malicious device could exploit
this to perform DMA attacks. To address this, the
Intel IOMMU driver will use bounce pages for those
buffers which don't fill a whole IOMMU page.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Tested-by: Xu Pengfei <pengfei.xu@intel.com>
Tested-by: Mika Westerberg <mika.westerberg@intel.com>
---
 drivers/iommu/intel-iommu.c | 151 +++++++++++++++++++++++++++---------
 1 file changed, 114 insertions(+), 37 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 4f2fdd68658c..4bdfc42c06f4 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -52,6 +52,7 @@
 #include <asm/irq_remapping.h>
 #include <asm/cacheflush.h>
 #include <asm/iommu.h>
+#include <trace/events/intel_iommu.h>
 
 #include "irq_remapping.h"
 #include "intel-pasid.h"
@@ -3670,12 +3671,14 @@ static dma_addr_t __intel_map_single(struct device *dev, phys_addr_t paddr,
 				     size_t size, int dir, u64 dma_mask)
 {
 	struct dmar_domain *domain;
-	phys_addr_t start_paddr;
+	dma_addr_t start_dma;
 	unsigned long iova_pfn;
 	int prot = 0;
 	int ret;
 	struct intel_iommu *iommu;
 	unsigned long paddr_pfn = paddr >> PAGE_SHIFT;
+	unsigned long nrpages;
+	struct bounce_param param;
 
 	BUG_ON(dir == DMA_NONE);
 
@@ -3687,9 +3690,10 @@ static dma_addr_t __intel_map_single(struct device *dev, phys_addr_t paddr,
 		return DMA_MAPPING_ERROR;
 
 	iommu = domain_get_iommu(domain);
-	size = aligned_nrpages(paddr, size);
+	nrpages = aligned_nrpages(paddr, size);
 
-	iova_pfn = intel_alloc_iova(dev, domain, dma_to_mm_pfn(size), dma_mask);
+	iova_pfn = intel_alloc_iova(dev, domain,
+				    dma_to_mm_pfn(nrpages), dma_mask);
 	if (!iova_pfn)
 		goto error;
 
@@ -3702,24 +3706,36 @@ static dma_addr_t __intel_map_single(struct device *dev, phys_addr_t paddr,
 		prot |= DMA_PTE_READ;
 	if (dir == DMA_FROM_DEVICE || dir == DMA_BIDIRECTIONAL)
 		prot |= DMA_PTE_WRITE;
+
+	start_dma = (dma_addr_t)iova_pfn << PAGE_SHIFT;
+	start_dma += offset_in_page(paddr);
+
 	/*
 	 * paddr - (paddr + size) might be partial page, we should map the whole
 	 * page.  Note: if two part of one page are separately mapped, we
 	 * might have two guest_addr mapping to the same host paddr, but this
 	 * is not a big problem
 	 */
-	ret = domain_pfn_mapping(domain, mm_to_dma_pfn(iova_pfn),
-				 mm_to_dma_pfn(paddr_pfn), size, prot);
+	memset(&param, 0, sizeof(param));
+	param.prot = prot;
+	param.dir = dir;
+	if (device_needs_bounce(dev)) {
+		ret = domain_bounce_map(domain, start_dma, paddr, size, &param);
+		if (!ret)
+			trace_bounce_map_single(dev, start_dma, paddr, size);
+	} else {
+		ret = domain_pfn_mapping(domain, mm_to_dma_pfn(iova_pfn),
+					 mm_to_dma_pfn(paddr_pfn),
+					 nrpages, prot);
+	}
 	if (ret)
 		goto error;
 
-	start_paddr = (phys_addr_t)iova_pfn << PAGE_SHIFT;
-	start_paddr += paddr & ~PAGE_MASK;
-	return start_paddr;
-
+	return start_dma;
 error:
 	if (iova_pfn)
-		free_iova_fast(&domain->iovad, iova_pfn, dma_to_mm_pfn(size));
+		free_iova_fast(&domain->iovad, iova_pfn,
+			       dma_to_mm_pfn(nrpages));
 	dev_err(dev, "Device request: %zx@%llx dir %d --- failed\n",
 		size, (unsigned long long)paddr, dir);
 	return DMA_MAPPING_ERROR;
@@ -3741,36 +3757,81 @@ static dma_addr_t intel_map_resource(struct device *dev, phys_addr_t phys_addr,
 	return __intel_map_single(dev, phys_addr, size, dir, *dev->dma_mask);
 }
 
-static void intel_unmap(struct device *dev, dma_addr_t dev_addr, size_t size)
+static void
+intel_unmap(struct device *dev, dma_addr_t dev_addr, size_t size,
+	    struct scatterlist *sglist, int nelems,
+	    enum dma_data_direction dir, unsigned long attrs)
 {
 	struct dmar_domain *domain;
 	unsigned long start_pfn, last_pfn;
-	unsigned long nrpages;
+	unsigned long nrpages = 0;
 	unsigned long iova_pfn;
 	struct intel_iommu *iommu;
-	struct page *freelist;
+	struct page *freelist = NULL;
 	struct pci_dev *pdev = NULL;
-
-	if (iommu_no_mapping(dev))
-		return;
+	struct bounce_param param;
 
 	if (dev_is_pci(dev))
 		pdev = to_pci_dev(dev);
 
+	if (iommu_no_mapping(dev))
+		return;
+
 	domain = find_domain(dev);
 	BUG_ON(!domain);
 
 	iommu = domain_get_iommu(domain);
 
-	iova_pfn = IOVA_PFN(dev_addr);
-
-	nrpages = aligned_nrpages(dev_addr, size);
-	start_pfn = mm_to_dma_pfn(iova_pfn);
-	last_pfn = start_pfn + nrpages - 1;
-
-	dev_dbg(dev, "Device unmapping: pfn %lx-%lx\n", start_pfn, last_pfn);
+	if (sglist) {
+		struct scatterlist *sg;
+		int i;
 
-	freelist = domain_unmap(domain, start_pfn, last_pfn);
+		dev_addr = sg_dma_address(sglist) & PAGE_MASK;
+		iova_pfn = IOVA_PFN(dev_addr);
+		for_each_sg(sglist, sg, nelems, i) {
+			nrpages += aligned_nrpages(sg_dma_address(sg),
+						   sg_dma_len(sg));
+		}
+		start_pfn = mm_to_dma_pfn(iova_pfn);
+		last_pfn = start_pfn + nrpages - 1;
+
+		if (device_needs_bounce(dev))
+			for_each_sg(sglist, sg, nelems, i) {
+				struct page *tmp;
+
+				tmp = NULL;
+				memset(&param, 0, sizeof(param));
+				param.freelist = &tmp;
+				param.dir = dir;
+				domain_bounce_unmap(domain, sg_dma_address(sg),
+						    sg_phys(sg), sg->length,
+						    &param);
+				if (tmp) {
+					tmp->freelist = freelist;
+					freelist = tmp;
+				}
+				trace_bounce_unmap_sg(dev, i, nelems,
+						      sg_dma_address(sg),
+						      sg_phys(sg), sg->length);
+			}
+		else
+			freelist = domain_unmap(domain, start_pfn, last_pfn);
+	} else {
+		iova_pfn = IOVA_PFN(dev_addr);
+		nrpages = aligned_nrpages(dev_addr, size);
+		start_pfn = mm_to_dma_pfn(iova_pfn);
+		last_pfn = start_pfn + nrpages - 1;
+
+		if (device_needs_bounce(dev)) {
+			memset(&param, 0, sizeof(param));
+			param.freelist = &freelist;
+			param.dir = dir;
+			domain_bounce_unmap(domain, dev_addr, 0, size, &param);
+			trace_bounce_unmap_single(dev, dev_addr, size);
+		} else {
+			freelist = domain_unmap(domain, start_pfn, last_pfn);
+		}
+	}
 
 	if (intel_iommu_strict || (pdev && pdev->untrusted)) {
 		iommu_flush_iotlb_psi(iommu, domain, start_pfn,
@@ -3792,7 +3853,7 @@ static void intel_unmap_page(struct device *dev, dma_addr_t dev_addr,
 			     size_t size, enum dma_data_direction dir,
 			     unsigned long attrs)
 {
-	intel_unmap(dev, dev_addr, size);
+	intel_unmap(dev, dev_addr, size, NULL, 0, dir, attrs);
 }
 
 static void *intel_alloc_coherent(struct device *dev, size_t size,
@@ -3852,7 +3913,7 @@ static void intel_free_coherent(struct device *dev, size_t size, void *vaddr,
 	size = PAGE_ALIGN(size);
 	order = get_order(size);
 
-	intel_unmap(dev, dma_handle, size);
+	intel_unmap(dev, dma_handle, size, NULL, 0, 0, attrs);
 	if (!dma_release_from_contiguous(dev, page, size >> PAGE_SHIFT))
 		__free_pages(page, order);
 }
@@ -3861,16 +3922,7 @@ static void intel_unmap_sg(struct device *dev, struct scatterlist *sglist,
 			   int nelems, enum dma_data_direction dir,
 			   unsigned long attrs)
 {
-	dma_addr_t startaddr = sg_dma_address(sglist) & PAGE_MASK;
-	unsigned long nrpages = 0;
-	struct scatterlist *sg;
-	int i;
-
-	for_each_sg(sglist, sg, nelems, i) {
-		nrpages += aligned_nrpages(sg_dma_address(sg), sg_dma_len(sg));
-	}
-
-	intel_unmap(dev, startaddr, nrpages << VTD_PAGE_SHIFT);
+	intel_unmap(dev, 0, 0, sglist, nelems, dir, attrs);
 }
 
 static int intel_nontranslate_map_sg(struct device *hddev,
@@ -3932,7 +3984,32 @@ static int intel_map_sg(struct device *dev, struct scatterlist *sglist, int nele
 
 	start_vpfn = mm_to_dma_pfn(iova_pfn);
 
-	ret = domain_sg_mapping(domain, start_vpfn, sglist, size, prot);
+	if (device_needs_bounce(dev)) {
+		for_each_sg(sglist, sg, nelems, i) {
+			unsigned int pgoff = offset_in_page(sg->offset);
+			struct bounce_param param;
+			dma_addr_t addr;
+
+			addr = ((dma_addr_t)iova_pfn << PAGE_SHIFT) + pgoff;
+			memset(&param, 0, sizeof(param));
+			param.prot = prot;
+			param.dir = dir;
+			ret = domain_bounce_map(domain, addr, sg_phys(sg),
+						sg->length, &param);
+			if (ret)
+				break;
+
+			trace_bounce_map_sg(dev, i, nelems, addr,
+					    sg_phys(sg), sg->length);
+
+			sg->dma_address = addr;
+			sg->dma_length = sg->length;
+			iova_pfn += aligned_nrpages(sg->offset, sg->length);
+		}
+	} else {
+		ret = domain_sg_mapping(domain, start_vpfn, sglist, size, prot);
+	}
+
 	if (unlikely(ret)) {
 		dma_pte_free_pagetable(domain, start_vpfn,
 				       start_vpfn + size - 1,
-- 
2.17.1


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

* Re: [PATCH v1 0/9] Bounce buffer for untrusted devices
  2019-03-12  5:59 [PATCH v1 0/9] Bounce buffer for untrusted devices Lu Baolu
                   ` (8 preceding siblings ...)
  2019-03-12  6:00 ` [PATCH v1 9/9] iommu/vt-d: Use bounce buffer for untrusted devices Lu Baolu
@ 2019-03-12  6:07 ` Lu Baolu
  9 siblings, 0 replies; 18+ messages in thread
From: Lu Baolu @ 2019-03-12  6:07 UTC (permalink / raw)
  To: David Woodhouse, Joerg Roedel, ashok.raj, jacob.jun.pan,
	alan.cox, kevin.tian, mika.westerberg, pengfei.xu
  Cc: baolu.lu, iommu, linux-kernel

Should be titled as "iommu/vt-d: Bounce buffer for untrusted devices".
Sorry for the inconvenience.

On 3/12/19 1:59 PM, Lu Baolu wrote:
> An external PCI device is a PCI peripheral device connected
> to the system through an external bus, such as Thunderbolt.
> What makes it different is that it can't be trusted to the
> same degree as the devices build into the system. Generally,
> a trusted PCIe device will DMA into the designated buffers
> and not overrun or otherwise write outside the specified
> bounds. But it's different for an external device. The minimum
> IOMMU mapping granularity is one page (4k), so for DMA transfers
> smaller than that a malicious PCIe device can access the whole
> page of memory even if it does not belong to the driver in
> question. This opens a possibility for DMA attack. For more
> information about DMA attacks imposed by an untrusted PCI/PCIe
> device, please refer to [2].
> 
> This implements bounce buffer for the untrusted external
> devices. The transfers should be limited in isolated pages
> so the IOMMU window does not cover memory outside of what
> the driver expects. Full pages within a buffer could be
> directly mapped in IOMMU page table, but for partial pages
> we use bounce page instead.
> 
> In addition, the IOMMU mappings cached in the IOTLB for
> untrusted devices should be invalidated immediately after
> the unmap operation. Otherwise, the IOMMU window is left
> open to attacks.
> 
> The implementation of bounce buffers for untrusted devices
> will cause a little performance overhead, but we didn't see
> any user experience problems. The users could use the kernel
> parameter of "intel_iommu=nobounce" to remove the performance
> overhead if they trust their devices enough.
> 
> The Thunderbolt vulnerabiltiies is public and has a nice
> name as Thunderclap nowadays. Please refer to [1] [3] for
> more information. This patch series aims to mitigate the
> concerns.
> 
> The bounce buffer idea:
> 
> Based-on-idea-by: Mika Westerberg <mika.westerberg@intel.com>
> Based-on-idea-by: Ashok Raj <ashok.raj@intel.com>
> Based-on-idea-by: Alan Cox <alan.cox@intel.com>
> 
> The patch series has been tested by:
> 
> Tested-by: Xu Pengfei <pengfei.xu@intel.com>
> Tested-by: Mika Westerberg <mika.westerberg@intel.com>
> 
> [1] https://thunderclap.io/
> [2] https://thunderclap.io/thunderclap-paper-ndss2019.pdf
> [3] https://christian.kellner.me/2019/02/27/thunderclap-and-linux/
> 
> Lu Baolu (9):
>    iommu/vt-d: Add trace events for domain map/unmap
>    iommu/vt-d: Add helpers for domain mapping/unmapping
>    iommu/vt-d: Add address walk helper
>    iommu/vt-d: Add bounce buffer API for domain map/unmap
>    iommu/vt-d: Add bounce buffer API for dma sync
>    iommu/vt-d: Check whether device requires bounce buffer
>    iommu/vt-d: Add dma sync ops for untrusted devices
>    iommu/vt-d: Flush IOTLB for untrusted device in time
>    iommu/vt-d: Use bounce buffer for untrusted devices
> 
>   .../admin-guide/kernel-parameters.txt         |   5 +
>   drivers/iommu/Makefile                        |   1 +
>   drivers/iommu/intel-iommu.c                   | 360 ++++++++++--
>   drivers/iommu/intel-pgtable.c                 | 518 ++++++++++++++++++
>   drivers/iommu/intel-trace.c                   |  14 +
>   include/linux/intel-iommu.h                   |  24 +
>   include/trace/events/intel_iommu.h            | 132 +++++
>   7 files changed, 1010 insertions(+), 44 deletions(-)
>   create mode 100644 drivers/iommu/intel-pgtable.c
>   create mode 100644 drivers/iommu/intel-trace.c
>   create mode 100644 include/trace/events/intel_iommu.h
> 

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

* Re: [PATCH v1 4/9] iommu/vt-d: Add bounce buffer API for domain map/unmap
  2019-03-12  6:00 ` [PATCH v1 4/9] iommu/vt-d: Add bounce buffer API for domain map/unmap Lu Baolu
@ 2019-03-12 16:38   ` Christoph Hellwig
  2019-03-13  2:04     ` Lu Baolu
  0 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2019-03-12 16:38 UTC (permalink / raw)
  To: Lu Baolu
  Cc: David Woodhouse, Joerg Roedel, ashok.raj, jacob.jun.pan,
	alan.cox, kevin.tian, mika.westerberg, pengfei.xu, iommu,
	linux-kernel, Jacob Pan

On Tue, Mar 12, 2019 at 02:00:00PM +0800, Lu Baolu wrote:
> This adds the APIs for bounce buffer specified domain
> map() and unmap(). The start and end partial pages will
> be mapped with bounce buffered pages instead. This will
> enhance the security of DMA buffer by isolating the DMA
> attacks from malicious devices.

Please reuse the swiotlb code instead of reinventing it.

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

* Re: [PATCH v1 4/9] iommu/vt-d: Add bounce buffer API for domain map/unmap
  2019-03-12 16:38   ` Christoph Hellwig
@ 2019-03-13  2:04     ` Lu Baolu
  2019-03-13  2:31       ` Lu Baolu
  0 siblings, 1 reply; 18+ messages in thread
From: Lu Baolu @ 2019-03-13  2:04 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: baolu.lu, David Woodhouse, Joerg Roedel, ashok.raj,
	jacob.jun.pan, alan.cox, kevin.tian, mika.westerberg, pengfei.xu,
	iommu, linux-kernel, Jacob Pan

Hi,

On 3/13/19 12:38 AM, Christoph Hellwig wrote:
> On Tue, Mar 12, 2019 at 02:00:00PM +0800, Lu Baolu wrote:
>> This adds the APIs for bounce buffer specified domain
>> map() and unmap(). The start and end partial pages will
>> be mapped with bounce buffered pages instead. This will
>> enhance the security of DMA buffer by isolating the DMA
>> attacks from malicious devices.
> 
> Please reuse the swiotlb code instead of reinventing it.
> 

I don't think we are doing the same thing as swiotlb here. But it's
always a good thing to reuse code if possible. I considered this when
writing this code, but I found it's hard to do. Do you mind pointing
me to the code which I could reuse?

Best regards,
Lu Baolu

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

* Re: [PATCH v1 4/9] iommu/vt-d: Add bounce buffer API for domain map/unmap
  2019-03-13  2:04     ` Lu Baolu
@ 2019-03-13  2:31       ` Lu Baolu
  2019-03-13 16:10         ` Christoph Hellwig
  0 siblings, 1 reply; 18+ messages in thread
From: Lu Baolu @ 2019-03-13  2:31 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: baolu.lu, David Woodhouse, Joerg Roedel, ashok.raj,
	jacob.jun.pan, alan.cox, kevin.tian, mika.westerberg, pengfei.xu,
	iommu, linux-kernel, Jacob Pan

Hi again,

On 3/13/19 10:04 AM, Lu Baolu wrote:
> Hi,
> 
> On 3/13/19 12:38 AM, Christoph Hellwig wrote:
>> On Tue, Mar 12, 2019 at 02:00:00PM +0800, Lu Baolu wrote:
>>> This adds the APIs for bounce buffer specified domain
>>> map() and unmap(). The start and end partial pages will
>>> be mapped with bounce buffered pages instead. This will
>>> enhance the security of DMA buffer by isolating the DMA
>>> attacks from malicious devices.
>>
>> Please reuse the swiotlb code instead of reinventing it.
>>

Just looked into the code again. At least we could reuse below
functions:

swiotlb_tbl_map_single()
swiotlb_tbl_unmap_single()
swiotlb_tbl_sync_single()

Anything else?

Best regards,
Lu Baolu

> 
> I don't think we are doing the same thing as swiotlb here. But it's
> always a good thing to reuse code if possible. I considered this when
> writing this code, but I found it's hard to do. Do you mind pointing
> me to the code which I could reuse?
> 
> Best regards,
> Lu Baolu
> 

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

* Re: [PATCH v1 4/9] iommu/vt-d: Add bounce buffer API for domain map/unmap
  2019-03-13  2:31       ` Lu Baolu
@ 2019-03-13 16:10         ` Christoph Hellwig
  2019-03-14  1:01           ` Lu Baolu
  2019-03-19  7:59           ` Lu Baolu
  0 siblings, 2 replies; 18+ messages in thread
From: Christoph Hellwig @ 2019-03-13 16:10 UTC (permalink / raw)
  To: Lu Baolu
  Cc: Christoph Hellwig, David Woodhouse, Joerg Roedel, ashok.raj,
	jacob.jun.pan, alan.cox, kevin.tian, mika.westerberg, pengfei.xu,
	iommu, linux-kernel, Jacob Pan

On Wed, Mar 13, 2019 at 10:31:52AM +0800, Lu Baolu wrote:
> Hi again,
> 
> On 3/13/19 10:04 AM, Lu Baolu wrote:
> > Hi,
> > 
> > On 3/13/19 12:38 AM, Christoph Hellwig wrote:
> > > On Tue, Mar 12, 2019 at 02:00:00PM +0800, Lu Baolu wrote:
> > > > This adds the APIs for bounce buffer specified domain
> > > > map() and unmap(). The start and end partial pages will
> > > > be mapped with bounce buffered pages instead. This will
> > > > enhance the security of DMA buffer by isolating the DMA
> > > > attacks from malicious devices.
> > > 
> > > Please reuse the swiotlb code instead of reinventing it.
> > > 
> 
> Just looked into the code again. At least we could reuse below
> functions:
> 
> swiotlb_tbl_map_single()
> swiotlb_tbl_unmap_single()
> swiotlb_tbl_sync_single()
> 
> Anything else?

Yes, that is probably about the level you want to reuse, given that the
next higher layer already has hooks into the direct mapping code.

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

* Re: [PATCH v1 4/9] iommu/vt-d: Add bounce buffer API for domain map/unmap
  2019-03-13 16:10         ` Christoph Hellwig
@ 2019-03-14  1:01           ` Lu Baolu
  2019-03-19  7:59           ` Lu Baolu
  1 sibling, 0 replies; 18+ messages in thread
From: Lu Baolu @ 2019-03-14  1:01 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: baolu.lu, David Woodhouse, Joerg Roedel, ashok.raj,
	jacob.jun.pan, alan.cox, kevin.tian, mika.westerberg, pengfei.xu,
	iommu, linux-kernel, Jacob Pan

Hi,

On 3/14/19 12:10 AM, Christoph Hellwig wrote:
> On Wed, Mar 13, 2019 at 10:31:52AM +0800, Lu Baolu wrote:
>> Hi again,
>>
>> On 3/13/19 10:04 AM, Lu Baolu wrote:
>>> Hi,
>>>
>>> On 3/13/19 12:38 AM, Christoph Hellwig wrote:
>>>> On Tue, Mar 12, 2019 at 02:00:00PM +0800, Lu Baolu wrote:
>>>>> This adds the APIs for bounce buffer specified domain
>>>>> map() and unmap(). The start and end partial pages will
>>>>> be mapped with bounce buffered pages instead. This will
>>>>> enhance the security of DMA buffer by isolating the DMA
>>>>> attacks from malicious devices.
>>>>
>>>> Please reuse the swiotlb code instead of reinventing it.
>>>>
>>
>> Just looked into the code again. At least we could reuse below
>> functions:
>>
>> swiotlb_tbl_map_single()
>> swiotlb_tbl_unmap_single()
>> swiotlb_tbl_sync_single()
>>
>> Anything else?
> 
> Yes, that is probably about the level you want to reuse, given that the
> next higher layer already has hooks into the direct mapping code.
> 

Okay. Thank you!

I will try to make this happen in v2.

Best regards,
Lu Baolu

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

* Re: [PATCH v1 4/9] iommu/vt-d: Add bounce buffer API for domain map/unmap
  2019-03-13 16:10         ` Christoph Hellwig
  2019-03-14  1:01           ` Lu Baolu
@ 2019-03-19  7:59           ` Lu Baolu
  2019-03-19 11:21             ` Robin Murphy
  1 sibling, 1 reply; 18+ messages in thread
From: Lu Baolu @ 2019-03-19  7:59 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: baolu.lu, David Woodhouse, Joerg Roedel, ashok.raj,
	jacob.jun.pan, alan.cox, kevin.tian, mika.westerberg, pengfei.xu,
	iommu, linux-kernel, Jacob Pan

Hi Christoph,

On 3/14/19 12:10 AM, Christoph Hellwig wrote:
> On Wed, Mar 13, 2019 at 10:31:52AM +0800, Lu Baolu wrote:
>> Hi again,
>>
>> On 3/13/19 10:04 AM, Lu Baolu wrote:
>>> Hi,
>>>
>>> On 3/13/19 12:38 AM, Christoph Hellwig wrote:
>>>> On Tue, Mar 12, 2019 at 02:00:00PM +0800, Lu Baolu wrote:
>>>>> This adds the APIs for bounce buffer specified domain
>>>>> map() and unmap(). The start and end partial pages will
>>>>> be mapped with bounce buffered pages instead. This will
>>>>> enhance the security of DMA buffer by isolating the DMA
>>>>> attacks from malicious devices.
>>>>
>>>> Please reuse the swiotlb code instead of reinventing it.
>>>>
>>
>> Just looked into the code again. At least we could reuse below
>> functions:
>>
>> swiotlb_tbl_map_single()
>> swiotlb_tbl_unmap_single()
>> swiotlb_tbl_sync_single()
>>
>> Anything else?
> 
> Yes, that is probably about the level you want to reuse, given that the
> next higher layer already has hooks into the direct mapping code.
> 

I am trying to change my code to reuse swiotlb. But I found that swiotlb
might not be suitable for my case.

Below is what I got with swiotlb_map():

phy_addr        size    tlb_addr
--------------------------------
0x167eec330     0x8     0x85dc6000
0x167eef5c0     0x40    0x85dc6800
0x167eec330     0x8     0x85dc7000
0x167eef5c0     0x40    0x85dc7800

But what I expected to get is:

phy_addr        size    tlb_addr
--------------------------------
0x167eec330     0x8     0xAAAAA330
0x167eef5c0     0x40    0xBBBBB5c0
0x167eec330     0x8     0xCCCCC330
0x167eef5c0     0x40    0xDDDDD5c0

, where 0xXXXXXX000 is the physical address of a bounced page.

Basically, I want a bounce page to replace a leaf page in the vt-d page
table, which maps a buffer with size less than a PAGE_SIZE.

Best regards,
Lu Baolu

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

* Re: [PATCH v1 4/9] iommu/vt-d: Add bounce buffer API for domain map/unmap
  2019-03-19  7:59           ` Lu Baolu
@ 2019-03-19 11:21             ` Robin Murphy
  0 siblings, 0 replies; 18+ messages in thread
From: Robin Murphy @ 2019-03-19 11:21 UTC (permalink / raw)
  To: Lu Baolu, Christoph Hellwig
  Cc: kevin.tian, mika.westerberg, ashok.raj, alan.cox, iommu,
	linux-kernel, pengfei.xu, jacob.jun.pan, David Woodhouse

On 19/03/2019 07:59, Lu Baolu wrote:
> Hi Christoph,
> 
> On 3/14/19 12:10 AM, Christoph Hellwig wrote:
>> On Wed, Mar 13, 2019 at 10:31:52AM +0800, Lu Baolu wrote:
>>> Hi again,
>>>
>>> On 3/13/19 10:04 AM, Lu Baolu wrote:
>>>> Hi,
>>>>
>>>> On 3/13/19 12:38 AM, Christoph Hellwig wrote:
>>>>> On Tue, Mar 12, 2019 at 02:00:00PM +0800, Lu Baolu wrote:
>>>>>> This adds the APIs for bounce buffer specified domain
>>>>>> map() and unmap(). The start and end partial pages will
>>>>>> be mapped with bounce buffered pages instead. This will
>>>>>> enhance the security of DMA buffer by isolating the DMA
>>>>>> attacks from malicious devices.
>>>>>
>>>>> Please reuse the swiotlb code instead of reinventing it.
>>>>>
>>>
>>> Just looked into the code again. At least we could reuse below
>>> functions:
>>>
>>> swiotlb_tbl_map_single()
>>> swiotlb_tbl_unmap_single()
>>> swiotlb_tbl_sync_single()
>>>
>>> Anything else?
>>
>> Yes, that is probably about the level you want to reuse, given that the
>> next higher layer already has hooks into the direct mapping code.
>>
> 
> I am trying to change my code to reuse swiotlb. But I found that swiotlb
> might not be suitable for my case.
> 
> Below is what I got with swiotlb_map():
> 
> phy_addr        size    tlb_addr
> --------------------------------
> 0x167eec330     0x8     0x85dc6000
> 0x167eef5c0     0x40    0x85dc6800
> 0x167eec330     0x8     0x85dc7000
> 0x167eef5c0     0x40    0x85dc7800
> 
> But what I expected to get is:
> 
> phy_addr        size    tlb_addr
> --------------------------------
> 0x167eec330     0x8     0xAAAAA330
> 0x167eef5c0     0x40    0xBBBBB5c0
> 0x167eec330     0x8     0xCCCCC330
> 0x167eef5c0     0x40    0xDDDDD5c0
> 
> , where 0xXXXXXX000 is the physical address of a bounced page.
> 
> Basically, I want a bounce page to replace a leaf page in the vt-d page
> table, which maps a buffer with size less than a PAGE_SIZE.

I'd imagine the thing to do would be to factor out the slot allocation 
in swiotlb_tbl_map_single() so that an IOMMU page pool/allocator can be 
hooked in as an alternative.

However we implement it, though, this should absolutely be a common 
IOMMU thing that all relevant DMA backends can opt into, and not 
specific to VT-d. I mean, it's already more or less the same concept as 
the PowerPC secure VM thing.

Robin.

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

end of thread, other threads:[~2019-03-19 11:21 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-12  5:59 [PATCH v1 0/9] Bounce buffer for untrusted devices Lu Baolu
2019-03-12  5:59 ` [PATCH v1 1/9] iommu/vt-d: Add trace events for domain map/unmap Lu Baolu
2019-03-12  5:59 ` [PATCH v1 2/9] iommu/vt-d: Add helpers for domain mapping/unmapping Lu Baolu
2019-03-12  5:59 ` [PATCH v1 3/9] iommu/vt-d: Add address walk helper Lu Baolu
2019-03-12  6:00 ` [PATCH v1 4/9] iommu/vt-d: Add bounce buffer API for domain map/unmap Lu Baolu
2019-03-12 16:38   ` Christoph Hellwig
2019-03-13  2:04     ` Lu Baolu
2019-03-13  2:31       ` Lu Baolu
2019-03-13 16:10         ` Christoph Hellwig
2019-03-14  1:01           ` Lu Baolu
2019-03-19  7:59           ` Lu Baolu
2019-03-19 11:21             ` Robin Murphy
2019-03-12  6:00 ` [PATCH v1 5/9] iommu/vt-d: Add bounce buffer API for dma sync Lu Baolu
2019-03-12  6:00 ` [PATCH v1 6/9] iommu/vt-d: Check whether device requires bounce buffer Lu Baolu
2019-03-12  6:00 ` [PATCH v1 7/9] iommu/vt-d: Add dma sync ops for untrusted devices Lu Baolu
2019-03-12  6:00 ` [PATCH v1 8/9] iommu/vt-d: Flush IOTLB for untrusted device in time Lu Baolu
2019-03-12  6:00 ` [PATCH v1 9/9] iommu/vt-d: Use bounce buffer for untrusted devices Lu Baolu
2019-03-12  6:07 ` [PATCH v1 0/9] Bounce " Lu Baolu

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