diff mbox

[intel-sgx-kernel-dev] intel_sgx: dec ctx->epc_cnt iff __eremove succeeds

Message ID 1490367163-25400-1-git-send-email-sean.j.christopherson@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sean Christopherson March 24, 2017, 2:52 p.m. UTC
sgx_free_page decrements ctx->epc_cnt prior to calling __eremove.  If
ERMOVE fails and the caller passed SGX_FREE_RETURN_ERROR then epc_cnt
will be decremented without actually freeing the page.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 drivers/platform/x86/intel_sgx_page_cache.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Jarkko Sakkinen March 25, 2017, 7:56 p.m. UTC | #1
On Fri, Mar 24, 2017 at 07:52:43AM -0700, Sean Christopherson wrote:
> sgx_free_page decrements ctx->epc_cnt prior to calling __eremove.  If
> ERMOVE fails and the caller passed SGX_FREE_RETURN_ERROR then epc_cnt
> will be decremented without actually freeing the page.
> 
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>

Thanks. I would merge these "slowly" i.e. lets get through the other
fix that you send that needed some minor refining and then move to this
one.

After these two fixes have been landed I can look into three other
patches.

/Jarkko

> ---
>  drivers/platform/x86/intel_sgx_page_cache.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c
> index e4f6b95..852066c 100644
> --- a/drivers/platform/x86/intel_sgx_page_cache.c
> +++ b/drivers/platform/x86/intel_sgx_page_cache.c
> @@ -539,8 +539,6 @@ int sgx_free_page(struct sgx_epc_page *entry, struct sgx_encl *encl,
>  	void *epc;
>  	int ret;
>  
> -	atomic_dec(&encl->tgid_ctx->epc_cnt);
> -
>  	epc = sgx_get_epc_page(entry);
>  	ret = __eremove(epc);
>  	sgx_put_epc_page(epc);
> @@ -552,6 +550,8 @@ int sgx_free_page(struct sgx_epc_page *entry, struct sgx_encl *encl,
>  		sgx_crit(encl, "EREMOVE returned %d\n", ret);
>  	}
>  
> +	atomic_dec(&encl->tgid_ctx->epc_cnt);
> +
>  	spin_lock(&sgx_free_list_lock);
>  	list_add(&entry->free_list, &sgx_free_list);
>  	sgx_nr_free_pages++;
> -- 
> 2.7.4
> 
> _______________________________________________
> intel-sgx-kernel-dev mailing list
> intel-sgx-kernel-dev@lists.01.org
> https://lists.01.org/mailman/listinfo/intel-sgx-kernel-dev
Jarkko Sakkinen March 25, 2017, 8:01 p.m. UTC | #2
On Sat, Mar 25, 2017 at 09:56:51PM +0200, Jarkko Sakkinen wrote:
> On Fri, Mar 24, 2017 at 07:52:43AM -0700, Sean Christopherson wrote:
> > sgx_free_page decrements ctx->epc_cnt prior to calling __eremove.  If
> > ERMOVE fails and the caller passed SGX_FREE_RETURN_ERROR then epc_cnt
> > will be decremented without actually freeing the page.
> > 
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> 
> Thanks. I would merge these "slowly" i.e. lets get through the other
> fix that you send that needed some minor refining and then move to this
> one.
> 
> After these two fixes have been landed I can look into three other
> patches.
> 
> /Jarkko

This one was such a no-brainer so that I just merged it.

/Jarkko

> 
> > ---
> >  drivers/platform/x86/intel_sgx_page_cache.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c
> > index e4f6b95..852066c 100644
> > --- a/drivers/platform/x86/intel_sgx_page_cache.c
> > +++ b/drivers/platform/x86/intel_sgx_page_cache.c
> > @@ -539,8 +539,6 @@ int sgx_free_page(struct sgx_epc_page *entry, struct sgx_encl *encl,
> >  	void *epc;
> >  	int ret;
> >  
> > -	atomic_dec(&encl->tgid_ctx->epc_cnt);
> > -
> >  	epc = sgx_get_epc_page(entry);
> >  	ret = __eremove(epc);
> >  	sgx_put_epc_page(epc);
> > @@ -552,6 +550,8 @@ int sgx_free_page(struct sgx_epc_page *entry, struct sgx_encl *encl,
> >  		sgx_crit(encl, "EREMOVE returned %d\n", ret);
> >  	}
> >  
> > +	atomic_dec(&encl->tgid_ctx->epc_cnt);
> > +
> >  	spin_lock(&sgx_free_list_lock);
> >  	list_add(&entry->free_list, &sgx_free_list);
> >  	sgx_nr_free_pages++;
> > -- 
> > 2.7.4
> > 
> > _______________________________________________
> > intel-sgx-kernel-dev mailing list
> > intel-sgx-kernel-dev@lists.01.org
> > https://lists.01.org/mailman/listinfo/intel-sgx-kernel-dev
diff mbox

Patch

diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c
index e4f6b95..852066c 100644
--- a/drivers/platform/x86/intel_sgx_page_cache.c
+++ b/drivers/platform/x86/intel_sgx_page_cache.c
@@ -539,8 +539,6 @@  int sgx_free_page(struct sgx_epc_page *entry, struct sgx_encl *encl,
 	void *epc;
 	int ret;
 
-	atomic_dec(&encl->tgid_ctx->epc_cnt);
-
 	epc = sgx_get_epc_page(entry);
 	ret = __eremove(epc);
 	sgx_put_epc_page(epc);
@@ -552,6 +550,8 @@  int sgx_free_page(struct sgx_epc_page *entry, struct sgx_encl *encl,
 		sgx_crit(encl, "EREMOVE returned %d\n", ret);
 	}
 
+	atomic_dec(&encl->tgid_ctx->epc_cnt);
+
 	spin_lock(&sgx_free_list_lock);
 	list_add(&entry->free_list, &sgx_free_list);
 	sgx_nr_free_pages++;