Re: [PATCH v2 0/6] Add support for privileged mappings

[Jordan Crouse]

On Mon, Jul 11, 2016 at 03:02:24PM +0100, Robin Murphy wrote:
> Hey Mitch,
> 
> Thanks for having the necessary go at the DMA API - I think the series
> looks broadly workable now.
> 
> On 09/07/16 03:09, Mitchel Humpherys wrote:
> > The following patch to the ARM SMMU driver:
> > 
> > commit d346180e70b91b3d5a1ae7e5603e65593d4622bc
> > Author: Robin Murphy 
> > Date:   Tue Jan 26 18:06:34 2016 +
> > 
> > iommu/arm-smmu: Treat all device transactions as unprivileged
> > 
> > started forcing all SMMU transactions to come through as "unprivileged".
> > The rationale given was that:
> > 
> >   (1) There is no way in the IOMMU API to even request privileged mappings.
> > 
> >   (2) It's difficult to implement a DMA mapper that correctly models the
> >   ARM VMSAv8 behavior of unprivileged-writeable =>
> >   privileged-execute-never.
> > 
> > This series rectifies (1) by introducing an IOMMU API for privileged
> > mappings and implements it in io-pgtable-arm.
> > 
> > This series rectifies (2) by introducing a new dma attribute
> > (DMA_ATTR_PRIVILEGED_EXECUTABLE) for users of the DMA API that need
> > privileged, executable mappings, and implements it in the arm64 IOMMU DMA
> > mapper.  The one known user (pl330.c) is converted over to the new
> > attribute.
> > 
> > Jordan and Jeremy can provide more info on the use case if needed, but the
> > high level is that it's a security feature to prevent attacks such as [1].
> 
> My understanding of that hack is that it involves switching the GPU into
> privileged mode to get at the mapping of the IOMMU registers in the
> first place, so I don't see at a glance how having privileged mappings
> defends against that. What it clearly does do, however, is get us _to_
> the point where it's necessary to do such a privilege switch in the
> first place, as opposed to everything being trivially wide-open, which
> is no bad thing.

It was a two pronged attack.  First the attacker had to use the GPU to construct
a new pagetable and some various GPU command buffers and then they could switch
the context to the new pagetable to complete the attack.

The first part was made possible by vast tracts of read/write memory that is
mapped globally and accessible to all users so it was easy and legal to write
whatever the user wished including a pusedo-pagetable.

As you said, while the actual fix for the exploit was blocking access to the 
SMMU
registers it isn't a bad thing if we get in the habit of assuming that we 
shouldn't
leave a bunch of scratch memory around for somebody clever to take advantage of.

Jordan
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 0/6] Add support for privileged mappings

[Robin Murphy]

Hey Mitch,

Thanks for having the necessary go at the DMA API - I think the series
looks broadly workable now.

On 09/07/16 03:09, Mitchel Humpherys wrote:
> The following patch to the ARM SMMU driver:
> 
> commit d346180e70b91b3d5a1ae7e5603e65593d4622bc
> Author: Robin Murphy 
> Date:   Tue Jan 26 18:06:34 2016 +
> 
> iommu/arm-smmu: Treat all device transactions as unprivileged
> 
> started forcing all SMMU transactions to come through as "unprivileged".
> The rationale given was that:
> 
>   (1) There is no way in the IOMMU API to even request privileged mappings.
> 
>   (2) It's difficult to implement a DMA mapper that correctly models the
>   ARM VMSAv8 behavior of unprivileged-writeable =>
>   privileged-execute-never.
> 
> This series rectifies (1) by introducing an IOMMU API for privileged
> mappings and implements it in io-pgtable-arm.
> 
> This series rectifies (2) by introducing a new dma attribute
> (DMA_ATTR_PRIVILEGED_EXECUTABLE) for users of the DMA API that need
> privileged, executable mappings, and implements it in the arm64 IOMMU DMA
> mapper.  The one known user (pl330.c) is converted over to the new
> attribute.
> 
> Jordan and Jeremy can provide more info on the use case if needed, but the
> high level is that it's a security feature to prevent attacks such as [1].

My understanding of that hack is that it involves switching the GPU into
privileged mode to get at the mapping of the IOMMU registers in the
first place, so I don't see at a glance how having privileged mappings
defends against that. What it clearly does do, however, is get us _to_
the point where it's necessary to do such a privilege switch in the
first place, as opposed to everything being trivially wide-open, which
is no bad thing.

Robin.

> 
> [1] https://github.com/robclark/kilroy
> 
> Changelog:
> 
>   v1..v2
> 
> - Added a new DMA attribute to make executable privileged mappings
>   work, and use that in the pl330 driver (suggested by Will).
> 
> 
> Jeremy Gebben (1):
>   iommu/io-pgtable-arm: add support for the IOMMU_PRIV flag
> 
> Mitchel Humpherys (5):
>   iommu: add IOMMU_PRIV attribute
>   Revert "iommu/arm-smmu: Treat all device transactions as unprivileged"
>   common: DMA-mapping: add DMA_ATTR_PRIVILEGED_EXECUTABLE attribute
>   arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED_EXECUTABLE
>   dmaengine: pl330: Make sure microcode is privileged-executable
> 
>  Documentation/DMA-attributes.txt |  9 +
>  arch/arm64/mm/dma-mapping.c  |  6 +++---
>  drivers/dma/pl330.c  |  7 +--
>  drivers/iommu/arm-smmu.c |  5 +
>  drivers/iommu/dma-iommu.c| 15 +++
>  drivers/iommu/io-pgtable-arm.c   | 16 +++-
>  include/linux/dma-attrs.h|  1 +
>  include/linux/dma-iommu.h|  3 ++-
>  include/linux/iommu.h|  1 +
>  9 files changed, 44 insertions(+), 19 deletions(-)
> 

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

[PATCH v2 0/6] Add support for privileged mappings

[Mitchel Humpherys]

The following patch to the ARM SMMU driver:

commit d346180e70b91b3d5a1ae7e5603e65593d4622bc
Author: Robin Murphy 
Date:   Tue Jan 26 18:06:34 2016 +

iommu/arm-smmu: Treat all device transactions as unprivileged

started forcing all SMMU transactions to come through as "unprivileged".
The rationale given was that:

  (1) There is no way in the IOMMU API to even request privileged mappings.

  (2) It's difficult to implement a DMA mapper that correctly models the
  ARM VMSAv8 behavior of unprivileged-writeable =>
  privileged-execute-never.

This series rectifies (1) by introducing an IOMMU API for privileged
mappings and implements it in io-pgtable-arm.

This series rectifies (2) by introducing a new dma attribute
(DMA_ATTR_PRIVILEGED_EXECUTABLE) for users of the DMA API that need
privileged, executable mappings, and implements it in the arm64 IOMMU DMA
mapper.  The one known user (pl330.c) is converted over to the new
attribute.

Jordan and Jeremy can provide more info on the use case if needed, but the
high level is that it's a security feature to prevent attacks such as [1].

[1] https://github.com/robclark/kilroy

Changelog:

  v1..v2

- Added a new DMA attribute to make executable privileged mappings
  work, and use that in the pl330 driver (suggested by Will).


Jeremy Gebben (1):
  iommu/io-pgtable-arm: add support for the IOMMU_PRIV flag

Mitchel Humpherys (5):
  iommu: add IOMMU_PRIV attribute
  Revert "iommu/arm-smmu: Treat all device transactions as unprivileged"
  common: DMA-mapping: add DMA_ATTR_PRIVILEGED_EXECUTABLE attribute
  arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED_EXECUTABLE
  dmaengine: pl330: Make sure microcode is privileged-executable

 Documentation/DMA-attributes.txt |  9 +
 arch/arm64/mm/dma-mapping.c  |  6 +++---
 drivers/dma/pl330.c  |  7 +--
 drivers/iommu/arm-smmu.c |  5 +
 drivers/iommu/dma-iommu.c| 15 +++
 drivers/iommu/io-pgtable-arm.c   | 16 +++-
 include/linux/dma-attrs.h|  1 +
 include/linux/dma-iommu.h|  3 ++-
 include/linux/iommu.h|  1 +
 9 files changed, 44 insertions(+), 19 deletions(-)

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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