On Wed, 2025-08-06 at 11:11 +0300, Elena Reshetova wrote: > All running enclaves and cryptographic assets (such as internal SGX > encryption keys) are assumed to be compromised whenever an SGX-related > microcode update occurs. To mitigate this assumed compromise the new > supervisor SGX instruction ENCLS[EUPDATESVN] can generate fresh > cryptographic assets. > > Before executing EUPDATESVN, all SGX memory must be marked as unused. > This requirement ensures that no potentially compromised enclave > survives the update and allows the system to safely regenerate > cryptographic assets. > > Add the method to perform ENCLS[EUPDATESVN]. However, until the > follow up patch that wires calling sgx_update_svn() from > sgx_inc_usage_count(), this code is not reachable.
Please check the text wrap. > > Signed-off-by: Elena Reshetova <[email protected]> > --- > arch/x86/include/asm/sgx.h | 31 +++++++------- > arch/x86/kernel/cpu/sgx/encls.h | 5 +++ > arch/x86/kernel/cpu/sgx/main.c | 73 +++++++++++++++++++++++++++++++++ > 3 files changed, 94 insertions(+), 15 deletions(-) > > diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h > index 2da5b3b117a1..0e13678f9cbd 100644 > --- a/arch/x86/include/asm/sgx.h > +++ b/arch/x86/include/asm/sgx.h > @@ -28,21 +28,22 @@ > #define SGX_CPUID_EPC_MASK GENMASK(3, 0) > > enum sgx_encls_function { > - ECREATE = 0x00, > - EADD = 0x01, > - EINIT = 0x02, > - EREMOVE = 0x03, > - EDGBRD = 0x04, > - EDGBWR = 0x05, > - EEXTEND = 0x06, > - ELDU = 0x08, > - EBLOCK = 0x09, > - EPA = 0x0A, > - EWB = 0x0B, > - ETRACK = 0x0C, > - EAUG = 0x0D, > - EMODPR = 0x0E, > - EMODT = 0x0F, > + ECREATE = 0x00, > + EADD = 0x01, > + EINIT = 0x02, > + EREMOVE = 0x03, > + EDGBRD = 0x04, > + EDGBWR = 0x05, > + EEXTEND = 0x06, > + ELDU = 0x08, > + EBLOCK = 0x09, > + EPA = 0x0A, > + EWB = 0x0B, > + ETRACK = 0x0C, > + EAUG = 0x0D, > + EMODPR = 0x0E, > + EMODT = 0x0F, > + EUPDATESVN = 0x18, > }; > > /** > 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 3a5cbd1c170e..d8c42524b590 100644 > --- a/arch/x86/kernel/cpu/sgx/main.c > +++ b/arch/x86/kernel/cpu/sgx/main.c > @@ -16,6 +16,7 @@ > #include <linux/vmalloc.h> > #include <asm/msr.h> > #include <asm/sgx.h> > +#include <asm/archrandom.h> > #include "driver.h" > #include "encl.h" > #include "encls.h" > @@ -917,6 +918,78 @@ int sgx_set_attribute(unsigned long *allowed_attributes, > } > EXPORT_SYMBOL_GPL(sgx_set_attribute); > > +/* Counter to count the active SGX users */ > +static int sgx_usage_count; > + > +/** > + * sgx_update_svn() - Attempt to call ENCLS[EUPDATESVN]. Add a newline would make the comment more readable. > + * This instruction attempts to update CPUSVN to the > + * currently loaded microcode update SVN and generate new > + * cryptographic assets. Ditto here. > + * Return: > + * 0: Success or not supported > + * -EAGAIN: Can be safely retried, failure is due to lack of > + * entropy in RNG. > + * -EIO: Unexpected error, retries are not advisable. > + */ You may want to make the description of the return code vertically aligned. And looking at the k-doc documentation, the format of the return codes could be improved: * Return: * * %0: - Success or not supported * * %-EAGAIN: - ... * * %-EIO: - ... Please see "Return values" part of: https://docs.kernel.org/doc-guide/kernel-doc.html Or you can switch to use normal comment. It's a static function anyway. > +static int __maybe_unused sgx_update_svn(void) > +{ > + int ret; > + > + /* > + * If EUPDATESVN is not available, it is ok to > + * silently skip it to comply with legacy behavior. > + */ > + if (!cpu_feature_enabled(X86_FEATURE_SGX_EUPDATESVN)) > + return 0; > + > + /* > + * EPC is guaranteed to be empty when there are no users. > + * Ensure we are on our first user before proceeding further. > + */ > + WARN(sgx_usage_count != 1, "Elevated usage count when calling > EUPDATESVN\n"); I am not sure whether this is needed. Wouldn't the ENCLS_WARN() at the end catch this case and the user is able to figure out what went wrong from the error code? Besides that, in _this_ patch, what prevents sgx_usage_count from being concurrently updated is still unknown. It's kinda weird to just use it here w/o seeing the actual mutex. > + > + for (int i = 0; i < RDRAND_RETRY_LOOPS; i++) { > + ret = __eupdatesvn(); > + > + /* Stop on success or unexpected errors: */ > + if (ret != SGX_INSUFFICIENT_ENTROPY) > + break; > + } > + > + switch (ret) { > + /* > + * SVN successfully updated. > + * Let users know when the update was successful. > + */ This is ugly. I would just put the comment inside the 'case'. > + case 0: > + pr_info("SVN updated successfully\n"); > + return 0; > + /* > + * SVN update failed since the current SVN is > + * not newer than CPUSVN. This is the most > + * common case and indicates no harm. > + */ > + case SGX_NO_UPDATE: > + return 0; > + /* > + * SVN update failed due to lack of entropy in DRNG. > + * Indicate to userspace that it should retry. > + */ > + case SGX_INSUFFICIENT_ENTROPY: > + return -EAGAIN; > + default: > + break; > + } Ditto for all the comments above.
