RE: [PATCH v6 10/11] intel_sgx: glue code for in-kernel LE

2017-12-13 Thread Christopherson, Sean J 
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.33]   Not tainted 4.14.0+ #21
[  484.331507] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this 
message.
[  484.332244] 3   D0   972  2 0x
[  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], >attributes, sizeof(uint64_t));
+   if (ret)
+   return ret;
+
+   ret = sgx_le_write(ctx->pipes[0], >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(>lock);
+   ret = __sgx_le_get_token(ctx, encl, sigstruct, token);
+   mutex_unlock(>lock);
+
+   return ret;
+}

RE: [PATCH v6 10/11] intel_sgx: glue code for in-kernel LE

2017-12-13 Thread Christopherson, Sean J 
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.33]   Not tainted 4.14.0+ #21
[  484.331507] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this 
message.
[  484.332244] 3   D0   972  2 0x
[  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], >attributes, sizeof(uint64_t));
+   if (ret)
+   return ret;
+
+   ret = sgx_le_write(ctx->pipes[0], >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(>lock);
+   ret = __sgx_le_get_token(ctx, encl, sigstruct, token);
+   mutex_unlock(>lock);
+
+   return ret;
+}

[PATCH v6 10/11] intel_sgx: glue code for in-kernel LE

2017-11-25 Thread Jarkko Sakkinen 
Glue code for hosting in-kernel Launch Enclave (LE) by using the user
space helper framework.

Tokens for launching enclaves are generated with by the following
protocol:

1. The driver sends a SIGSTRUCT blob to the LE hosting process
   to the input pipe.
2. The LE hosting process reads the SIGSTRUCT blob from the input
   pipe.
3. After generating a EINITTOKEN blob, the LE hosting process writes
   it to the output pipe.
4. The driver reads the EINITTOKEN blob from the output pipe.

If IA32_SGXLEPUBKEYHASH* MSRs are writable and they don't have the
public key hash of the LE they will be updated.

Signed-off-by: Jarkko Sakkinen 
---
 drivers/platform/x86/intel_sgx/Kconfig |   2 +
 drivers/platform/x86/intel_sgx/Makefile|   1 +
 drivers/platform/x86/intel_sgx/sgx.h   |  24 ++
 drivers/platform/x86/intel_sgx/sgx_encl.c  |  18 ++
 drivers/platform/x86/intel_sgx/sgx_ioctl.c |   4 +-
 drivers/platform/x86/intel_sgx/sgx_le.c| 319 +
 .../platform/x86/intel_sgx/sgx_le_proxy_piggy.S|   4 +
 drivers/platform/x86/intel_sgx/sgx_main.c  |  49 +++-
 drivers/platform/x86/intel_sgx/sgx_util.c  |  25 ++
 9 files changed, 442 insertions(+), 4 deletions(-)
 create mode 100644 drivers/platform/x86/intel_sgx/sgx_le.c

diff --git a/drivers/platform/x86/intel_sgx/Kconfig 
b/drivers/platform/x86/intel_sgx/Kconfig
index 050861bb5229..a0daccbc1036 100644
--- a/drivers/platform/x86/intel_sgx/Kconfig
+++ b/drivers/platform/x86/intel_sgx/Kconfig
@@ -9,6 +9,8 @@ config INTEL_SGX
default n
depends on X86_64 && CPU_SUP_INTEL
select MMU_NOTIFIER
+   select CRYPTO
+   select CRYPTO_SHA256
---help---
Intel(R) SGX is a set of CPU instructions that can be used by
applications to set aside private regions of code and data.  The code
diff --git a/drivers/platform/x86/intel_sgx/Makefile 
b/drivers/platform/x86/intel_sgx/Makefile
index fc76f3c923ce..7ca1a7c6ab77 100644
--- a/drivers/platform/x86/intel_sgx/Makefile
+++ b/drivers/platform/x86/intel_sgx/Makefile
@@ -11,6 +11,7 @@ intel_sgx-$(CONFIG_INTEL_SGX) += \
sgx_page_cache.o \
sgx_util.o \
sgx_vma.o \
+   sgx_le.o \
sgx_le_proxy_piggy.o
 
 $(eval $(call config_filename,INTEL_SGX_SIGNING_KEY))
diff --git a/drivers/platform/x86/intel_sgx/sgx.h 
b/drivers/platform/x86/intel_sgx/sgx.h
index 384e25244d92..d0cb8d5dd53a 100644
--- a/drivers/platform/x86/intel_sgx/sgx.h
+++ b/drivers/platform/x86/intel_sgx/sgx.h
@@ -60,8 +60,11 @@
 #ifndef __ARCH_INTEL_SGX_H__
 #define __ARCH_INTEL_SGX_H__
 
+#include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -161,13 +164,19 @@ struct sgx_encl {
struct mmu_notifier mmu_notifier;
 };
 
+extern unsigned char sgx_le_proxy[];
+extern unsigned char sgx_le_proxy_end[];
+extern struct sgx_sigstruct sgx_le_ss;
 extern struct workqueue_struct *sgx_add_page_wq;
 extern u64 sgx_encl_size_max_32;
 extern u64 sgx_encl_size_max_64;
 extern u64 sgx_xfrm_mask;
 extern u32 sgx_misc_reserved;
 extern u32 sgx_xsave_size_tbl[64];
+extern u64 sgx_le_pubkeyhash[4];
+extern bool sgx_unlocked_msrs;
 
+extern const struct file_operations sgx_fops;
 extern const struct vm_operations_struct sgx_vm_ops;
 
 #define sgx_pr_ratelimited(level, encl, fmt, ...)\
@@ -227,6 +236,9 @@ struct sgx_encl_page *sgx_fault_page(struct vm_area_struct 
*vma,
 unsigned int flags);
 
 
+int sgx_get_key_hash(struct crypto_shash *tfm, const void *modulus, void 
*hash);
+int sgx_get_key_hash_simple(const void *modulus, void *hash);
+
 extern struct mutex sgx_tgid_ctx_mutex;
 extern struct list_head sgx_tgid_ctx_list;
 extern atomic_t sgx_va_pages_cnt;
@@ -241,4 +253,16 @@ void sgx_put_page(void *ptr);
 void sgx_eblock(struct sgx_encl *encl, void *epc_page);
 void sgx_etrack(struct sgx_encl *encl);
 
+extern struct sgx_le_ctx sgx_le_ctx;
+
+int sgx_le_init(struct sgx_le_ctx *ctx);
+void sgx_le_exit(struct sgx_le_ctx *ctx);
+void sgx_le_stop(struct sgx_le_ctx *ctx);
+int sgx_le_start(struct sgx_le_ctx *ctx);
+
+int sgx_le_get_token(struct sgx_le_ctx *ctx,
+const struct sgx_encl *encl,
+const struct sgx_sigstruct *sigstruct,
+struct sgx_einittoken *token);
+
 #endif /* __ARCH_X86_INTEL_SGX_H__ */
diff --git a/drivers/platform/x86/intel_sgx/sgx_encl.c 
b/drivers/platform/x86/intel_sgx/sgx_encl.c
index 57dc1f90f4dc..0ffae3c02342 100644
--- a/drivers/platform/x86/intel_sgx/sgx_encl.c
+++ b/drivers/platform/x86/intel_sgx/sgx_encl.c
@@ -866,6 +866,14 @@ static int sgx_einit(struct sgx_encl *encl, struct 
sgx_sigstruct *sigstruct,
return ret;
 }
 
+static void sgx_update_pubkeyhash(void)
+{
+   wrmsrl(MSR_IA32_SGXLEPUBKEYHASH0, sgx_le_pubkeyhash[0]);
+   wrmsrl(MSR_IA32_SGXLEPUBKEYHASH1, sgx_le_pubkeyhash[1]);
+

[PATCH v6 10/11] intel_sgx: glue code for in-kernel LE

2017-11-25 Thread Jarkko Sakkinen 
Glue code for hosting in-kernel Launch Enclave (LE) by using the user
space helper framework.

Tokens for launching enclaves are generated with by the following
protocol:

1. The driver sends a SIGSTRUCT blob to the LE hosting process
   to the input pipe.
2. The LE hosting process reads the SIGSTRUCT blob from the input
   pipe.
3. After generating a EINITTOKEN blob, the LE hosting process writes
   it to the output pipe.
4. The driver reads the EINITTOKEN blob from the output pipe.

If IA32_SGXLEPUBKEYHASH* MSRs are writable and they don't have the
public key hash of the LE they will be updated.

Signed-off-by: Jarkko Sakkinen 
---
 drivers/platform/x86/intel_sgx/Kconfig |   2 +
 drivers/platform/x86/intel_sgx/Makefile|   1 +
 drivers/platform/x86/intel_sgx/sgx.h   |  24 ++
 drivers/platform/x86/intel_sgx/sgx_encl.c  |  18 ++
 drivers/platform/x86/intel_sgx/sgx_ioctl.c |   4 +-
 drivers/platform/x86/intel_sgx/sgx_le.c| 319 +
 .../platform/x86/intel_sgx/sgx_le_proxy_piggy.S|   4 +
 drivers/platform/x86/intel_sgx/sgx_main.c  |  49 +++-
 drivers/platform/x86/intel_sgx/sgx_util.c  |  25 ++
 9 files changed, 442 insertions(+), 4 deletions(-)
 create mode 100644 drivers/platform/x86/intel_sgx/sgx_le.c

diff --git a/drivers/platform/x86/intel_sgx/Kconfig 
b/drivers/platform/x86/intel_sgx/Kconfig
index 050861bb5229..a0daccbc1036 100644
--- a/drivers/platform/x86/intel_sgx/Kconfig
+++ b/drivers/platform/x86/intel_sgx/Kconfig
@@ -9,6 +9,8 @@ config INTEL_SGX
default n
depends on X86_64 && CPU_SUP_INTEL
select MMU_NOTIFIER
+   select CRYPTO
+   select CRYPTO_SHA256
---help---
Intel(R) SGX is a set of CPU instructions that can be used by
applications to set aside private regions of code and data.  The code
diff --git a/drivers/platform/x86/intel_sgx/Makefile 
b/drivers/platform/x86/intel_sgx/Makefile
index fc76f3c923ce..7ca1a7c6ab77 100644
--- a/drivers/platform/x86/intel_sgx/Makefile
+++ b/drivers/platform/x86/intel_sgx/Makefile
@@ -11,6 +11,7 @@ intel_sgx-$(CONFIG_INTEL_SGX) += \
sgx_page_cache.o \
sgx_util.o \
sgx_vma.o \
+   sgx_le.o \
sgx_le_proxy_piggy.o
 
 $(eval $(call config_filename,INTEL_SGX_SIGNING_KEY))
diff --git a/drivers/platform/x86/intel_sgx/sgx.h 
b/drivers/platform/x86/intel_sgx/sgx.h
index 384e25244d92..d0cb8d5dd53a 100644
--- a/drivers/platform/x86/intel_sgx/sgx.h
+++ b/drivers/platform/x86/intel_sgx/sgx.h
@@ -60,8 +60,11 @@
 #ifndef __ARCH_INTEL_SGX_H__
 #define __ARCH_INTEL_SGX_H__
 
+#include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -161,13 +164,19 @@ struct sgx_encl {
struct mmu_notifier mmu_notifier;
 };
 
+extern unsigned char sgx_le_proxy[];
+extern unsigned char sgx_le_proxy_end[];
+extern struct sgx_sigstruct sgx_le_ss;
 extern struct workqueue_struct *sgx_add_page_wq;
 extern u64 sgx_encl_size_max_32;
 extern u64 sgx_encl_size_max_64;
 extern u64 sgx_xfrm_mask;
 extern u32 sgx_misc_reserved;
 extern u32 sgx_xsave_size_tbl[64];
+extern u64 sgx_le_pubkeyhash[4];
+extern bool sgx_unlocked_msrs;
 
+extern const struct file_operations sgx_fops;
 extern const struct vm_operations_struct sgx_vm_ops;
 
 #define sgx_pr_ratelimited(level, encl, fmt, ...)\
@@ -227,6 +236,9 @@ struct sgx_encl_page *sgx_fault_page(struct vm_area_struct 
*vma,
 unsigned int flags);
 
 
+int sgx_get_key_hash(struct crypto_shash *tfm, const void *modulus, void 
*hash);
+int sgx_get_key_hash_simple(const void *modulus, void *hash);
+
 extern struct mutex sgx_tgid_ctx_mutex;
 extern struct list_head sgx_tgid_ctx_list;
 extern atomic_t sgx_va_pages_cnt;
@@ -241,4 +253,16 @@ void sgx_put_page(void *ptr);
 void sgx_eblock(struct sgx_encl *encl, void *epc_page);
 void sgx_etrack(struct sgx_encl *encl);
 
+extern struct sgx_le_ctx sgx_le_ctx;
+
+int sgx_le_init(struct sgx_le_ctx *ctx);
+void sgx_le_exit(struct sgx_le_ctx *ctx);
+void sgx_le_stop(struct sgx_le_ctx *ctx);
+int sgx_le_start(struct sgx_le_ctx *ctx);
+
+int sgx_le_get_token(struct sgx_le_ctx *ctx,
+const struct sgx_encl *encl,
+const struct sgx_sigstruct *sigstruct,
+struct sgx_einittoken *token);
+
 #endif /* __ARCH_X86_INTEL_SGX_H__ */
diff --git a/drivers/platform/x86/intel_sgx/sgx_encl.c 
b/drivers/platform/x86/intel_sgx/sgx_encl.c
index 57dc1f90f4dc..0ffae3c02342 100644
--- a/drivers/platform/x86/intel_sgx/sgx_encl.c
+++ b/drivers/platform/x86/intel_sgx/sgx_encl.c
@@ -866,6 +866,14 @@ static int sgx_einit(struct sgx_encl *encl, struct 
sgx_sigstruct *sigstruct,
return ret;
 }
 
+static void sgx_update_pubkeyhash(void)
+{
+   wrmsrl(MSR_IA32_SGXLEPUBKEYHASH0, sgx_le_pubkeyhash[0]);
+   wrmsrl(MSR_IA32_SGXLEPUBKEYHASH1, sgx_le_pubkeyhash[1]);
+   wrmsrl(MSR_IA32_SGXLEPUBKEYHASH2,