[PATCH 0/2] Use the correct page tables for SVA under PTI

[PATCH 0/2] Use the correct page tables for SVA under PTI

[Jacob Pan]

Shared virtual addressing allows DMA to user virtual address, But the
x86 IOMMU drivers are using the kernel copy of the process page tables
when PTI is enabled. This patchset tightens the security intended by
PTI by performing SVA binding with the appropriate process PGDs.

I have tested on Intel platform only, would appreciate itif someone
could help with testing SVA-KPTI on an AMD system.

Jacob Pan (2):
  x86: mm: Allow PTI helpers to be used outside x86/mm
  iommu: Use the user PGD for SVA if PTI is enabled

 arch/x86/include/asm/pgtable.h | 5 +++++
 drivers/iommu/amd/iommu_v2.c   | 4 +++-
 drivers/iommu/intel/svm.c      | 5 ++++-
 3 files changed, 12 insertions(+), 2 deletions(-)

-- 
2.25.1

[PATCH 1/2] x86: mm: Allow PTI helpers to be used outside x86/mm

[Jacob Pan]

With the support of shared virtual addressing(SVA), x86 IOMMUs also
need to get access to user PGD when sharing user mappings.

This patch makes sure the PTI helper function to retrieve user PGD
is available regardless the state of CONFIG_PAGE_TABLE_ISOLATION.
So far, such need is x86 only.

Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
 arch/x86/include/asm/pgtable.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index 44e2d6f1dbaa..42f55281e232 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -1227,6 +1227,11 @@ static inline p4d_t *user_to_kernel_p4dp(p4d_t *p4dp)
 {
 	return ptr_clear_bit(p4dp, PTI_PGTABLE_SWITCH_BIT);
 }
+#else
+static inline pgd_t *kernel_to_user_pgdp(pgd_t *pgdp)
+{
+	return pgdp;
+}
 #endif /* CONFIG_PAGE_TABLE_ISOLATION */
 
 /*
-- 
2.25.1

[PATCH 2/2] iommu: Use the user PGD for SVA if PTI is enabled

[Jacob Pan]

With page table isolation, the kernel manages two sets of page tables
for each process: one for user one for kernel. When enabling SVA, the
current x86 IOMMU drivers bind device and PASID with the kernel copy
of the process page table.

While there is no known "Meltdown" type of DMA attack, exposing
kernel mapping to DMA intended for userspace makes the system vulnerable
unnecessarily. It also breaks the intention of PTI.

This patch replaces kernel page table PGD with the user counterpart,
thus fulfill the promise of PTI on the DMA side.

Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
 drivers/iommu/amd/iommu_v2.c | 4 +++-
 drivers/iommu/intel/svm.c    | 5 ++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/amd/iommu_v2.c b/drivers/iommu/amd/iommu_v2.c
index 696d5555be57..aea3075b94af 100644
--- a/drivers/iommu/amd/iommu_v2.c
+++ b/drivers/iommu/amd/iommu_v2.c
@@ -600,6 +600,7 @@ int amd_iommu_bind_pasid(struct pci_dev *pdev, u32 pasid,
 	struct pasid_state *pasid_state;
 	struct device_state *dev_state;
 	struct mm_struct *mm;
+	pgd_t *pgd;
 	u32 sbdf;
 	int ret;
 
@@ -645,8 +646,9 @@ int amd_iommu_bind_pasid(struct pci_dev *pdev, u32 pasid,
 	if (ret)
 		goto out_unregister;
 
+	pgd = static_cpu_has(X86_FEATURE_PTI) ? kernel_to_user_pgdp(mm->pgd) : mm->pgd;
 	ret = amd_iommu_domain_set_gcr3(dev_state->domain, pasid,
-					__pa(pasid_state->mm->pgd));
+					__pa(pgd));
 	if (ret)
 		goto out_clear_state;
 
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index 8bcfb93dda56..7472cd98d3e8 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -332,6 +332,7 @@ static struct iommu_sva *intel_svm_bind_mm(struct intel_iommu *iommu,
 	struct intel_svm *svm;
 	unsigned long sflags;
 	int ret = 0;
+	pgd_t *pgd;
 
 	svm = pasid_private_find(mm->pasid);
 	if (!svm) {
@@ -394,7 +395,9 @@ static struct iommu_sva *intel_svm_bind_mm(struct intel_iommu *iommu,
 	sflags = (flags & SVM_FLAG_SUPERVISOR_MODE) ?
 			PASID_FLAG_SUPERVISOR_MODE : 0;
 	sflags |= cpu_feature_enabled(X86_FEATURE_LA57) ? PASID_FLAG_FL5LP : 0;
-	ret = intel_pasid_setup_first_level(iommu, dev, mm->pgd, mm->pasid,
+
+	pgd = static_cpu_has(X86_FEATURE_PTI) ? kernel_to_user_pgdp(mm->pgd) : mm->pgd;
+	ret = intel_pasid_setup_first_level(iommu, dev, pgd, mm->pasid,
 					    FLPT_DEFAULT_DID, sflags);
 	if (ret)
 		goto free_sdev;
-- 
2.25.1

Re: [PATCH 2/2] iommu: Use the user PGD for SVA if PTI is enabled

[Dave Hansen]

On 8/22/22 13:12, Jacob Pan wrote:
> @@ -394,7 +395,9 @@ static struct iommu_sva *intel_svm_bind_mm(struct intel_iommu *iommu,
>  	sflags = (flags & SVM_FLAG_SUPERVISOR_MODE) ?
>  			PASID_FLAG_SUPERVISOR_MODE : 0;
>  	sflags |= cpu_feature_enabled(X86_FEATURE_LA57) ? PASID_FLAG_FL5LP : 0;
> -	ret = intel_pasid_setup_first_level(iommu, dev, mm->pgd, mm->pasid,
> +
> +	pgd = static_cpu_has(X86_FEATURE_PTI) ? kernel_to_user_pgdp(mm->pgd) : mm->pgd;
> +	ret = intel_pasid_setup_first_level(iommu, dev, pgd, mm->pasid,
>  					    FLPT_DEFAULT_DID, sflags);

This X86_FEATURE_PTI should really be done within a helper.

I'd probably do this with a *new* helper since all of the existing
kernel_to_user_pgdp() users seem to be within a PTI #ifdef.

Maybe something like:

pgd_t *mm_user_pgd(struct mm_struct *mm)
{
#ifdef CONFIG_PAGE_TABLE_ISOLATION
	if (cpu_feature_enabled(X86_FEATURE_PTI))
		return kernel_to_user_pgdp(mm->pgd);
#endif
	return mm->pgd;
}

That #ifdef could even go away if your kernel_to_user_pgdp() stub from
patch 1/2 was available.  I'm not sure it's worth it though.

Re: [PATCH 2/2] iommu: Use the user PGD for SVA if PTI is enabled

[Jacob Pan]

Hi Dave,

On Mon, 22 Aug 2022 15:31:20 -0700, Dave Hansen <dave.hansen@intel.com>
wrote:

> On 8/22/22 13:12, Jacob Pan wrote:
> > @@ -394,7 +395,9 @@ static struct iommu_sva *intel_svm_bind_mm(struct
> > intel_iommu *iommu, sflags = (flags & SVM_FLAG_SUPERVISOR_MODE) ?
> >  			PASID_FLAG_SUPERVISOR_MODE : 0;
> >  	sflags |= cpu_feature_enabled(X86_FEATURE_LA57) ?
> > PASID_FLAG_FL5LP : 0;
> > -	ret = intel_pasid_setup_first_level(iommu, dev, mm->pgd,
> > mm->pasid, +
> > +	pgd = static_cpu_has(X86_FEATURE_PTI) ?
> > kernel_to_user_pgdp(mm->pgd) : mm->pgd;
> > +	ret = intel_pasid_setup_first_level(iommu, dev, pgd, mm->pasid,
> >  					    FLPT_DEFAULT_DID, sflags);
> >  
> 
> This X86_FEATURE_PTI should really be done within a helper.
> 
> I'd probably do this with a *new* helper since all of the existing
> kernel_to_user_pgdp() users seem to be within a PTI #ifdef.
> 
> Maybe something like:
> 
> pgd_t *mm_user_pgd(struct mm_struct *mm)
> {
> #ifdef CONFIG_PAGE_TABLE_ISOLATION
> 	if (cpu_feature_enabled(X86_FEATURE_PTI))
> 		return kernel_to_user_pgdp(mm->pgd);
> #endif
> 	return mm->pgd;
> }
> 
Sounds good. I thought about a helper also, thinking there are so many other
cpu_has(X86_FEATURE_PTI) checks already :)

> That #ifdef could even go away if your kernel_to_user_pgdp() stub from
> patch 1/2 was available.  I'm not sure it's worth it though.
I will remove 1/2 and keep the uniform style of the existing helpers.

Thanks for the suggestion,

Jacob

Re: [PATCH 2/2] iommu: Use the user PGD for SVA if PTI is enabled

[Dave Hansen]

On 8/22/22 16:24, Jacob Pan wrote:
> Sounds good. I thought about a helper also, thinking there are so many other
> cpu_has(X86_FEATURE_PTI) checks already :)

Yes, but almost all of those are in PTI-#ifdef'd code already.

Re: [PATCH 0/2] Use the correct page tables for SVA under PTI

[Jacob Pan]

Hi Baolu/Joerg,

Just wondering if you have any comments? Thanks.

On Mon, 22 Aug 2022 13:12:11 -0700, Jacob Pan
<jacob.jun.pan@linux.intel.com> wrote:

> Shared virtual addressing allows DMA to user virtual address, But the
> x86 IOMMU drivers are using the kernel copy of the process page tables
> when PTI is enabled. This patchset tightens the security intended by
> PTI by performing SVA binding with the appropriate process PGDs.
> 
> I have tested on Intel platform only, would appreciate itif someone
> could help with testing SVA-KPTI on an AMD system.
> 
> Jacob Pan (2):
>   x86: mm: Allow PTI helpers to be used outside x86/mm
>   iommu: Use the user PGD for SVA if PTI is enabled
> 
>  arch/x86/include/asm/pgtable.h | 5 +++++
>  drivers/iommu/amd/iommu_v2.c   | 4 +++-
>  drivers/iommu/intel/svm.c      | 5 ++++-
>  3 files changed, 12 insertions(+), 2 deletions(-)
> 


Thanks,

Jacob

Re: [PATCH 2/2] iommu: Use the user PGD for SVA if PTI is enabled

[Baolu Lu]

On 8/23/22 4:12 AM, Jacob Pan wrote:
> With page table isolation, the kernel manages two sets of page tables
> for each process: one for user one for kernel. When enabling SVA, the
> current x86 IOMMU drivers bind device and PASID with the kernel copy
> of the process page table.
> 
> While there is no known "Meltdown" type of DMA attack, exposing
> kernel mapping to DMA intended for userspace makes the system vulnerable
> unnecessarily. It also breaks the intention of PTI.
> 
> This patch replaces kernel page table PGD with the user counterpart,
> thus fulfill the promise of PTI on the DMA side.
> 
> Signed-off-by: Jacob Pan<jacob.jun.pan@linux.intel.com>

Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>

Best regards,
baolu