diff mbox

[intel-sgx-kernel-dev,RFC,10/10] intel_sgx: update IA32_SGXLEPUBKEYHASH* MSRs

Message ID 20170901173002.8442-11-jarkko.sakkinen@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jarkko Sakkinen Sept. 1, 2017, 5:30 p.m. UTC 
Check if IA32_SGXLEPUBKEYHASH* MSRs match. If they do not match, allow
the driver initialization to continue only if they are writable. In this
case update them with the MRSIGNER of the launch enclave.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/platform/x86/intel_sgx/sgx.h               |  3 ++
 drivers/platform/x86/intel_sgx/sgx_encl.c          | 45 ++++++++++++++++++++++
 .../platform/x86/intel_sgx/sgx_le_proxy_piggy.S    |  4 ++
 drivers/platform/x86/intel_sgx/sgx_main.c          | 26 +++++++++++++
 4 files changed, 78 insertions(+)

Comments

Jarkko Sakkinen Sept. 5, 2017, 4:49 a.m. UTC | #1 
On Fri, Sep 01, 2017 at 08:30:02PM +0300, Jarkko Sakkinen wrote:
> Check if IA32_SGXLEPUBKEYHASH* MSRs match. If they do not match, allow
> the driver initialization to continue only if they are writable. In this
> case update them with the MRSIGNER of the launch enclave.
> 
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

This commit is not right. I've forgot to squash some fixups.

Today I have to prepare some slides to LPC an LSS and do TPM
maintainer tasks. I'll sent updated patch set tmrw.

/Jarkko

> ---
>  drivers/platform/x86/intel_sgx/sgx.h               |  3 ++
>  drivers/platform/x86/intel_sgx/sgx_encl.c          | 45 ++++++++++++++++++++++
>  .../platform/x86/intel_sgx/sgx_le_proxy_piggy.S    |  4 ++
>  drivers/platform/x86/intel_sgx/sgx_main.c          | 26 +++++++++++++
>  4 files changed, 78 insertions(+)
> 
> diff --git a/drivers/platform/x86/intel_sgx/sgx.h b/drivers/platform/x86/intel_sgx/sgx.h
> index 338a8bf7acfd..2399871d2e0b 100644
> --- a/drivers/platform/x86/intel_sgx/sgx.h
> +++ b/drivers/platform/x86/intel_sgx/sgx.h
> @@ -164,6 +164,9 @@ extern u64 sgx_encl_size_max_64;
>  extern u64 sgx_xfrm_mask;
>  extern u32 sgx_ssaframesize_tbl[64];
>  extern bool sgx_has_sgx2;
> +extern bool sgx_locked_msrs;
> +extern void *sgx_msrs_set;
> +extern u64 sgx_le_pubkeyhash[4];
>  
>  extern const struct file_operations sgx_fops;
>  extern const struct vm_operations_struct sgx_vm_ops;
> diff --git a/drivers/platform/x86/intel_sgx/sgx_encl.c b/drivers/platform/x86/intel_sgx/sgx_encl.c
> index cde732bb6e83..3167dbb0452f 100644
> --- a/drivers/platform/x86/intel_sgx/sgx_encl.c
> +++ b/drivers/platform/x86/intel_sgx/sgx_encl.c
> @@ -68,6 +68,7 @@
>  #include <linux/slab.h>
>  #include <linux/hashtable.h>
>  #include <linux/shmem_fs.h>
> +#include <linux/percpu.h>
>  
>  struct sgx_add_page_req {
>  	struct sgx_encl *encl;
> @@ -732,12 +733,47 @@ int sgx_encl_add_page(struct sgx_encl *encl, unsigned long addr, void *data,
>  	return ret;
>  }
>  
> +static void *sgx_set_pubkeyhash_msrs(void)
> +{
> +	u64 msrs_pubkeyhash[4];
> +	bool *msrs_set;
> +
> +	msrs_set = get_cpu_var(sgx_msrs_set);
> +	if (*msrs_set)
> +		return msrs_set;
> +
> +	rdmsrl(MSR_IA32_SGXLEPUBKEYHASH0, msrs_pubkeyhash[0]);
> +	rdmsrl(MSR_IA32_SGXLEPUBKEYHASH1, msrs_pubkeyhash[1]);
> +	rdmsrl(MSR_IA32_SGXLEPUBKEYHASH2, msrs_pubkeyhash[2]);
> +	rdmsrl(MSR_IA32_SGXLEPUBKEYHASH3, msrs_pubkeyhash[3]);
> +
> +	if ((sgx_le_pubkeyhash[0] == msrs_pubkeyhash[0]) &&
> +	    (sgx_le_pubkeyhash[1] == msrs_pubkeyhash[1]) &&
> +	    (sgx_le_pubkeyhash[2] == msrs_pubkeyhash[2]) &&
> +	    (sgx_le_pubkeyhash[3] == msrs_pubkeyhash[3]))
> +		return msrs_set;
> +
> +	if (sgx_locked_msrs) {
> +		pr_err("intel_sgx: the LE public key MSRs do not match\n");
> +		put_cpu_var(msrs_set);
> +		return ERR_PTR(-ENODEV);
> +	}
> +
> +	pr_info("intel_sgx: updating the LE public key MSRs\n");
> +	wrmsrl(MSR_IA32_SGXLEPUBKEYHASH0, sgx_le_pubkeyhash[0]);
> +	wrmsrl(MSR_IA32_SGXLEPUBKEYHASH1, sgx_le_pubkeyhash[1]);
> +	wrmsrl(MSR_IA32_SGXLEPUBKEYHASH2, sgx_le_pubkeyhash[2]);
> +	wrmsrl(MSR_IA32_SGXLEPUBKEYHASH3, sgx_le_pubkeyhash[3]);
> +	return msrs_set;
> +}
> +
>  int sgx_encl_init(struct sgx_encl *encl, struct sgx_sigstruct *sigstruct,
>  		  struct sgx_einittoken *einittoken)
>  {
>  	int ret = SGX_UNMASKED_EVENT;
>  	struct sgx_epc_page *secs_epc = encl->secs_page.epc_page;
>  	void *secs_va = NULL;
> +	void *cpu_lock;
>  	int i;
>  	int j;
>  
> @@ -752,9 +788,18 @@ int sgx_encl_init(struct sgx_encl *encl, struct sgx_sigstruct *sigstruct,
>  
>  	for (i = 0; i < SGX_EINIT_SLEEP_COUNT; i++) {
>  		for (j = 0; j < SGX_EINIT_SPIN_COUNT; j++) {
> +			cpu_lock = sgx_set_pubkeyhash_msrs();
> +			if (IS_ERR(cpu_lock)) {
> +				mutex_unlock(&encl->lock);
> +				return PTR_ERR(cpu_lock);
> +			}
> +
>  			secs_va = sgx_get_page(secs_epc);
>  			ret = __einit(sigstruct, einittoken, secs_va);
>  			sgx_put_page(secs_va);
> +
> +			put_cpu_var(cpu_lock);
> +
>  			if (ret == SGX_UNMASKED_EVENT)
>  				continue;
>  			else
> diff --git a/drivers/platform/x86/intel_sgx/sgx_le_proxy_piggy.S b/drivers/platform/x86/intel_sgx/sgx_le_proxy_piggy.S
> index faced8a9a75a..e1e3742a0c93 100644
> --- a/drivers/platform/x86/intel_sgx/sgx_le_proxy_piggy.S
> +++ b/drivers/platform/x86/intel_sgx/sgx_le_proxy_piggy.S
> @@ -9,3 +9,7 @@ GLOBAL(sgx_le_proxy)
>  END(sgx_le_proxy)
>  
>  GLOBAL(sgx_le_proxy_end)
> +
> +GLOBAL(sgx_le_ss)
> +	.incbin	"drivers/platform/x86/intel_sgx/le/enclave/sgx_le.ss"
> +END(sgx_le_ss)
> diff --git a/drivers/platform/x86/intel_sgx/sgx_main.c b/drivers/platform/x86/intel_sgx/sgx_main.c
> index baebc233b3e9..e4364d4d77bb 100644
> --- a/drivers/platform/x86/intel_sgx/sgx_main.c
> +++ b/drivers/platform/x86/intel_sgx/sgx_main.c
> @@ -68,6 +68,7 @@
>  #include <linux/hashtable.h>
>  #include <linux/kthread.h>
>  #include <linux/platform_device.h>
> +#include <linux/percpu.h>
>  
>  #define DRV_DESCRIPTION "Intel SGX Driver"
>  #define DRV_VERSION "0.10"
> @@ -80,6 +81,7 @@ MODULE_VERSION(DRV_VERSION);
>   * Global data.
>   */
>  
> +extern struct sgx_sigstruct sgx_le_ss;
>  struct workqueue_struct *sgx_add_page_wq;
>  #define SGX_MAX_EPC_BANKS 8
>  struct sgx_epc_bank sgx_epc_banks[SGX_MAX_EPC_BANKS];
> @@ -89,6 +91,9 @@ u64 sgx_encl_size_max_64;
>  u64 sgx_xfrm_mask = 0x3;
>  u32 sgx_ssaframesize_tbl[64];
>  bool sgx_has_sgx2;
> +bool sgx_locked_msrs;
> +void *sgx_msrs_set;
> +u64 sgx_le_pubkeyhash[4];
>  
>  #ifdef CONFIG_COMPAT
>  long sgx_compat_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
> @@ -191,6 +196,14 @@ static int sgx_dev_init(struct device *dev)
>  	if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
>  		return -ENODEV;
>  
> +	sgx_msrs_set = alloc_percpu(bool);
> +	if (!sgx_msrs_set)
> +		return -ENOMEM;
> +
> +	ret = sgx_get_key_hash_simple(sgx_le_ss.modulus, sgx_le_pubkeyhash);
> +	if (ret)
> +		return ret;
> +
>  	for (i = 0; i < SGX_MAX_EPC_BANKS; i++) {
>  		cpuid_count(SGX_CPUID, i + 2, &eax, &ebx, &ecx, &edx);
>  		if (!(eax & 0xf))
> @@ -268,6 +281,7 @@ static int sgx_dev_init(struct device *dev)
>  static int sgx_drv_probe(struct platform_device *pdev)
>  {
>  	unsigned int eax, ebx, ecx, edx;
> +	unsigned long fc;
>  	int i;
>  
>  	if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
> @@ -284,6 +298,17 @@ static int sgx_drv_probe(struct platform_device *pdev)
>  		return -ENODEV;
>  	}
>  
> +	if (!boot_cpu_has(X86_FEATURE_SGX_LC)) {
> +		pr_err("intel_sgx: the CPU is missing launch control\n");
> +		return -ENODEV;
> +	}
> +
> +	rdmsrl(MSR_IA32_FEATURE_CONTROL, fc);
> +	if (!(fc & FEATURE_CONTROL_SGX_LAUNCH_CONTROL_ENABLE)) {
> +		pr_info("intel_sgx: the LE public key MSRs are locked\n");
> +		sgx_locked_msrs = true;
> +	}
> +
>  	cpuid_count(SGX_CPUID, 0x0, &eax, &ebx, &ecx, &edx);
>  	if (!(eax & 1)) {
>  		pr_err("intel_sgx: CPU does not support the SGX1 instructions\n");
> @@ -328,6 +353,7 @@ static int sgx_drv_remove(struct platform_device *pdev)
>  		iounmap((void *)sgx_epc_banks[i].va);
>  #endif
>  	sgx_page_cache_teardown();
> +	free_percpu(sgx_msrs_set);
>  
>  	return 0;
>  }
> -- 
> 2.14.1
>
Jarkko Sakkinen Sept. 6, 2017, 1:12 p.m. UTC | #2 
On Tue, Sep 05, 2017 at 07:49:50AM +0300, Jarkko Sakkinen wrote:
> On Fri, Sep 01, 2017 at 08:30:02PM +0300, Jarkko Sakkinen wrote:
> > Check if IA32_SGXLEPUBKEYHASH* MSRs match. If they do not match, allow
> > the driver initialization to continue only if they are writable. In this
> > case update them with the MRSIGNER of the launch enclave.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> 
> This commit is not right. I've forgot to squash some fixups.
> 
> Today I have to prepare some slides to LPC an LSS and do TPM
> maintainer tasks. I'll sent updated patch set tmrw.
> 
> /Jarkko

Still pending to fix this as I have had more unrelated work than
I expected. Fixing this asap and sending v2.

/Jarkko
Jarkko Sakkinen Sept. 7, 2017, 4:06 p.m. UTC | #3 
On Wed, Sep 06, 2017 at 04:12:34PM +0300, Jarkko Sakkinen wrote:
> On Tue, Sep 05, 2017 at 07:49:50AM +0300, Jarkko Sakkinen wrote:
> > On Fri, Sep 01, 2017 at 08:30:02PM +0300, Jarkko Sakkinen wrote:
> > > Check if IA32_SGXLEPUBKEYHASH* MSRs match. If they do not match, allow
> > > the driver initialization to continue only if they are writable. In this
> > > case update them with the MRSIGNER of the launch enclave.
> > > 
> > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > 
> > This commit is not right. I've forgot to squash some fixups.
> > 
> > Today I have to prepare some slides to LPC an LSS and do TPM
> > maintainer tasks. I'll sent updated patch set tmrw.
> > 
> > /Jarkko
> 
> Still pending to fix this as I have had more unrelated work than
> I expected. Fixing this asap and sending v2.

The crash is fixed in the 'le' branch. I'll do the other updates
to the patch set before sending v2 documented in  my response to
the cover letter.

/Jarkko
diff mbox

Patch

diff --git a/drivers/platform/x86/intel_sgx/sgx.h b/drivers/platform/x86/intel_sgx/sgx.h
index 338a8bf7acfd..2399871d2e0b 100644
--- a/drivers/platform/x86/intel_sgx/sgx.h
+++ b/drivers/platform/x86/intel_sgx/sgx.h
@@ -164,6 +164,9 @@  extern u64 sgx_encl_size_max_64;
 extern u64 sgx_xfrm_mask;
 extern u32 sgx_ssaframesize_tbl[64];
 extern bool sgx_has_sgx2;
+extern bool sgx_locked_msrs;
+extern void *sgx_msrs_set;
+extern u64 sgx_le_pubkeyhash[4];
 
 extern const struct file_operations sgx_fops;
 extern const struct vm_operations_struct sgx_vm_ops;
diff --git a/drivers/platform/x86/intel_sgx/sgx_encl.c b/drivers/platform/x86/intel_sgx/sgx_encl.c
index cde732bb6e83..3167dbb0452f 100644
--- a/drivers/platform/x86/intel_sgx/sgx_encl.c
+++ b/drivers/platform/x86/intel_sgx/sgx_encl.c
@@ -68,6 +68,7 @@ 
 #include <linux/slab.h>
 #include <linux/hashtable.h>
 #include <linux/shmem_fs.h>
+#include <linux/percpu.h>
 
 struct sgx_add_page_req {
 	struct sgx_encl *encl;
@@ -732,12 +733,47 @@  int sgx_encl_add_page(struct sgx_encl *encl, unsigned long addr, void *data,
 	return ret;
 }
 
+static void *sgx_set_pubkeyhash_msrs(void)
+{
+	u64 msrs_pubkeyhash[4];
+	bool *msrs_set;
+
+	msrs_set = get_cpu_var(sgx_msrs_set);
+	if (*msrs_set)
+		return msrs_set;
+
+	rdmsrl(MSR_IA32_SGXLEPUBKEYHASH0, msrs_pubkeyhash[0]);
+	rdmsrl(MSR_IA32_SGXLEPUBKEYHASH1, msrs_pubkeyhash[1]);
+	rdmsrl(MSR_IA32_SGXLEPUBKEYHASH2, msrs_pubkeyhash[2]);
+	rdmsrl(MSR_IA32_SGXLEPUBKEYHASH3, msrs_pubkeyhash[3]);
+
+	if ((sgx_le_pubkeyhash[0] == msrs_pubkeyhash[0]) &&
+	    (sgx_le_pubkeyhash[1] == msrs_pubkeyhash[1]) &&
+	    (sgx_le_pubkeyhash[2] == msrs_pubkeyhash[2]) &&
+	    (sgx_le_pubkeyhash[3] == msrs_pubkeyhash[3]))
+		return msrs_set;
+
+	if (sgx_locked_msrs) {
+		pr_err("intel_sgx: the LE public key MSRs do not match\n");
+		put_cpu_var(msrs_set);
+		return ERR_PTR(-ENODEV);
+	}
+
+	pr_info("intel_sgx: updating the LE public key MSRs\n");
+	wrmsrl(MSR_IA32_SGXLEPUBKEYHASH0, sgx_le_pubkeyhash[0]);
+	wrmsrl(MSR_IA32_SGXLEPUBKEYHASH1, sgx_le_pubkeyhash[1]);
+	wrmsrl(MSR_IA32_SGXLEPUBKEYHASH2, sgx_le_pubkeyhash[2]);
+	wrmsrl(MSR_IA32_SGXLEPUBKEYHASH3, sgx_le_pubkeyhash[3]);
+	return msrs_set;
+}
+
 int sgx_encl_init(struct sgx_encl *encl, struct sgx_sigstruct *sigstruct,
 		  struct sgx_einittoken *einittoken)
 {
 	int ret = SGX_UNMASKED_EVENT;
 	struct sgx_epc_page *secs_epc = encl->secs_page.epc_page;
 	void *secs_va = NULL;
+	void *cpu_lock;
 	int i;
 	int j;
 
@@ -752,9 +788,18 @@  int sgx_encl_init(struct sgx_encl *encl, struct sgx_sigstruct *sigstruct,
 
 	for (i = 0; i < SGX_EINIT_SLEEP_COUNT; i++) {
 		for (j = 0; j < SGX_EINIT_SPIN_COUNT; j++) {
+			cpu_lock = sgx_set_pubkeyhash_msrs();
+			if (IS_ERR(cpu_lock)) {
+				mutex_unlock(&encl->lock);
+				return PTR_ERR(cpu_lock);
+			}
+
 			secs_va = sgx_get_page(secs_epc);
 			ret = __einit(sigstruct, einittoken, secs_va);
 			sgx_put_page(secs_va);
+
+			put_cpu_var(cpu_lock);
+
 			if (ret == SGX_UNMASKED_EVENT)
 				continue;
 			else
diff --git a/drivers/platform/x86/intel_sgx/sgx_le_proxy_piggy.S b/drivers/platform/x86/intel_sgx/sgx_le_proxy_piggy.S
index faced8a9a75a..e1e3742a0c93 100644
--- a/drivers/platform/x86/intel_sgx/sgx_le_proxy_piggy.S
+++ b/drivers/platform/x86/intel_sgx/sgx_le_proxy_piggy.S
@@ -9,3 +9,7 @@  GLOBAL(sgx_le_proxy)
 END(sgx_le_proxy)
 
 GLOBAL(sgx_le_proxy_end)
+
+GLOBAL(sgx_le_ss)
+	.incbin	"drivers/platform/x86/intel_sgx/le/enclave/sgx_le.ss"
+END(sgx_le_ss)
diff --git a/drivers/platform/x86/intel_sgx/sgx_main.c b/drivers/platform/x86/intel_sgx/sgx_main.c
index baebc233b3e9..e4364d4d77bb 100644
--- a/drivers/platform/x86/intel_sgx/sgx_main.c
+++ b/drivers/platform/x86/intel_sgx/sgx_main.c
@@ -68,6 +68,7 @@ 
 #include <linux/hashtable.h>
 #include <linux/kthread.h>
 #include <linux/platform_device.h>
+#include <linux/percpu.h>
 
 #define DRV_DESCRIPTION "Intel SGX Driver"
 #define DRV_VERSION "0.10"
@@ -80,6 +81,7 @@  MODULE_VERSION(DRV_VERSION);
  * Global data.
  */
 
+extern struct sgx_sigstruct sgx_le_ss;
 struct workqueue_struct *sgx_add_page_wq;
 #define SGX_MAX_EPC_BANKS 8
 struct sgx_epc_bank sgx_epc_banks[SGX_MAX_EPC_BANKS];
@@ -89,6 +91,9 @@  u64 sgx_encl_size_max_64;
 u64 sgx_xfrm_mask = 0x3;
 u32 sgx_ssaframesize_tbl[64];
 bool sgx_has_sgx2;
+bool sgx_locked_msrs;
+void *sgx_msrs_set;
+u64 sgx_le_pubkeyhash[4];
 
 #ifdef CONFIG_COMPAT
 long sgx_compat_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
@@ -191,6 +196,14 @@  static int sgx_dev_init(struct device *dev)
 	if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
 		return -ENODEV;
 
+	sgx_msrs_set = alloc_percpu(bool);
+	if (!sgx_msrs_set)
+		return -ENOMEM;
+
+	ret = sgx_get_key_hash_simple(sgx_le_ss.modulus, sgx_le_pubkeyhash);
+	if (ret)
+		return ret;
+
 	for (i = 0; i < SGX_MAX_EPC_BANKS; i++) {
 		cpuid_count(SGX_CPUID, i + 2, &eax, &ebx, &ecx, &edx);
 		if (!(eax & 0xf))
@@ -268,6 +281,7 @@  static int sgx_dev_init(struct device *dev)
 static int sgx_drv_probe(struct platform_device *pdev)
 {
 	unsigned int eax, ebx, ecx, edx;
+	unsigned long fc;
 	int i;
 
 	if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
@@ -284,6 +298,17 @@  static int sgx_drv_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	if (!boot_cpu_has(X86_FEATURE_SGX_LC)) {
+		pr_err("intel_sgx: the CPU is missing launch control\n");
+		return -ENODEV;
+	}
+
+	rdmsrl(MSR_IA32_FEATURE_CONTROL, fc);
+	if (!(fc & FEATURE_CONTROL_SGX_LAUNCH_CONTROL_ENABLE)) {
+		pr_info("intel_sgx: the LE public key MSRs are locked\n");
+		sgx_locked_msrs = true;
+	}
+
 	cpuid_count(SGX_CPUID, 0x0, &eax, &ebx, &ecx, &edx);
 	if (!(eax & 1)) {
 		pr_err("intel_sgx: CPU does not support the SGX1 instructions\n");
@@ -328,6 +353,7 @@  static int sgx_drv_remove(struct platform_device *pdev)
 		iounmap((void *)sgx_epc_banks[i].va);
 #endif
 	sgx_page_cache_teardown();
+	free_percpu(sgx_msrs_set);
 
 	return 0;
 }