Re: [PATCH v3 0/6] Add support for privileged mappings
On Mon, Jul 25 2016 at 10:50:13 AM, Will Deacon wrote: > On Fri, Jul 22, 2016 at 01:39:45PM -0700, Mitchel Humpherys wrote: >> On Fri, Jul 22 2016 at 05:51:07 PM, Will Deacon wrote: >> > On Tue, Jul 19, 2016 at 01:36:49PM -0700, 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) for users of the DMA API that need privileged >> >> mappings which are inaccessible to lesser-privileged execution levels, 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]. >> > >> > This all looks good to me: >> > >> > Acked-by: Will Deacon >> > >> > It looks pretty fiddly to merge, however. How are you planning to get >> > this upstream? >> >> Fiddly in what way? Do you mean in relation to "dma-mapping: Use >> unsigned long for dma_attrs" [1]? I admit I wasn't aware of that >> activity until Robin mentioned it. It looks like it's merged on >> next/master, shall I rebase/rework on that and resend? > > Fiddly in that it touches multiple subsystems. I guess routing it via > the iommu tree (Joerg) might be the best bet. Sounds good. I'm going to rebase on linux-next as well anyways to get the new dma attrs format and resend. -Mitch -- 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
Re: [PATCH v3 0/6] Add support for privileged mappings
On Fri, Jul 22, 2016 at 01:39:45PM -0700, Mitchel Humpherys wrote: > On Fri, Jul 22 2016 at 05:51:07 PM, Will Deacon wrote: > > On Tue, Jul 19, 2016 at 01:36:49PM -0700, 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) for users of the DMA API that need privileged > >> mappings which are inaccessible to lesser-privileged execution levels, 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]. > > > > This all looks good to me: > > > > Acked-by: Will Deacon > > > > It looks pretty fiddly to merge, however. How are you planning to get > > this upstream? > > Fiddly in what way? Do you mean in relation to "dma-mapping: Use > unsigned long for dma_attrs" [1]? I admit I wasn't aware of that > activity until Robin mentioned it. It looks like it's merged on > next/master, shall I rebase/rework on that and resend? Fiddly in that it touches multiple subsystems. I guess routing it via the iommu tree (Joerg) might be the best bet. Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 0/6] Add support for privileged mappings
On Fri, Jul 22 2016 at 05:51:07 PM, Will Deacon wrote: > On Tue, Jul 19, 2016 at 01:36:49PM -0700, 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) for users of the DMA API that need privileged >> mappings which are inaccessible to lesser-privileged execution levels, 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]. > > This all looks good to me: > > Acked-by: Will Deacon > > It looks pretty fiddly to merge, however. How are you planning to get > this upstream? Fiddly in what way? Do you mean in relation to "dma-mapping: Use unsigned long for dma_attrs" [1]? I admit I wasn't aware of that activity until Robin mentioned it. It looks like it's merged on next/master, shall I rebase/rework on that and resend? [1] https://lkml.org/lkml/2016/7/13/198 -Mitch -- 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
Re: [PATCH v3 0/6] Add support for privileged mappings
On Tue, Jul 19, 2016 at 01:36:49PM -0700, 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) for users of the DMA API that need privileged > mappings which are inaccessible to lesser-privileged execution levels, 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]. This all looks good to me: Acked-by: Will Deacon It looks pretty fiddly to merge, however. How are you planning to get this upstream? Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 0/6] Add support for privileged mappings
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) for users of the DMA API that need privileged mappings which are inaccessible to lesser-privileged execution levels, 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: v2..v3 - Incorporated feedback from Robin: * Various comments and re-wordings. * Use existing bit definitions for IOMMU_PRIV implementation in io-pgtable-arm. * Renamed and redocumented dma_direction_to_prot. * Don't worry about executability in new DMA attr. 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 common: DMA-mapping: add DMA_ATTR_PRIVILEGED attribute arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED dmaengine: pl330: Make sure microcode is privileged Revert "iommu/arm-smmu: Treat all device transactions as unprivileged" Documentation/DMA-attributes.txt | 10 ++ arch/arm64/mm/dma-mapping.c | 6 +++--- drivers/dma/pl330.c | 7 +-- drivers/iommu/arm-smmu.c | 5 + drivers/iommu/dma-iommu.c| 16 +++- drivers/iommu/io-pgtable-arm.c | 5 - include/linux/dma-attrs.h| 1 + include/linux/dma-iommu.h| 3 ++- include/linux/iommu.h| 1 + 9 files changed, 38 insertions(+), 16 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
