All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <sean.j.christopherson@intel.com>
To: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Cc: linux-sgx@vger.kernel.org,
	Shay Katz-zamir <shay.katz-zamir@intel.com>,
	Serge Ayoun <serge.ayoun@intel.com>
Subject: [PATCH for_v22 07/11] x86/sgx: Check that enclave is created at beginning of EADD/EINIT ioctl
Date: Wed,  7 Aug 2019 17:12:50 -0700	[thread overview]
Message-ID: <20190808001254.11926-8-sean.j.christopherson@intel.com> (raw)
In-Reply-To: <20190808001254.11926-1-sean.j.christopherson@intel.com>

Move the EADD/EINIT checks on SGX_ENCL_CREATED to the very beginning of
the ioctl() flows.  Deferring the check until the core code is fragile
as all code leading up to that point must be careful that it only uses
members of @encl that are initialized at allocation time.  For example,
the flush_work() call in sgx_encl_init() will crash if the enclave has
not been created.

Note, there is no need to take encl->lock to check SGX_ENCL_CREATED so
long as SGX_ENCL_CREATED is set only after the enclave is fully
initialized, it's not the kernel's responsibility to guard against
sgx_encl_create() racing with EADD/EINIT.  Add a comment to highlight
the dependency.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 arch/x86/kernel/cpu/sgx/driver/ioctl.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/driver/ioctl.c b/arch/x86/kernel/cpu/sgx/driver/ioctl.c
index 6a580361e20e..700d65c96b9a 100644
--- a/arch/x86/kernel/cpu/sgx/driver/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/driver/ioctl.c
@@ -326,6 +326,12 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs)
 	encl->base = secs->base;
 	encl->size = secs->size;
 	encl->ssaframesize = secs->ssa_frame_size;
+
+	/*
+	 * Set SGX_ENCL_CREATED only after the enclave is fully prepped.  This
+	 * allows other flows to check if the enclave has been created without
+	 * taking encl->lock.
+	 */
 	encl->flags |= SGX_ENCL_CREATED;
 
 	mutex_unlock(&encl->lock);
@@ -516,8 +522,7 @@ static int sgx_encl_add_page(struct sgx_encl *encl, unsigned long addr,
 
 	mutex_lock(&encl->lock);
 
-	if (!(encl->flags & SGX_ENCL_CREATED) ||
-	    (encl->flags & (SGX_ENCL_INITIALIZED | SGX_ENCL_DEAD))) {
+	if (encl->flags & (SGX_ENCL_INITIALIZED | SGX_ENCL_DEAD)) {
 		ret = -EFAULT;
 		goto out;
 	}
@@ -597,6 +602,9 @@ static long sgx_ioc_enclave_add_page(struct file *filep, void __user *arg)
 	void *data;
 	int ret;
 
+	if (!(encl->flags & SGX_ENCL_CREATED))
+		return -EINVAL;
+
 	if (copy_from_user(&addp, arg, sizeof(addp)))
 		return -EFAULT;
 
@@ -685,8 +693,7 @@ static int sgx_encl_init(struct sgx_encl *encl, struct sgx_sigstruct *sigstruct,
 
 	mutex_lock(&encl->lock);
 
-	if (!(encl->flags & SGX_ENCL_CREATED) ||
-	    (encl->flags & (SGX_ENCL_INITIALIZED | SGX_ENCL_DEAD))) {
+	if (encl->flags & (SGX_ENCL_INITIALIZED | SGX_ENCL_DEAD)) {
 		ret = -EFAULT;
 		goto err_out;
 	}
@@ -753,6 +760,9 @@ static long sgx_ioc_enclave_init(struct file *filep, void __user *arg)
 	struct page *initp_page;
 	int ret;
 
+	if (!(encl->flags & SGX_ENCL_CREATED))
+		return -EINVAL;
+
 	if (copy_from_user(&einit, arg, sizeof(einit)))
 		return -EFAULT;
 
-- 
2.22.0


  parent reply	other threads:[~2019-08-08  0:13 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-08  0:12 [PATCH for_v22 00/11] x86/sgx: Bug fixes for v22 Sean Christopherson
2019-08-08  0:12 ` [PATCH for_v22 01/11] x86/sgx: Fix an SECS collision with enclave page at VA=0 Sean Christopherson
2019-08-08 15:34   ` Jarkko Sakkinen
2019-08-08 15:44     ` Sean Christopherson
2019-08-09 15:13       ` Jarkko Sakkinen
2019-08-09 20:44   ` Jarkko Sakkinen
2019-08-09 20:59     ` Jarkko Sakkinen
2019-08-08  0:12 ` [PATCH for_v22 02/11] x86/sgx: Fix incorrect NULL pointer check Sean Christopherson
2019-08-08 15:36   ` Jarkko Sakkinen
2019-08-09 21:16     ` Jarkko Sakkinen
2019-08-08  0:12 ` [PATCH for_v22 03/11] x86/sgx: Return '0' when sgx_ioc_enclave_set_attribute() succeeds Sean Christopherson
2019-08-08 15:37   ` Jarkko Sakkinen
2019-08-08  0:12 ` [PATCH for_v22 04/11] x86/sgx: x86/sgx: Require EADD destination to be page aligned Sean Christopherson
2019-08-08 15:38   ` Jarkko Sakkinen
2019-08-08  0:12 ` [PATCH for_v22 05/11] x86/sgx: Require EADD source " Sean Christopherson
2019-08-08 15:44   ` Jarkko Sakkinen
2019-08-08  0:12 ` [PATCH for_v22 06/11] x86/sgx: Check the bounds of the enclave address against ELRANGE Sean Christopherson
2019-08-08 15:45   ` Jarkko Sakkinen
2019-08-09 21:21     ` Jarkko Sakkinen
2019-08-08  0:12 ` Sean Christopherson [this message]
2019-08-08 15:47   ` [PATCH for_v22 07/11] x86/sgx: Check that enclave is created at beginning of EADD/EINIT ioctl Jarkko Sakkinen
2019-08-09 23:40   ` Jarkko Sakkinen
2019-08-10  0:03     ` Sean Christopherson
2019-08-10  0:10       ` Sean Christopherson
2019-08-08  0:12 ` [PATCH for_v22 08/11] x86/sgx: Do not free enclave resources on redundant ECREATE Sean Christopherson
2019-08-08 15:48   ` Jarkko Sakkinen
2019-08-08  0:12 ` [PATCH for_v22 09/11] x86/sgx: Refactor error handling for user of sgx_encl_grow() Sean Christopherson
2019-08-08 15:49   ` Jarkko Sakkinen
2019-08-08  0:12 ` [PATCH for_v22 10/11] x86/sgx: Call sgx_encl_grow() with the enclave's lock held Sean Christopherson
2019-08-08 15:52   ` Jarkko Sakkinen
2019-08-08 15:55     ` Sean Christopherson
2019-08-09 16:12       ` Jarkko Sakkinen
2019-08-10 11:32   ` Jarkko Sakkinen
2019-08-08  0:12 ` [PATCH for_v22 11/11] x86/sgx: Shrink the enclave if ECREATE/EADD fails Sean Christopherson
2019-08-08 15:50   ` Jarkko Sakkinen
2019-08-08 18:03     ` Sean Christopherson
2019-08-09 16:13       ` Jarkko Sakkinen
2019-08-10 11:37       ` Jarkko Sakkinen
2019-08-08 15:18 ` [PATCH for_v22 00/11] x86/sgx: Bug fixes for v22 Jarkko Sakkinen
2019-08-08 15:57 ` Jarkko Sakkinen
2019-08-10 11:44 ` Jarkko Sakkinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190808001254.11926-8-sean.j.christopherson@intel.com \
    --to=sean.j.christopherson@intel.com \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=linux-sgx@vger.kernel.org \
    --cc=serge.ayoun@intel.com \
    --cc=shay.katz-zamir@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.