linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Christopherson, Sean J" <sean.j.christopherson@intel.com>
To: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	"platform-driver-x86@vger.kernel.org" 
	<platform-driver-x86@vger.kernel.org>,
	"x86@kernel.org" <x86@kernel.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Darren Hart <dvhart@infradead.org>,
	Andy Shevchenko <andy@infradead.org>
Subject: RE: [PATCH v6 10/11] intel_sgx: glue code for in-kernel LE
Date: Wed, 13 Dec 2017 23:34:43 +0000	[thread overview]
Message-ID: <37306EFA9975BE469F115FDE982C075BC6B3B619@ORSMSX108.amr.corp.intel.com> (raw)
In-Reply-To: <20171125193132.24321-11-jarkko.sakkinen@linux.intel.com>

On Sat, Nov 25, 2017 at 09:29:27PM +0200, Jarkko Sakkinen wrote:
+static int __sgx_le_get_token(struct sgx_le_ctx *ctx,
+			      const struct sgx_encl *encl,
+			      const struct sgx_sigstruct *sigstruct,
+			      struct sgx_einittoken *token)
+{
+	u8 mrsigner[32];
+	ssize_t ret;
+
+	if (!ctx->tgid)
+		return -EIO;
+
+	ret = sgx_get_key_hash(ctx->tfm, sigstruct->modulus, mrsigner);
+	if (ret)
+		return ret;
+
+	if (!memcmp(mrsigner, sgx_le_pubkeyhash, 32))
+		return 0;

Not sure what other code would need to be juggled to make it happen,
but checking the enclave's signer against sgx_le_pubkeyhash needs to
be moved outside of the sgx_le_ctx mutex, e.g. to sgx_le_get_token.
Deadlocks can occur because the kernel's LE uses the same EINIT flow
as normal enclaves.  If a userspace enclave is created immediately
after opening /dev/sgx it can race with the LE to acquire the lock.
If the userspace enclave wins, it will block indefinitely waiting
for the LE to generate the token, while the LE is blocked waiting
for the lock.

This bug is trivial to reproduce on a single threaded system/VM.

Hung task trace for the in-kernel enclave:

[  484.330510] INFO: task 3:972 blocked for more than 120 seconds.
[  484.331111]       Not tainted 4.14.0+ #21
[  484.331507] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  484.332244] 3               D    0   972      2 0x00000000
[  484.332248] Call Trace:
[  484.332265]  __schedule+0x3c2/0x890
[  484.332267]  schedule+0x36/0x80
[  484.332268]  schedule_preempt_disabled+0xe/0x10
[  484.332269]  __mutex_lock.isra.2+0x2b1/0x4e0
[  484.332274]  __mutex_lock_slowpath+0x13/0x20
[  484.332276]  mutex_lock+0x2f/0x40
[  484.332278]  sgx_le_get_token+0x3e/0x166 [intel_sgx]
[  484.332282]  sgx_ioc_enclave_init+0xd2/0x120 [intel_sgx]
[  484.332283]  sgx_ioctl+0xdc/0x180 [intel_sgx]
[  484.332306]  do_vfs_ioctl+0xa1/0x5e0
[  484.332308]  SyS_ioctl+0x79/0x90
[  484.332310]  entry_SYSCALL_64_fastpath+0x1e/0xa9


+
+	ret = sgx_le_write(ctx->pipes[0], sigstruct->body.mrenclave, 32);
+	if (ret)
+		return ret;
+
+	ret = sgx_le_write(ctx->pipes[0], mrsigner, 32);
+	if (ret)
+		return ret;
+
+	ret = sgx_le_write(ctx->pipes[0], &encl->attributes, sizeof(uint64_t));
+	if (ret)
+		return ret;
+
+	ret = sgx_le_write(ctx->pipes[0], &encl->xfrm, sizeof(uint64_t));
+	if (ret)
+		return ret;
+
+	return sgx_le_read(ctx->pipes[1], token, sizeof(*token));
+}
+
+int sgx_le_get_token(struct sgx_le_ctx *ctx,
+		     const struct sgx_encl *encl,
+		     const struct sgx_sigstruct *sigstruct,
+		     struct sgx_einittoken *token)
+{
+	int ret;
+
+	mutex_lock(&ctx->lock);
+	ret = __sgx_le_get_token(ctx, encl, sigstruct, token);
+	mutex_unlock(&ctx->lock);
+
+	return ret;
+}

  reply	other threads:[~2017-12-13 23:34 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-25 19:29 [PATCH v6 00/11] Intel SGX Driver Jarkko Sakkinen
2017-11-25 19:29 ` [PATCH v6 01/11] intel_sgx: updated MAINTAINERS Jarkko Sakkinen
2017-11-25 20:19   ` Joe Perches
2017-11-26 10:41     ` Bjørn Mork
2017-11-26 15:33       ` Jarkko Sakkinen
2017-11-25 19:29 ` [PATCH v6 02/11] x86: add SGX definition to cpufeature Jarkko Sakkinen
2017-11-25 19:29 ` [PATCH v6 03/11] x86: define IA32_FEATURE_CONTROL.SGX_ENABLE Jarkko Sakkinen
2017-11-28 17:13   ` Sean Christopherson
2017-11-28 20:47     ` Jarkko Sakkinen
2017-11-25 19:29 ` [PATCH v6 04/11] x86: define IA32_FEATUE_CONTROL.SGX_LC Jarkko Sakkinen
2017-11-28 17:16   ` Sean Christopherson
2017-11-28 18:28     ` Sean Christopherson
2017-11-28 20:53       ` Jarkko Sakkinen
2017-11-28 21:24         ` Jarkko Sakkinen
2017-11-28 21:33           ` Sean Christopherson
2017-11-28 21:55             ` Jarkko Sakkinen
2017-11-28 22:00               ` Sean Christopherson
2017-11-28 22:21                 ` Jarkko Sakkinen
2017-11-29 15:38                   ` Jarkko Sakkinen
2017-11-30  3:05                     ` Kai Huang
2017-11-30 16:44                       ` Jarkko Sakkinen
2017-11-28 21:40           ` Jarkko Sakkinen
2017-11-28 21:44             ` Sean Christopherson
2017-11-28 22:03               ` Jarkko Sakkinen
2017-11-28 22:11                 ` Jarkko Sakkinen
2017-11-28 20:51     ` Jarkko Sakkinen
2017-11-25 19:29 ` [PATCH v6 05/11] x86: add SGX MSRs to msr-index.h Jarkko Sakkinen
2017-11-28 17:22   ` Sean Christopherson
2017-11-28 20:48     ` Jarkko Sakkinen
2017-11-25 19:29 ` [PATCH v6 06/11] intel_sgx: driver for Intel Software Guard Extensions Jarkko Sakkinen
2017-11-26 17:33   ` Jarkko Sakkinen
2017-11-26 18:46     ` Jarkko Sakkinen
2017-11-26 19:01       ` Jarkko Sakkinen
2017-11-29 18:02         ` Jarkko Sakkinen
2017-11-28 19:07   ` Sean Christopherson
2017-11-30 17:32   ` Sean Christopherson
2017-12-04  8:59     ` Jarkko Sakkinen
2017-11-25 19:29 ` [PATCH v6 07/11] intel_sgx: ptrace() support Jarkko Sakkinen
2017-11-26 15:44   ` Jarkko Sakkinen
2017-11-26 15:50     ` Joe Perches
2017-11-26 16:53       ` Jarkko Sakkinen
2017-11-26 17:01   ` Jarkko Sakkinen
2017-11-25 19:29 ` [PATCH v6 08/11] intel_sgx: in-kernel launch enclave Jarkko Sakkinen
2017-11-27  9:58   ` Till Smejkal
2017-11-28 20:17     ` Jarkko Sakkinen
2017-11-28 22:38   ` Jarkko Sakkinen
2017-12-04  9:23     ` Jarkko Sakkinen
2017-11-25 19:29 ` [PATCH v6 09/11] fs/pipe.c: export create_pipe_files() and replace_fd() Jarkko Sakkinen
2017-11-28 14:35   ` Christoph Hellwig
2017-11-28 20:42     ` Jarkko Sakkinen
2017-11-28 21:05       ` Christoph Hellwig
2017-11-28 21:57         ` Jarkko Sakkinen
2017-11-29 23:13           ` Christoph Hellwig
2017-11-30 16:43             ` Jarkko Sakkinen
2017-11-30 18:38               ` James Bottomley
2017-12-04  9:00                 ` Jarkko Sakkinen
2017-12-07 17:37                   ` Jarkko Sakkinen
2017-12-08 13:05                     ` Jarkko Sakkinen
2017-11-25 19:29 ` [PATCH v6 10/11] intel_sgx: glue code for in-kernel LE Jarkko Sakkinen
2017-12-13 23:34   ` Christopherson, Sean J [this message]
2017-11-25 19:29 ` [PATCH v6 11/11] intel_sgx: driver documentation Jarkko Sakkinen
2017-12-12 14:07 ` [PATCH v6 00/11] Intel SGX Driver Pavel Machek
2017-12-14 11:18   ` Jarkko Sakkinen
2017-12-19 23:33   ` Jarkko Sakkinen
2017-12-20 13:18     ` Jarkko Sakkinen
2018-01-04 14:17 ` Cedric Blancher
2018-01-04 14:27   ` Greg Kroah-Hartman
2018-01-04 19:18     ` Ozgur
2018-01-04 15:08   ` James Bottomley
2018-01-09 14:27   ` Jarkko Sakkinen
2018-02-08  8:46     ` Pavel Machek
2018-02-08 13:48       ` 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=37306EFA9975BE469F115FDE982C075BC6B3B619@ORSMSX108.amr.corp.intel.com \
    --to=sean.j.christopherson@intel.com \
    --cc=andy@infradead.org \
    --cc=dvhart@infradead.org \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=x86@kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).