Changes since v5 following reviews by Ingo, Kai, Jarkko and Dave: - rebase on x86 tip - patch 1 is fixed to do correct unwinding in case of errors - patch 2: add feature flag to cpuid-deps.c - patch 3: remove unused SGX_EPC_NOT_READY error code - patch 4: fix x86 feature check, return -EAGAIN on SGX_INSUFFICIENT_ENTROPY and -EIO on other non- expected errors. Comments/style are also fixed. - patch 5: rewrite commit message, add comments inline
Changes since v4 following reviews by Dave and Jarkko: - breakdown the single patch into 4 patches as suggested by Dave - fix error unwinding in sgx_(vepc_)open as suggested by Jarkko - numerous code improvements suggested by Dave - numerous additional code comments and commit message improvements as suggested by Dave - switch to usage of atomic64_t for sgx_usage_count to ensure overflows cannot happen as suggested by Dave - do not return a error case when failing with SGX_INSUFFICIENT_ENTROPY, fail silently as suggested by Dave Changes since v3 following reviews by Kai and Sean: - Change the overall approach to the one suggested by Sean and do the EUPDATESVN execution during sgx_open() and sgx_vepc_open(). Note, I do not try to do EUPDATESVN during the release() flows since it doesnt save any noticable amount of time during next open flow per underlying EUPDATESVN microcode logic. - In sgx_update_svn() remove the check that we are running under VMM and expect the VMM to instead expose correct CPUID - Move the actual CPUID leaf check out of sgx_update_svn() into sgx_init() - Use existing RDRAND_RETRY_LOOPS define instead of 10 - Change the sgx_update_svn() to return 0 only in success cases (or if unsupported) - Various smaller cosmetic fixes The still to be discussed question is what sgx_update_svn() should return in case of various failures. The current version follows suggestion by Kai and would return an error (and block sgx_(vepc_)open() in all cases, including running out of entropy. I think this might be the correct approach for SGX_INSUFFICIENT_ENTROPY since in such cases userspace can retry the open() and also will get an info about what is actually blocking the EUPDATEVSN (and can act on this). However, this is a change in existing API and therefore debatable and I would like to hear people's feedback. Changes since v2 following review by Jarkko: - formatting of comments is fixed - change from pr_error to ENCLS_WARN to communicate errors from EUPDATESVN - In case an unknown error is detected (must not happen per spec), make page allocation from EPC fail in order to prevent EPC usage Changes since v1 following review by Jarkko: - first and second patch are squashed together and a better explanation of the change is added into the commit message - third and fourth patch are also combined for better understanding of error code purposes used in 4th patch - implementation of sgx_updatesvn adjusted following Jarkko's suggestions - minor fixes in both commit messages and code from the review - dropping co-developed-by tag since the code now differs enough from the original submission. However, the reference where the original code came from and credits to original author is kept Background ---------- In case an SGX vulnerability is discovered and TCB recovery for SGX is triggered, Intel specifies a process that must be followed for a given vulnerability. Steps to mitigate can vary based on vulnerability type, affected components, etc. In some cases, a vulnerability can be mitigated via a runtime recovery flow by shutting down all running SGX enclaves, clearing enclave page cache (EPC), applying a microcode patch that does not require a reboot (via late microcode loading) and restarting all SGX enclaves. Problem statement ------------------------- Even when the above-described runtime recovery flow to mitigate the SGX vulnerability is followed, the SGX attestation evidence will still reflect the security SVN version being equal to the previous state of security SVN (containing vulnerability) that created and managed the enclave until the runtime recovery event. This limitation currently can be only overcome via a platform reboot, which negates all the benefits from the rebootless late microcode loading and not required in this case for functional or security purposes. Proposed solution ----------------- SGX architecture introduced a new instruction called EUPDATESVN [1] to Ice Lake. It allows updating security SVN version, given that EPC is completely empty. The latter is required for security reasons in order to reason that enclave security posture is as secure as the security SVN version of the TCB that created it. This series enables opportunistic execution of EUPDATESVN upon first EPC page allocation for a first enclave to be run on the platform. This series is partly based on the previous work done by Cathy Zhang [2], which attempted to enable forceful destruction of all SGX enclaves and execution of EUPDATESVN upon successful application of any microcode patch. This approach is determined as being too intrusive for the running SGX enclaves, especially taking into account that it would be performed upon *every* microcode patch application regardless if it changes the security SVN version or not (change to the security SVN version is a rare event). Testing ------- Tested on EMR machine using kernel-6.15.0_rc7 & sgx selftests. Also tested on a Kaby Lake machine without EUPDATESVN support. If Google folks in CC can test on their side, it would be greatly appreciated. References ---------- [1] https://cdrdv2.intel.com/v1/dl/getContent/648682?explicitVersion=true [2] https://lore.kernel.org/all/20220520103904.1216-1-cathy.zh...@intel.com/ Elena Reshetova (5): x86/sgx: Introduce a counter to count the sgx_(vepc_)open() x86/cpufeatures: Add X86_FEATURE_SGX_EUPDATESVN feature flag x86/sgx: Define error codes for use by ENCLS[EUPDATESVN] x86/sgx: Implement ENCLS[EUPDATESVN] x86/sgx: Enable automatic SVN updates for SGX enclaves arch/x86/include/asm/cpufeatures.h | 1 + arch/x86/include/asm/sgx.h | 37 +++++--- arch/x86/kernel/cpu/cpuid-deps.c | 1 + arch/x86/kernel/cpu/scattered.c | 1 + arch/x86/kernel/cpu/sgx/driver.c | 22 +++-- arch/x86/kernel/cpu/sgx/encl.c | 1 + arch/x86/kernel/cpu/sgx/encls.h | 5 + arch/x86/kernel/cpu/sgx/main.c | 112 +++++++++++++++++++++++ arch/x86/kernel/cpu/sgx/sgx.h | 3 + arch/x86/kernel/cpu/sgx/virt.c | 16 +++- tools/arch/x86/include/asm/cpufeatures.h | 1 + 11 files changed, 177 insertions(+), 23 deletions(-) -- 2.45.2
