linux-sgx.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86/sgx: Fix sgx_encl_may_map locking
@ 2020-10-05  0:41 Jarkko Sakkinen
  2020-10-05  3:27 ` Haitao Huang
  0 siblings, 1 reply; 3+ messages in thread
From: Jarkko Sakkinen @ 2020-10-05  0:41 UTC (permalink / raw)
  To: linux-sgx
  Cc: Jarkko Sakkinen, Haitao Huang, Sean Christopherson,
	Jethro Beekman, Matthew Wilcox, Dave Hansen

Fix the issue further discussed in:

1. https://lore.kernel.org/linux-sgx/op.0rwbv916wjvjmi@mqcpg7oapc828.gar.corp.intel.com/
2. https://lore.kernel.org/linux-sgx/20201003195440.GD20115@casper.infradead.org/

Reported-by: Haitao Huang <haitao.huang@linux.intel.com>
Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Cc: Jethro Beekman <jethro@fortanix.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 arch/x86/kernel/cpu/sgx/encl.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c
index ae45f8f0951e..a225e96c7a39 100644
--- a/arch/x86/kernel/cpu/sgx/encl.c
+++ b/arch/x86/kernel/cpu/sgx/encl.c
@@ -304,11 +304,10 @@ int sgx_encl_may_map(struct sgx_encl *encl, unsigned long start,
 		     unsigned long end, unsigned long vm_flags)
 {
 	unsigned long vm_prot_bits = vm_flags & (VM_READ | VM_WRITE | VM_EXEC);
-	unsigned long idx_start = PFN_DOWN(start);
-	unsigned long idx_end = PFN_DOWN(end - 1);
+	unsigned long start_i = PFN_DOWN(start);
+	unsigned long end_i = PFN_DOWN(end - 1);
 	struct sgx_encl_page *page;
-
-	XA_STATE(xas, &encl->page_array, idx_start);
+	int i;
 
 	/*
 	 * Disallow READ_IMPLIES_EXEC tasks as their VMA permissions might
@@ -317,9 +316,14 @@ int sgx_encl_may_map(struct sgx_encl *encl, unsigned long start,
 	if (current->personality & READ_IMPLIES_EXEC)
 		return -EACCES;
 
-	xas_for_each(&xas, page, idx_end)
+	for (i = start_i; i <= end_i; i++) {
+		mutex_lock(&encl->lock);
+		page = xa_load(&encl->page_array, i);
+		mutex_unlock(&encl->lock);
+
 		if (!page || (~page->vm_max_prot_bits & vm_prot_bits))
 			return -EACCES;
+	}
 
 	return 0;
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] x86/sgx: Fix sgx_encl_may_map locking
  2020-10-05  0:41 [PATCH] x86/sgx: Fix sgx_encl_may_map locking Jarkko Sakkinen
@ 2020-10-05  3:27 ` Haitao Huang
  2020-10-05  3:43   ` Jarkko Sakkinen
  0 siblings, 1 reply; 3+ messages in thread
From: Haitao Huang @ 2020-10-05  3:27 UTC (permalink / raw)
  To: linux-sgx, Jarkko Sakkinen
  Cc: Sean Christopherson, Jethro Beekman, Matthew Wilcox, Dave Hansen

On Sun, 04 Oct 2020 19:41:20 -0500, Jarkko Sakkinen  
<jarkko.sakkinen@linux.intel.com> wrote:

> Fix the issue further discussed in:
>
> 1.  
> https://lore.kernel.org/linux-sgx/op.0rwbv916wjvjmi@mqcpg7oapc828.gar.corp.intel.com/
> 2.  
> https://lore.kernel.org/linux-sgx/20201003195440.GD20115@casper.infradead.org/
>
> Reported-by: Haitao Huang <haitao.huang@linux.intel.com>
> Cc: Sean Christopherson <sean.j.christopherson@intel.com>
> Cc: Jethro Beekman <jethro@fortanix.com>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>  arch/x86/kernel/cpu/sgx/encl.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/sgx/encl.c  
> b/arch/x86/kernel/cpu/sgx/encl.c
> index ae45f8f0951e..a225e96c7a39 100644
> --- a/arch/x86/kernel/cpu/sgx/encl.c
> +++ b/arch/x86/kernel/cpu/sgx/encl.c
> @@ -304,11 +304,10 @@ int sgx_encl_may_map(struct sgx_encl *encl,  
> unsigned long start,
>  		     unsigned long end, unsigned long vm_flags)
>  {
>  	unsigned long vm_prot_bits = vm_flags & (VM_READ | VM_WRITE | VM_EXEC);
> -	unsigned long idx_start = PFN_DOWN(start);
> -	unsigned long idx_end = PFN_DOWN(end - 1);
> +	unsigned long start_i = PFN_DOWN(start);
> +	unsigned long end_i = PFN_DOWN(end - 1);
>  	struct sgx_encl_page *page;
> -
> -	XA_STATE(xas, &encl->page_array, idx_start);
> +	int i;
> 	/*
>  	 * Disallow READ_IMPLIES_EXEC tasks as their VMA permissions might
> @@ -317,9 +316,14 @@ int sgx_encl_may_map(struct sgx_encl *encl,  
> unsigned long start,
>  	if (current->personality & READ_IMPLIES_EXEC)
>  		return -EACCES;
> -	xas_for_each(&xas, page, idx_end)
> +	for (i = start_i; i <= end_i; i++) {
> +		mutex_lock(&encl->lock);
> +		page = xa_load(&encl->page_array, i);
> +		mutex_unlock(&encl->lock);
> +
>  		if (!page || (~page->vm_max_prot_bits & vm_prot_bits))
>  			return -EACCES;
> +	}
> 	return 0;
>  }

Works with no PROVE_LOCKING complaints.
Haitao

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] x86/sgx: Fix sgx_encl_may_map locking
  2020-10-05  3:27 ` Haitao Huang
@ 2020-10-05  3:43   ` Jarkko Sakkinen
  0 siblings, 0 replies; 3+ messages in thread
From: Jarkko Sakkinen @ 2020-10-05  3:43 UTC (permalink / raw)
  To: Haitao Huang
  Cc: linux-sgx, Sean Christopherson, Jethro Beekman, Matthew Wilcox,
	Dave Hansen

On Sun, Oct 04, 2020 at 10:27:43PM -0500, Haitao Huang wrote:
> On Sun, 04 Oct 2020 19:41:20 -0500, Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> 
> > Fix the issue further discussed in:
> > 
> > 1. https://lore.kernel.org/linux-sgx/op.0rwbv916wjvjmi@mqcpg7oapc828.gar.corp.intel.com/
> > 2. https://lore.kernel.org/linux-sgx/20201003195440.GD20115@casper.infradead.org/
> > 
> > Reported-by: Haitao Huang <haitao.huang@linux.intel.com>
> > Cc: Sean Christopherson <sean.j.christopherson@intel.com>
> > Cc: Jethro Beekman <jethro@fortanix.com>
> > Cc: Matthew Wilcox <willy@infradead.org>
> > Cc: Dave Hansen <dave.hansen@linux.intel.com>
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > ---
> >  arch/x86/kernel/cpu/sgx/encl.c | 14 +++++++++-----
> >  1 file changed, 9 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/cpu/sgx/encl.c
> > b/arch/x86/kernel/cpu/sgx/encl.c
> > index ae45f8f0951e..a225e96c7a39 100644
> > --- a/arch/x86/kernel/cpu/sgx/encl.c
> > +++ b/arch/x86/kernel/cpu/sgx/encl.c
> > @@ -304,11 +304,10 @@ int sgx_encl_may_map(struct sgx_encl *encl,
> > unsigned long start,
> >  		     unsigned long end, unsigned long vm_flags)
> >  {
> >  	unsigned long vm_prot_bits = vm_flags & (VM_READ | VM_WRITE | VM_EXEC);
> > -	unsigned long idx_start = PFN_DOWN(start);
> > -	unsigned long idx_end = PFN_DOWN(end - 1);
> > +	unsigned long start_i = PFN_DOWN(start);
> > +	unsigned long end_i = PFN_DOWN(end - 1);
> >  	struct sgx_encl_page *page;
> > -
> > -	XA_STATE(xas, &encl->page_array, idx_start);
> > +	int i;
> > 	/*
> >  	 * Disallow READ_IMPLIES_EXEC tasks as their VMA permissions might
> > @@ -317,9 +316,14 @@ int sgx_encl_may_map(struct sgx_encl *encl,
> > unsigned long start,
> >  	if (current->personality & READ_IMPLIES_EXEC)
> >  		return -EACCES;
> > -	xas_for_each(&xas, page, idx_end)
> > +	for (i = start_i; i <= end_i; i++) {
> > +		mutex_lock(&encl->lock);
> > +		page = xa_load(&encl->page_array, i);
> > +		mutex_unlock(&encl->lock);
> > +
> >  		if (!page || (~page->vm_max_prot_bits & vm_prot_bits))
> >  			return -EACCES;
> > +	}
> > 	return 0;
> >  }
> 
> Works with no PROVE_LOCKING complaints.
> Haitao

Great. That is a good reference point. Thank you.

v2 should work too but needs to be checked.

/Jarkko

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-10-05  3:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-05  0:41 [PATCH] x86/sgx: Fix sgx_encl_may_map locking Jarkko Sakkinen
2020-10-05  3:27 ` Haitao Huang
2020-10-05  3:43   ` Jarkko Sakkinen

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