On Mon, May 19, 2025 at 10:24:30AM +0300, Elena Reshetova wrote:
> The SGX attestation architecture assumes a compromise
> of all running enclaves and cryptographic assets
> (like internal SGX encryption keys) whenever a microcode
> update affects SGX. To mitigate the impact of this presumed
> compromise, a new supervisor SGX instruction: ENCLS[EUPDATESVN],
> is introduced to update SGX microcode version and generate
> new cryptographic assets in runtime after SGX microcode update.
>
> EUPDATESVN requires that SGX memory to be marked as "unused"
> before it will succeed. This ensures that no compromised enclave
> can survive the process and provides an opportunity to generate
> new cryptographic assets.
>
> Add the method to perform ENCLS[EUPDATESVN].
>
> Signed-off-by: Elena Reshetova <elena.reshet...@intel.com>
> ---
> arch/x86/kernel/cpu/sgx/encls.h | 5 +++
> arch/x86/kernel/cpu/sgx/main.c | 57 +++++++++++++++++++++++++++++++++
> 2 files changed, 62 insertions(+)
>
> diff --git a/arch/x86/kernel/cpu/sgx/encls.h b/arch/x86/kernel/cpu/sgx/encls.h
> index 99004b02e2ed..d9160c89a93d 100644
> --- a/arch/x86/kernel/cpu/sgx/encls.h
> +++ b/arch/x86/kernel/cpu/sgx/encls.h
> @@ -233,4 +233,9 @@ static inline int __eaug(struct sgx_pageinfo *pginfo,
> void *addr)
> return __encls_2(EAUG, pginfo, addr);
> }
>
> +/* Attempt to update CPUSVN at runtime. */
> +static inline int __eupdatesvn(void)
> +{
> + return __encls_ret_1(EUPDATESVN, "");
> +}
> #endif /* _X86_ENCLS_H */
> diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
> index 80d565e6f2ad..fd71e2ddca59 100644
> --- a/arch/x86/kernel/cpu/sgx/main.c
> +++ b/arch/x86/kernel/cpu/sgx/main.c
> @@ -15,6 +15,7 @@
> #include <linux/sysfs.h>
> #include <linux/vmalloc.h>
> #include <asm/sgx.h>
> +#include <asm/archrandom.h>
> #include "driver.h"
> #include "encl.h"
> #include "encls.h"
> @@ -917,6 +918,62 @@ EXPORT_SYMBOL_GPL(sgx_set_attribute);
> /* Counter to count the active SGX users */
> static atomic64_t sgx_usage_count;
>
> +/**
> + * sgx_updatesvn() - Attempt to call ENCLS[EUPDATESVN]
> + * If EPC is empty, this instruction attempts to update CPUSVN to the
> + * currently loaded microcode update SVN and generate new
> + * cryptographic assets.sgx_updatesvn() Most of the time, there will
Is there something wrong here in the text? It looks malformed.
> + * be no update and that's OK.
> + *
> + * Return:
> + * 0: Success, not supported or run out of entropy
> + */
> +static int sgx_update_svn(void)
> +{
> + int ret;
> +
> + /*
> + * If EUPDATESVN is not available, it is ok to
> + * silently skip it to comply with legacy behavior.
> + */
> + if (!X86_FEATURE_SGX_EUPDATESVN)
> + return 0;
> +
> + for (int i = 0; i < RDRAND_RETRY_LOOPS; i++) {
> + ret = __eupdatesvn();
> +
> + /* Stop on success or unexpected errors: */
> + if (ret != SGX_INSUFFICIENT_ENTROPY)
> + break;
> + }
> +
> + /*
> + * SVN either was up-to-date or SVN update failed due
> + * to lack of entropy. In both cases, we want to return
> + * 0 in order not to break sgx_(vepc_)open. We dont expect
> + * SGX_INSUFFICIENT_ENTROPY error unless underlying RDSEED
> + * is under heavy pressure.
> + */
> + if ((ret == SGX_NO_UPDATE) || (ret == SGX_INSUFFICIENT_ENTROPY))
if (ret == SGX_NO_UPDATE || ret == SGX_INSUFFICIENT_ENTROPY)
> + return 0;
> +
> + if (!ret) {
> + /*
> + * SVN successfully updated.
> + * Let users know when the update was successful.
> + */
This comment is like as useless as an inline comment can ever possibly
be. Please, remove it.
> + pr_info("SVN updated successfully\n");
Let's not add this either in the scope of this patch set.
> + return 0;
> + }
Since you parse error codes already, I don't understand why deal with
the success case in the middle of doing that.
More consistent would be (not also the use of unlikely()):
if (ret == SGX_NO_UPDATE || ret == SGX_INSUFFICIENT_ENTROPY)
return 0;
/*
* EUPDATESVN was called when EPC is empty, all other error
* codes are unexpected.
*/
if (unlikely(ret)) {
ENCLS_WARN(ret, "EUPDATESVN");
return ret;
}
return 0;
}
This is how I would rewrite the tail of this function.
BR, Jarkko
