Re: [PATCH v4 0/9] iommu: Bounce page for untrusted devices

2019-06-12 Thread Mika Westerberg
On Wed, Jun 12, 2019 at 11:00:06AM +0800, Lu Baolu wrote:
> > What kind of devices did you test it with?
> 
> Most test work was done by Xu Pengfei (cc'ed). He has run the code
> on real platforms with various thunderbolt peripherals (usb, disk,
> network, etc.).

In addtition to that we are also in works to build a real thunderclap
platform to verify it can only access the bounce buffer if the DMA
transfer was to a memory not filling the whole page.


Re: [PATCH v4 0/9] iommu: Bounce page for untrusted devices

2019-06-11 Thread Lu Baolu

Hi,

On 6/11/19 12:10 AM, Konrad Rzeszutek Wilk wrote:

On Mon, Jun 03, 2019 at 09:16:11AM +0800, Lu Baolu wrote:

The Thunderbolt vulnerabilities are public and have a nice
name as Thunderclap [1] [3] nowadays. This patch series aims
to mitigate those concerns.


.. Forgot to ask but should the patches also include the CVE number?
Or at least the last one that enables this?


I am sorry, but what's CVE number and where could I get one?

Best regards,
Baolu


Re: [PATCH v4 0/9] iommu: Bounce page for untrusted devices

2019-06-11 Thread Lu Baolu

Hi,

On 6/10/19 11:42 PM, Konrad Rzeszutek Wilk wrote:

On Mon, Jun 03, 2019 at 09:16:11AM +0800, Lu Baolu wrote:

The Thunderbolt vulnerabilities are public and have a nice
name as Thunderclap [1] [3] nowadays. This patch series aims
to mitigate those concerns.

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. Previously (v3 and before), we proposed
an optimisation to only copy the head and tail of the buffer
if it spans multiple pages, and directly map the ones in the
middle. Figure 1 gives a big picture about this solution.

 swiotlb System
 IOVA  bounce page   Memory
  .-.  .-..-.
  | |  | || |
  | |  | || |
buffer_start .-.  .-..-.
  | |->| |***>| |
  | |  | | swiotlb| |
  | |  | | mapping| |
  IOMMU Page  '-'  '-''-'
   Boundary   | | | |
  | | | |
  | | | |
  | |>| |
  | |IOMMU mapping| |
  | | | |
  IOMMU Page  .-. .-.
   Boundary   | | | |
  | | | |
  | |>| |
  | | IOMMU mapping   | |
  | | | |
  | | | |
  IOMMU Page  .-.  .-..-.
   Boundary   | |  | || |
  | |  | || |
  | |->| |***>| |
   buffer_end '-'  '-' swiotlb'-'
  | |  | | mapping| |
  | |  | || |
  '-'  '-''-'
   Figure 1: A big view of iommu bounce page

As Robin Murphy pointed out, this ties us to using strict mode for
TLB maintenance, which may not be an overall win depending on the
balance between invalidation bandwidth vs. memcpy bandwidth. If we
use standard SWIOTLB logic to always copy the whole thing, we should
be able to release the bounce pages via the flush queue to allow
'safe' lazy unmaps. So since v4 we start to use the standard swiotlb
logic.

 swiotlb System
 IOVA  bounce page   Memory
buffer_start .-.  .-..-.
  | |  | || |
  | |  | || |
  | |  | |.-.physical
  | |->| | -->| |_start
  | |iommu | | swiotlb| |
  | | map  | |   map  | |
  IOMMU Page  .-.  .-.'-'


The prior picture had 'buffer_start' at an offset in the page. I am
assuming you meant that here in as well?


In prior picture, since we only use bounce buffer for head and tail
partial-page buffers, so we need to return buffer_start at the same
offset as the physical buffer.

Here, we use a whole swiotlb bounce buffer, hence we should use the same
offset as the bounce buffer (a.k.a. offset = 0).



Meaning it starts at the same offset as 'physical_start' in the right
side box?


   Boundary   | |  | || 

Re: [PATCH v4 0/9] iommu: Bounce page for untrusted devices

2019-06-10 Thread Konrad Rzeszutek Wilk
On Mon, Jun 03, 2019 at 09:16:11AM +0800, Lu Baolu wrote:
> The Thunderbolt vulnerabilities are public and have a nice
> name as Thunderclap [1] [3] nowadays. This patch series aims
> to mitigate those concerns.

.. Forgot to ask but should the patches also include the CVE number?
Or at least the last one that enables this?

Thanks.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v4 0/9] iommu: Bounce page for untrusted devices

2019-06-10 Thread Konrad Rzeszutek Wilk
On Mon, Jun 03, 2019 at 09:16:11AM +0800, Lu Baolu wrote:
> The Thunderbolt vulnerabilities are public and have a nice
> name as Thunderclap [1] [3] nowadays. This patch series aims
> to mitigate those concerns.
> 
> 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. Previously (v3 and before), we proposed
> an optimisation to only copy the head and tail of the buffer
> if it spans multiple pages, and directly map the ones in the
> middle. Figure 1 gives a big picture about this solution.
> 
> swiotlb System
> IOVA  bounce page   Memory
>  .-.  .-..-.
>  | |  | || |
>  | |  | || |
> buffer_start .-.  .-..-.
>  | |->| |***>| |
>  | |  | | swiotlb| |
>  | |  | | mapping| |
>  IOMMU Page  '-'  '-''-'
>   Boundary   | | | |
>  | | | |
>  | | | |
>  | |>| |
>  | |IOMMU mapping| |
>  | | | |
>  IOMMU Page  .-. .-.
>   Boundary   | | | |
>  | | | |
>  | |>| |
>  | | IOMMU mapping   | |
>  | | | |
>  | | | |
>  IOMMU Page  .-.  .-..-.
>   Boundary   | |  | || |
>  | |  | || |
>  | |->| |***>| |
>   buffer_end '-'  '-' swiotlb'-'
>  | |  | | mapping| |
>  | |  | || |
>  '-'  '-''-'
>   Figure 1: A big view of iommu bounce page 
> 
> As Robin Murphy pointed out, this ties us to using strict mode for
> TLB maintenance, which may not be an overall win depending on the
> balance between invalidation bandwidth vs. memcpy bandwidth. If we
> use standard SWIOTLB logic to always copy the whole thing, we should
> be able to release the bounce pages via the flush queue to allow
> 'safe' lazy unmaps. So since v4 we start to use the standard swiotlb
> logic.
> 
> swiotlb System
> IOVA  bounce page   Memory
> buffer_start .-.  .-..-.
>  | |  | || |
>  | |  | || |
>  | |  | |.-.physical
>  | |->| | -->| |_start  
>  | |iommu | | swiotlb| |
>  | | map  | |   map  | |
>  IOMMU Page  .-.  .-.'-'

The prior picture had 'buffer_start' at an offset in the page. I am
assuming you meant that here in as well?

Meaning it starts at the same offset as 'physical_start' in the right
side box?

>   Boundary   | |  | || |
>  | |  | || |
>  | |->| || |
>  | |iommu | || |
>  | | map  |