| Message ID | 1490793956-555-1-git-send-email-sean.j.christopherson@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Christopherson, Sean J <sean.j.christopherson@intel.com> wrote: > If a ctx whose refcount has gone to zero is encountered when adding > an encl to the appropriate ctx, then delete the ctx from the global > list prior to adding a new ctx. It is possible for multiple encls > to call sgx_add_to_tgid_ctx while sgx_tgid_ctx_release is waiting to > acquire sgx_tgid_ctx_mutex. If the existing ctx is not deleted from > the list, subsequent calls to sgx_add_to_tgid_ctx will see the dying > ctx instead of the new ctx and will add their own redundant instance > of ctx. > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> Just realized the new context is added to the head and not the tail, so deleting the dying context is not absolutely necessary (and the commit message is not accurate). Deleting the old context may be good for robustness, though. > --- > drivers/platform/x86/intel_sgx_ioctl.c | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/drivers/platform/x86/intel_sgx_ioctl.c > b/drivers/platform/x86/intel_sgx_ioctl.c index e0e2f14..9ead1b9 100644 > --- a/drivers/platform/x86/intel_sgx_ioctl.c > +++ b/drivers/platform/x86/intel_sgx_ioctl.c > @@ -99,11 +99,15 @@ 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_unless_zero(&ctx->refcount)) { > - encl->tgid_ctx = ctx; > - mutex_unlock(&sgx_tgid_ctx_mutex); > - put_pid(tgid); > - return 0; > + if (ctx) { > + if (kref_get_unless_zero(&ctx->refcount)) { > + encl->tgid_ctx = ctx; > + mutex_unlock(&sgx_tgid_ctx_mutex); > + put_pid(tgid); > + return 0; > + } > + else > + list_del_init(&ctx->list); > } > > ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); > -- > 2.7.4 >
On Wed, Mar 29, 2017 at 06:25:56AM -0700, Sean Christopherson wrote: > If a ctx whose refcount has gone to zero is encountered when adding > an encl to the appropriate ctx, then delete the ctx from the global > list prior to adding a new ctx. It is possible for multiple encls > to call sgx_add_to_tgid_ctx while sgx_tgid_ctx_release is waiting to > acquire sgx_tgid_ctx_mutex. If the existing ctx is not deleted from > the list, subsequent calls to sgx_add_to_tgid_ctx will see the dying > ctx instead of the new ctx and will add their own redundant instance > of ctx. > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> Thanks, good catch. /Jarkko > --- > drivers/platform/x86/intel_sgx_ioctl.c | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/drivers/platform/x86/intel_sgx_ioctl.c b/drivers/platform/x86/intel_sgx_ioctl.c > index e0e2f14..9ead1b9 100644 > --- a/drivers/platform/x86/intel_sgx_ioctl.c > +++ b/drivers/platform/x86/intel_sgx_ioctl.c > @@ -99,11 +99,15 @@ 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_unless_zero(&ctx->refcount)) { > - encl->tgid_ctx = ctx; > - mutex_unlock(&sgx_tgid_ctx_mutex); > - put_pid(tgid); > - return 0; > + if (ctx) { > + if (kref_get_unless_zero(&ctx->refcount)) { > + encl->tgid_ctx = ctx; > + mutex_unlock(&sgx_tgid_ctx_mutex); > + put_pid(tgid); > + return 0; > + } > + else > + list_del_init(&ctx->list); > } > > ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); > -- > 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 --git a/drivers/platform/x86/intel_sgx_ioctl.c b/drivers/platform/x86/intel_sgx_ioctl.c index e0e2f14..9ead1b9 100644 --- a/drivers/platform/x86/intel_sgx_ioctl.c +++ b/drivers/platform/x86/intel_sgx_ioctl.c @@ -99,11 +99,15 @@ 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_unless_zero(&ctx->refcount)) { - encl->tgid_ctx = ctx; - mutex_unlock(&sgx_tgid_ctx_mutex); - put_pid(tgid); - return 0; + if (ctx) { + if (kref_get_unless_zero(&ctx->refcount)) { + encl->tgid_ctx = ctx; + mutex_unlock(&sgx_tgid_ctx_mutex); + put_pid(tgid); + return 0; + } + else + list_del_init(&ctx->list); } ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
If a ctx whose refcount has gone to zero is encountered when adding an encl to the appropriate ctx, then delete the ctx from the global list prior to adding a new ctx. It is possible for multiple encls to call sgx_add_to_tgid_ctx while sgx_tgid_ctx_release is waiting to acquire sgx_tgid_ctx_mutex. If the existing ctx is not deleted from the list, subsequent calls to sgx_add_to_tgid_ctx will see the dying ctx instead of the new ctx and will add their own redundant instance of ctx. Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> --- drivers/platform/x86/intel_sgx_ioctl.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)