> On Mon, 2025-05-19 at 10:24 +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 <[email protected]>
> > ---
> > 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
> > + * 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;
>
> Should be:
>
> if (!cpu_feature_enabled(X86_FEATURE_SGX_EUPDATESVN))
> return 0;
Yes, right. Will fix.
>
> > +
> > + 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))
> > + return 0;
>
> I am a little bit confused why we should return 0 when running out of
> entropy.
>
> It seems in v4 you said it's not that hard to cause EUPDATESVN to fail
> reliably:
>
> And to make it more concrete, I made some simple tests based
> on program for stress testing rdrand/rdseed that was discussed in that
> threat earlier: https://lkml.org/lkml/2024/2/6/746
> Using this stress testing and enough threads, I can make EUPDATESVN fail
> reliably and quite easily even with 10 time retry loop by kernel.
>
> Returning 0 will make sgx_open() succeed if I read your next patch correctly,
> which means this will allow enclave to be created when updating SVN fails.
Yes, correct.
>
> Why not just fail sgx_open() (e.g., return -EBUSY, or -EAGAIN) in that case?
> Userspace can then retry?
The idea on the patch was that such a scenario where we run out of entropy
should not happen in real life unless RDSEED is under stress (in case we
accidentally collided, we do a 10 time retry). So, in this case we keep the
legacy
behaviour, i.e. proceeding without EUPDATESVN. But I can change to the above
logic to return -EAGAIN in this case if everyone thinks it is a better
approach.
Best Regards,
Elena.
>
> > +
> > + if (!ret) {
> > + /*
> > + * SVN successfully updated.
> > + * Let users know when the update was successful.
> > + */
> > + pr_info("SVN updated successfully\n");
> > + return 0;
> > + }
> > +
> > + /*
> > + * EUPDATESVN was called when EPC is empty, all other error
> > + * codes are unexpected.
> > + */
> > + ENCLS_WARN(ret, "EUPDATESVN");
> > + return ret;
> > +}
> > +
> > int sgx_inc_usage_count(void)
> > {
> > atomic64_inc(&sgx_usage_count);
