On Wed, 2022-04-13 at 14:10 -0700, Reinette Chatre wrote: > Now that the discussions surrounding the support for SGX2 is settling, > the kselftest audience is added to the discussion for the first time > to consider the testing of the new features. > > V3: https://lore.kernel.org/lkml/cover.1648847675.git.reinette.chatre@xxxxxxxxx/ > > Changes since V3 that directly impact user space: > - SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS ioctl()'s struct > sgx_enclave_restrict_permissions no longer provides entire secinfo, > just the new permissions in new "permissions" struct member. (Jarkko) > - Rename SGX_IOC_ENCLAVE_MODIFY_TYPE ioctl() to > SGX_IOC_ENCLAVE_MODIFY_TYPES. (Jarkko) > - SGX_IOC_ENCLAVE_MODIFY_TYPES ioctl()'s struct sgx_enclave_modify_type > no longer provides entire secinfo, just the new page type in new > "page_type" struct member. (Jarkko) > > Details about changes since V3 that do not directly impact user space: > - Add new patch to enable VA pages to be added without invoking reclaimer > directly if no EPC pages are available, failing instead. This enables > VA pages to be added with enclave's mutex held. Fixes an issue > encountered by Haitao. More details in new patch "x86/sgx: Support VA page > allocation without reclaiming". > - While refactoring, change existing code to consistently use > IS_ALIGNED(). (Jarkko) > - Many patches received a tag from Jarkko. > - Many smaller changes, please refer to individual patches. > > V2: https://lore.kernel.org/lkml/cover.1644274683.git.reinette.chatre@xxxxxxxxx/ > > Changes since V2 that directly impact user space: > - Maximum allowed permissions of dynamically added pages is RWX, > previously limited to RW. (Jarkko) > Dynamically added pages are initially created with architecturally > limited EPCM permissions of RW. mmap() and mprotect() of these pages > with RWX permissions would no longer be blocked by SGX driver. PROT_EXEC > on dynamically added pages will be possible after running ENCLU[EMODPE] > from within the enclave with appropriate VMA permissions. > > - The kernel no longer attempts to track the EPCM runtime permissions. (Jarkko) > Consequences are: > - Kernel does not modify PTEs to follow EPCM permissions. User space > will receive #PF with SGX error code in cases where the V2 > implementation would have resulted in regular (non-SGX) page fault > error code. > - SGX_IOC_ENCLAVE_RELAX_PERMISSIONS is removed. This ioctl() was used > to clear PTEs after permissions were modified from within the enclave > and ensure correct PTEs are installed. Since PTEs no longer track > EPCM permissions the changes in EPCM permissions would not impact PTEs. > As long as new permissions are within the maximum vetted permissions > (vm_max_prot_bits) only ENCLU[EMODPE] from within enclave is needed, > as accompanied by appropriate VMA permissions. > > - struct sgx_enclave_restrict_perm renamed to > sgx_enclave_restrict_permissions (Jarkko) > > - struct sgx_enclave_modt renamed to struct sgx_enclave_modify_type > to be consistent with the verbose naming of other SGX uapi structs. > > Details about changes since V2 that do not directly impact user space: > - Kernel no longer tracks the runtime EPCM permissions with the aim of > installing accurate PTEs. (Jarkko) > - In support of this change the following patches were removed: > Documentation/x86: Document SGX permission details > x86/sgx: Support VMA permissions more relaxed than enclave permissions > x86/sgx: Add pfn_mkwrite() handler for present PTEs > x86/sgx: Add sgx_encl_page->vm_run_prot_bits for dynamic permission changes > x86/sgx: Support relaxing of enclave page permissions > - No more handling of scenarios where VMA permissions may be more > relaxed than what the EPCM allows. Enclaves are not prevented > from accessing such pages and the EPCM permissions are entrusted > to control access as supported by the SGX error code in page faults. > - No more explicit setting of protection bits in page fault handler. > Protection bits are inherited from VMA similar to SGX1 support. > > - Selftest patches are moved to the end of the series. (Jarkko) > > - New patch contributed by Jarkko to avoid duplicated code: > x86/sgx: Export sgx_encl_page_alloc() > > - New patch separating changes from existing patch. (Jarkko) > x86/sgx: Export sgx_encl_{grow,shrink}() > > - New patch to keep one required benefit from the (now removed) kernel > EPCM permission tracking: > x86/sgx: Support loading enclave page without VMA permissions check > > - Updated cover letter to reflect architecture changes. > > - Many smaller changes, please refer to individual patches. > > V1: https://lore.kernel.org/linux-sgx/cover.1638381245.git.reinette.chatre@xxxxxxxxx/ > > Changes since V1 that directly impact user space: > - SGX2 permission changes changed from a single ioctl() named > SGX_IOC_PAGE_MODP to two new ioctl()s: > SGX_IOC_ENCLAVE_RELAX_PERMISSIONS and > SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS, supported by two different > parameter structures (SGX_IOC_ENCLAVE_RELAX_PERMISSIONS does > not support a result output parameter) (Jarkko). > > User space flow impact: After user space runs ENCLU[EMODPE] it > needs to call SGX_IOC_ENCLAVE_RELAX_PERMISSIONS to have PTEs > updated. Previously running SGX_IOC_PAGE_MODP in this scenario > resulted in EPCM.PR being set but calling > SGX_IOC_ENCLAVE_RELAX_PERMISSIONS will not result in EPCM.PR > being set anymore and thus no need for an additional > ENCLU[EACCEPT]. > > - SGX_IOC_ENCLAVE_RELAX_PERMISSIONS and > SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS > obtain new permissions from secinfo as parameter instead of > the permissions directly (Jarkko). > > - ioctl() supporting SGX2 page type change is renamed from > SGX_IOC_PAGE_MODT to SGX_IOC_ENCLAVE_MODIFY_TYPE (Jarkko). > > - SGX_IOC_ENCLAVE_MODIFY_TYPE obtains new page type from secinfo > as parameter instead of the page type directly (Jarkko). > > - ioctl() supporting SGX2 page removal is renamed from > SGX_IOC_PAGE_REMOVE to SGX_IOC_ENCLAVE_REMOVE_PAGES (Jarkko). > > - All ioctl() parameter structures have been renamed as a result of the > ioctl() renaming: > SGX_IOC_ENCLAVE_RELAX_PERMISSIONS => struct sgx_enclave_relax_perm > SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS => struct sgx_enclave_restrict_perm > SGX_IOC_ENCLAVE_MODIFY_TYPE => struct sgx_enclave_modt > SGX_IOC_ENCLAVE_REMOVE_PAGES => struct sgx_enclave_remove_pages > > Changes since V1 that do not directly impact user space: > - Number of patches in series increased from 25 to 32 primarily because > of splitting the original submission: > - Wrappers for the new SGX2 functions are introduced in three separate > patches replacing the original "x86/sgx: Add wrappers for SGX2 > functions" > (Jarkko). > - Moving and renaming sgx_encl_ewb_cpumask() is done with two patches > replacing the original "x86/sgx: Use more generic name for enclave > cpumask function" (Jarkko). > - Support for SGX2 EPCM permission changes is split into two ioctls(), > one for relaxing and one for restricting permissions, each introduced > by a new patch replacing the original "x86/sgx: Support enclave page > permission changes" (Jarkko). > - Extracted code used by existing ioctls() for usage by new ioctl()s > into a new utility in new patch "x86/sgx: Create utility to validate > user provided offset and length" (Dave did not specifically ask for > this but it addresses his review feedback). > - Two new Documentation patches to support the SGX2 work > ("Documentation/x86: Introduce enclave runtime management") and > a dedicated section on the enclave permission management > ("Documentation/x86: Document SGX permission details") (Andy). > - Most patches were reworked to improve the language by: > * aiming to refer to exact item instead of English rephrasing (Jarkko). > * use ioctl() instead of ioctl throughout (Dave). > * Use "relaxed" instead of "exceed" when referring to permissions > (Dave). > - Improved documentation with several additions to > Documentation/x86/sgx.rst. > - Many smaller changes, please refer to individual patches. > > Hi Everybody, > > The current Linux kernel support for SGX includes support for SGX1 that > requires that an enclave be created with properties that accommodate all > usages over its (the enclave's) lifetime. This includes properties such > as permissions of enclave pages, the number of enclave pages, and the > number of threads supported by the enclave. > > Consequences of this requirement to have the enclave be created to > accommodate all usages include: > * pages needing to support relocated code are required to have RWX > permissions for their entire lifetime, > * an enclave needs to be created with the maximum stack and heap > projected to be needed during the enclave's entire lifetime which > can be longer than the processes running within it, > * an enclave needs to be created with support for the maximum number > of threads projected to run in the enclave. > > Since SGX1 a few more functions were introduced, collectively called > SGX2, that support modifications to an initialized enclave. Hardware > supporting these functions are already available as listed on > https://github.com/ayeks/SGX-hardware > > This series adds support for SGX2, also referred to as Enclave Dynamic > Memory Management (EDMM). This includes: > > * Support modifying EPCM permissions of regular enclave pages belonging > to an initialized enclave. Only permission restriction is supported > via a new ioctl() SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS. Relaxing of > EPCM permissions can only be done from within the enclave with the > SGX instruction ENCLU[EMODPE]. > > * Support dynamic addition of regular enclave pages to an initialized > enclave. At creation new pages are architecturally limited to RW EPCM > permissions but will be accessible with PROT_EXEC after the enclave > runs ENCLU[EMODPE] to relax EPCM permissions to RWX. > Pages are dynamically added to an initialized enclave from the SGX > page fault handler. > > * Support expanding an initialized enclave to accommodate more threads. > More threads can be accommodated by an enclave with the addition of > Thread Control Structure (TCS) pages that is done by changing the > type of regular enclave pages to TCS pages using a new ioctl() > SGX_IOC_ENCLAVE_MODIFY_TYPES. > > * Support removing regular and TCS pages from an initialized enclave. > Removing pages is accomplished in two stages as supported by two new > ioctl()s SGX_IOC_ENCLAVE_MODIFY_TYPES (same ioctl() as mentioned in > previous bullet) and SGX_IOC_ENCLAVE_REMOVE_PAGES. > > * Tests covering all the new flows, some edge cases, and one > comprehensive stress scenario. > > No additional work is needed to support SGX2 in a virtualized > environment. All tests included in this series passed when run from > a guest as tested with the recent QEMU release based on 6.2.0 > that supports SGX. > > Patches 1 through 14 prepare the existing code for SGX2 support by > introducing the SGX2 functions, refactoring code, and tracking enclave > page types. > > Patches 15 through 21 enable the SGX2 features and include a > Documentation patch. > > Patches 22 through 31 test several scenarios of all the enabled > SGX2 features. > > This series is based on v5.18-rc2. > > Your feedback will be greatly appreciated. > > Regards, > > Reinette > > Jarkko Sakkinen (1): > x86/sgx: Export sgx_encl_page_alloc() > > Reinette Chatre (30): > x86/sgx: Add short descriptions to ENCLS wrappers > x86/sgx: Add wrapper for SGX2 EMODPR function > x86/sgx: Add wrapper for SGX2 EMODT function > x86/sgx: Add wrapper for SGX2 EAUG function > x86/sgx: Support loading enclave page without VMA permissions check > x86/sgx: Export sgx_encl_ewb_cpumask() > x86/sgx: Rename sgx_encl_ewb_cpumask() as sgx_encl_cpumask() > x86/sgx: Move PTE zap code to new sgx_zap_enclave_ptes() > x86/sgx: Make sgx_ipi_cb() available internally > x86/sgx: Create utility to validate user provided offset and length > x86/sgx: Keep record of SGX page type > x86/sgx: Export sgx_encl_{grow,shrink}() > x86/sgx: Support VA page allocation without reclaiming > x86/sgx: Support restricting of enclave page permissions > x86/sgx: Support adding of pages to an initialized enclave > x86/sgx: Tighten accessible memory range after enclave initialization > x86/sgx: Support modifying SGX page type > x86/sgx: Support complete page removal > x86/sgx: Free up EPC pages directly to support large page ranges > Documentation/x86: Introduce enclave runtime management section > selftests/sgx: Add test for EPCM permission changes > selftests/sgx: Add test for TCS page permission changes > selftests/sgx: Test two different SGX2 EAUG flows > selftests/sgx: Introduce dynamic entry point > selftests/sgx: Introduce TCS initialization enclave operation > selftests/sgx: Test complete changing of page type flow > selftests/sgx: Test faulty enclave behavior > selftests/sgx: Test invalid access to removed enclave page > selftests/sgx: Test reclaiming of untouched page > selftests/sgx: Page removal stress test > > Documentation/x86/sgx.rst | 15 + > arch/x86/include/asm/sgx.h | 8 + > arch/x86/include/uapi/asm/sgx.h | 61 + > arch/x86/kernel/cpu/sgx/encl.c | 329 +++- > arch/x86/kernel/cpu/sgx/encl.h | 15 +- > arch/x86/kernel/cpu/sgx/encls.h | 33 + > arch/x86/kernel/cpu/sgx/ioctl.c | 640 +++++++- > arch/x86/kernel/cpu/sgx/main.c | 75 +- > arch/x86/kernel/cpu/sgx/sgx.h | 3 + > tools/testing/selftests/sgx/defines.h | 23 + > tools/testing/selftests/sgx/load.c | 41 + > tools/testing/selftests/sgx/main.c | 1435 +++++++++++++++++ > tools/testing/selftests/sgx/main.h | 1 + > tools/testing/selftests/sgx/test_encl.c | 68 + > .../selftests/sgx/test_encl_bootstrap.S | 6 + > 15 files changed, 2625 insertions(+), 128 deletions(-) > > > base-commit: ce522ba9ef7e2d9fb22a39eb3371c0c64e2a433e IMHO, we can pull this after +1 version. I think I had only one nit (one character to a struct name it was), and I've been testing this series *extensively* with real-world code (wasm run-time that we are developing), so I'm confident that it is *good enough*. Reinette, for the EMODT patch, as long as you fix the struct name you can add my reviewed-by and also tested-by to that patch before you send it! It's so narrow change. BR, Jarkko
