Linux-Sgx Archive on lore.kernel.org
 help / color / 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
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 index

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 publically 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

Linux-Sgx Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-sgx/0 linux-sgx/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-sgx linux-sgx/ https://lore.kernel.org/linux-sgx \
		linux-sgx@vger.kernel.org linux-sgx@archiver.kernel.org
	public-inbox-index linux-sgx


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-sgx


AGPL code for this site: git clone https://public-inbox.org/ public-inbox