On Tue, Sep 03, 2019 at 05:26:41PM +0300, Jarkko Sakkinen wrote:
> From: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
>
> 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@xxxxxxxxx>
> Co-developed-by: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx>
> ---
> 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.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
