diff mbox series

[v22,10/24] x86/sgx: Add sgx_einit() for wrapping ENCLS[EINIT]

Message ID 20190903142655.21943-11-jarkko.sakkinen@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series Intel SGX foundations | expand

Commit Message

Jarkko Sakkinen Sept. 3, 2019, 2:26 p.m. UTC 
From: Sean Christopherson <sean.j.christopherson@intel.com>

Enclaves are SGX hosted measured and signed software entities. ENCLS[EINIT]
leaf function checks that the enclave has a legit signed measurement and
transforms the enclave to the state ready for execution. The signed
measurement is provided by the caller in the form of SIGSTRUCT data
structure [1].

Wrap ENCLS[EINIT] into sgx_einit(). Set MSR_IA32_SGXLEPUBKEYHASH* MSRs to
match the public key contained in the SIGSTRUCT [2]. This sets Linux to
enforce a policy where the provided public key is as long as the signed
measurement matches the enclave contents in memory.

Add a per-cpu cache to avoid unnecessary reads and write to the MSRs
as they are expensive operations.

[1] Intel SDM: 37.1.3 ENCLAVE SIGNATURE STRUCTURE (SIGSTRUCT)
[2] Intel SDM: 38.1.4 Intel SGX Launch Control Configuration

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Co-developed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 arch/x86/kernel/cpu/sgx/main.c | 51 ++++++++++++++++++++++++++++++++++
 arch/x86/kernel/cpu/sgx/sgx.h  |  2 ++
 2 files changed, 53 insertions(+)

Comments

Borislav Petkov Oct. 8, 2019, 5:30 p.m. UTC | #1 
On Tue, Sep 03, 2019 at 05:26:41PM +0300, Jarkko Sakkinen wrote:
> From: Sean Christopherson <sean.j.christopherson@intel.com>
> 
> Enclaves are SGX hosted measured and signed software entities. ENCLS[EINIT]

		SGX-hosted, measured, and ...

> leaf function checks that the enclave has a legit signed measurement and
> transforms the enclave to the state ready for execution. The signed
> measurement is provided by the caller in the form of SIGSTRUCT data
> structure [1].
> 
> Wrap ENCLS[EINIT] into sgx_einit(). Set MSR_IA32_SGXLEPUBKEYHASH* MSRs to
> match the public key contained in the SIGSTRUCT [2]. This sets Linux to
> enforce a policy where the provided public key is as long as the signed
> measurement matches the enclave contents in memory.

That subclause needs to be separated, maybe:

  ... the provided public key is - as long as the signed measurement
  matches the the enclave contents - in memory.

Provided you mean that, of course.

> Add a per-cpu cache to avoid unnecessary reads and write to the MSRs

reads and writes?

> as they are expensive operations.
> 
> [1] Intel SDM: 37.1.3 ENCLAVE SIGNATURE STRUCTURE (SIGSTRUCT)
> [2] Intel SDM: 38.1.4 Intel SGX Launch Control Configuration
> 
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> Co-developed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>  arch/x86/kernel/cpu/sgx/main.c | 51 ++++++++++++++++++++++++++++++++++
>  arch/x86/kernel/cpu/sgx/sgx.h  |  2 ++
>  2 files changed, 53 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
> index 6b4727df72ca..d3ed742e90fe 100644
> --- a/arch/x86/kernel/cpu/sgx/main.c
> +++ b/arch/x86/kernel/cpu/sgx/main.c
> @@ -17,6 +17,9 @@ EXPORT_SYMBOL_GPL(sgx_epc_sections);
>  
>  int sgx_nr_epc_sections;
>  
> +/* A per-cpu cache for the last known values of IA32_SGXLEPUBKEYHASHx MSRs. */
> +static DEFINE_PER_CPU(u64 [4], sgx_lepubkeyhash_cache);
> +
>  static struct sgx_epc_page *sgx_section_get_page(
>  	struct sgx_epc_section *section)
>  {
> @@ -106,6 +109,54 @@ void sgx_free_page(struct sgx_epc_page *page)
>  }
>  EXPORT_SYMBOL_GPL(sgx_free_page);
>  
> +static void sgx_update_lepubkeyhash_msrs(u64 *lepubkeyhash, bool enforce)
> +{
> +	u64 *cache;
> +	int i;
> +
> +	cache = per_cpu(sgx_lepubkeyhash_cache, smp_processor_id());
> +	for (i = 0; i < 4; i++) {
> +		if (enforce || (lepubkeyhash[i] != cache[i])) {
> +			wrmsrl(MSR_IA32_SGXLEPUBKEYHASH0 + i, lepubkeyhash[i]);
> +			cache[i] = lepubkeyhash[i];
> +		}
> +	}
> +}
> +
> +/**
> + * sgx_einit - initialize an enclave
> + * @sigstruct:		a pointer a SIGSTRUCT
> + * @token:		a pointer an EINITTOKEN (optional)
> + * @secs:		a pointer a SECS

That's a strange formulation "a pointer a/an" ? "to" missing?

> + * @lepubkeyhash:	the desired value for IA32_SGXLEPUBKEYHASHx MSRs
> + *
> + * Execute ENCLS[EINIT], writing the IA32_SGXLEPUBKEYHASHx MSRs according
> + * to @lepubkeyhash (if possible and necessary).
> + *
> + * Return:
> + *   0 on success,
> + *   -errno or SGX error on failure
> + */
> +int sgx_einit(struct sgx_sigstruct *sigstruct, struct sgx_einittoken *token,
> +	      struct sgx_epc_page *secs, u64 *lepubkeyhash)
> +{
> +	int ret;
> +
> +	if (!boot_cpu_has(X86_FEATURE_SGX_LC))
> +		return __einit(sigstruct, token, sgx_epc_addr(secs));
> +
> +	preempt_disable();
> +	sgx_update_lepubkeyhash_msrs(lepubkeyhash, false);
> +	ret = __einit(sigstruct, token, sgx_epc_addr(secs));
> +	if (ret == SGX_INVALID_EINITTOKEN) {
> +		sgx_update_lepubkeyhash_msrs(lepubkeyhash, true);
> +		ret = __einit(sigstruct, token, sgx_epc_addr(secs));
> +	}
> +	preempt_enable();
> +	return ret;
> +}
> +EXPORT_SYMBOL(sgx_einit);

I was about to ask why isn't this export _GPL() but it goes away in a
later patch.
Sean Christopherson Oct. 8, 2019, 5:45 p.m. UTC | #2 
On Tue, Oct 08, 2019 at 07:30:35PM +0200, Borislav Petkov wrote:
> I was about to ask why isn't this export _GPL() but it goes away in a
> later patch.

Speaking of later patches, don't bother reviewing patches 19/24 or 20/20,
the vDSO function and selftest respectively.  I'm in the process of
reworking them for v23, reviewing them in their current state is likely a
waste of your time.
Sean Christopherson Oct. 8, 2019, 5:46 p.m. UTC | #3 
On Tue, Oct 08, 2019 at 10:45:37AM -0700, Sean Christopherson wrote:
> On Tue, Oct 08, 2019 at 07:30:35PM +0200, Borislav Petkov wrote:
> > I was about to ask why isn't this export _GPL() but it goes away in a
> > later patch.
> 
> Speaking of later patches, don't bother reviewing patches 19/24 or 20/20,
> the vDSO function and selftest respectively.  I'm in the process of
> reworking them for v23, reviewing them in their current state is likely a
> waste of your time.

Gah, 20/24...
Borislav Petkov Oct. 8, 2019, 5:53 p.m. UTC | #4 
On Tue, Oct 08, 2019 at 10:46:19AM -0700, Sean Christopherson wrote:
> On Tue, Oct 08, 2019 at 10:45:37AM -0700, Sean Christopherson wrote:
> > On Tue, Oct 08, 2019 at 07:30:35PM +0200, Borislav Petkov wrote:
> > > I was about to ask why isn't this export _GPL() but it goes away in a
> > > later patch.
> > 
> > Speaking of later patches, don't bother reviewing patches 19/24 or 20/20,
> > the vDSO function and selftest respectively.  I'm in the process of
> > reworking them for v23, reviewing them in their current state is likely a
> > waste of your time.
> 
> Gah, 20/24...

Thanks for the heads-up - appreciate it!

:-)
diff mbox series

Patch

diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
index 6b4727df72ca..d3ed742e90fe 100644
--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -17,6 +17,9 @@  EXPORT_SYMBOL_GPL(sgx_epc_sections);
 
 int sgx_nr_epc_sections;
 
+/* A per-cpu cache for the last known values of IA32_SGXLEPUBKEYHASHx MSRs. */
+static DEFINE_PER_CPU(u64 [4], sgx_lepubkeyhash_cache);
+
 static struct sgx_epc_page *sgx_section_get_page(
 	struct sgx_epc_section *section)
 {
@@ -106,6 +109,54 @@  void sgx_free_page(struct sgx_epc_page *page)
 }
 EXPORT_SYMBOL_GPL(sgx_free_page);
 
+static void sgx_update_lepubkeyhash_msrs(u64 *lepubkeyhash, bool enforce)
+{
+	u64 *cache;
+	int i;
+
+	cache = per_cpu(sgx_lepubkeyhash_cache, smp_processor_id());
+	for (i = 0; i < 4; i++) {
+		if (enforce || (lepubkeyhash[i] != cache[i])) {
+			wrmsrl(MSR_IA32_SGXLEPUBKEYHASH0 + i, lepubkeyhash[i]);
+			cache[i] = lepubkeyhash[i];
+		}
+	}
+}
+
+/**
+ * sgx_einit - initialize an enclave
+ * @sigstruct:		a pointer a SIGSTRUCT
+ * @token:		a pointer an EINITTOKEN (optional)
+ * @secs:		a pointer a SECS
+ * @lepubkeyhash:	the desired value for IA32_SGXLEPUBKEYHASHx MSRs
+ *
+ * Execute ENCLS[EINIT], writing the IA32_SGXLEPUBKEYHASHx MSRs according
+ * to @lepubkeyhash (if possible and necessary).
+ *
+ * Return:
+ *   0 on success,
+ *   -errno or SGX error on failure
+ */
+int sgx_einit(struct sgx_sigstruct *sigstruct, struct sgx_einittoken *token,
+	      struct sgx_epc_page *secs, u64 *lepubkeyhash)
+{
+	int ret;
+
+	if (!boot_cpu_has(X86_FEATURE_SGX_LC))
+		return __einit(sigstruct, token, sgx_epc_addr(secs));
+
+	preempt_disable();
+	sgx_update_lepubkeyhash_msrs(lepubkeyhash, false);
+	ret = __einit(sigstruct, token, sgx_epc_addr(secs));
+	if (ret == SGX_INVALID_EINITTOKEN) {
+		sgx_update_lepubkeyhash_msrs(lepubkeyhash, true);
+		ret = __einit(sigstruct, token, sgx_epc_addr(secs));
+	}
+	preempt_enable();
+	return ret;
+}
+EXPORT_SYMBOL(sgx_einit);
+
 static __init void sgx_free_epc_section(struct sgx_epc_section *section)
 {
 	struct sgx_epc_page *page;
diff --git a/arch/x86/kernel/cpu/sgx/sgx.h b/arch/x86/kernel/cpu/sgx/sgx.h
index 210510a28ce0..41d4130c33a2 100644
--- a/arch/x86/kernel/cpu/sgx/sgx.h
+++ b/arch/x86/kernel/cpu/sgx/sgx.h
@@ -67,5 +67,7 @@  int sgx_page_reclaimer_init(void);
 struct sgx_epc_page *sgx_alloc_page(void);
 int __sgx_free_page(struct sgx_epc_page *page);
 void sgx_free_page(struct sgx_epc_page *page);
+int sgx_einit(struct sgx_sigstruct *sigstruct, struct sgx_einittoken *token,
+	      struct sgx_epc_page *secs, u64 *lepubkeyhash);
 
 #endif /* _X86_SGX_H */