[intel-sgx-kernel-dev] intel_sgx: check ctx.refcnt != 0 in sgx_add_to_tgid_ctx
diff mbox

Message ID 1485450995-23780-1-git-send-email-sean.j.christopherson@intel.com
State New
Headers show

Commit Message

Sean Christopherson Jan. 26, 2017, 5:16 p.m. UTC
Use kref_get_unless_zero() when attempting to add an enclave to an
existing sgx_tgid_ctx to ensure the ctx is not about to be released.
A process may destroy its last enclave while simultaneously creating
a new enclave, resulting in the ctx's refcnt decrementing to zero as
the new enclave is querying sgx_tgid_ctx_list.

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

Comments

Jarkko Sakkinen Jan. 27, 2017, 6:27 a.m. UTC | #1
On Thu, Jan 26, 2017 at 09:16:35AM -0800, Sean Christopherson wrote:
> Use kref_get_unless_zero() when attempting to add an enclave to an
> existing sgx_tgid_ctx to ensure the ctx is not about to be released.
> A process may destroy its last enclave while simultaneously creating
> a new enclave, resulting in the ctx's refcnt decrementing to zero as
> the new enclave is querying sgx_tgid_ctx_list.
> 
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>

LGTM

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

/Jarkko

> ---
>  drivers/platform/x86/intel_sgx_ioctl.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel_sgx_ioctl.c b/drivers/platform/x86/intel_sgx_ioctl.c
> index f2cc2c1..8c4330f 100644
> --- a/drivers/platform/x86/intel_sgx_ioctl.c
> +++ b/drivers/platform/x86/intel_sgx_ioctl.c
> @@ -98,8 +98,7 @@ static int sgx_add_to_tgid_ctx(struct sgx_encl *encl)
>  	mutex_lock(&sgx_tgid_ctx_mutex);
>  
>  	ctx = sgx_find_tgid_ctx(tgid);
> -	if (ctx) {
> -		kref_get(&ctx->refcount);
> +	if (ctx && kref_get_unless_zero(&ctx->refcount)) {
>  		encl->tgid_ctx = ctx;
>  		mutex_unlock(&sgx_tgid_ctx_mutex);
>  		put_pid(tgid);
> -- 
> 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 Feb. 1, 2017, 8:05 p.m. UTC | #2
On Fri, Jan 27, 2017 at 08:27:11AM +0200, Jarkko Sakkinen wrote:
> On Thu, Jan 26, 2017 at 09:16:35AM -0800, Sean Christopherson wrote:
> > Use kref_get_unless_zero() when attempting to add an enclave to an
> > existing sgx_tgid_ctx to ensure the ctx is not about to be released.
> > A process may destroy its last enclave while simultaneously creating
> > a new enclave, resulting in the ctx's refcnt decrementing to zero as
> > the new enclave is querying sgx_tgid_ctx_list.
> > 
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> 
> LGTM
> 
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

I just noticed that I hadn't applied this. Now it's pushed. Sorry
about that!

/Jarkko

Patch
diff mbox

diff --git a/drivers/platform/x86/intel_sgx_ioctl.c b/drivers/platform/x86/intel_sgx_ioctl.c
index f2cc2c1..8c4330f 100644
--- a/drivers/platform/x86/intel_sgx_ioctl.c
+++ b/drivers/platform/x86/intel_sgx_ioctl.c
@@ -98,8 +98,7 @@  static int sgx_add_to_tgid_ctx(struct sgx_encl *encl)
 	mutex_lock(&sgx_tgid_ctx_mutex);
 
 	ctx = sgx_find_tgid_ctx(tgid);
-	if (ctx) {
-		kref_get(&ctx->refcount);
+	if (ctx && kref_get_unless_zero(&ctx->refcount)) {
 		encl->tgid_ctx = ctx;
 		mutex_unlock(&sgx_tgid_ctx_mutex);
 		put_pid(tgid);