On Tue, 2022-05-10 at 11:08 -0700, Reinette Chatre wrote: > V4: https://lore.kernel.org/lkml/cover.1649878359.git.reinette.chatre@xxxxxxxxx/ > > Changes since V4 that directly impact user space: > - SGX_IOC_ENCLAVE_MODIFY_TYPES ioctl()'s struct was renamed > from struct sgx_enclave_modify_type to > struct sgx_enclave_modify_types. (Jarkko) > > Details about changes since V4 that do not directly impact user space: > - Related function names were changed to match with the struct name > change: > sgx_ioc_enclave_modify_type() -> sgx_ioc_enclave_modify_types() > sgx_enclave_modify_type() -> sgx_enclave_modify_types() > - Revert a SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS parameter check that > requires read permission. The hardware does support restricting > enclave page permission to zero permissions. Replace with > permission check to ensure read permission is set when write permission > is set. This is verified early to prevent a later fault of the > instruction. (Vijay). > - Do not attempt direct reclaim if no EPC pages available during page > fault. mmap_lock is already held in page fault handler so attempting > to take it again while running sgx_reclaim_pages() has risk of > deadlock. This was discovered by lockdep during stress testing. > - Pick up Reviewed-by and Tested-by tags from Jarkko. > - Pick up Tested-by tags from Haitao after testing with Intel SGX SDK/PSW. > - Pick up Tested-by tags from Vijay after testing with Gramine. > > 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-rc5 with recently submitted SGX shmem > fixes applied: > https://lore.kernel.org/linux-sgx/cover.1652131695.git.reinette.chatre@xxxxxxxxx/ > A repo with both series applied is available: > repo: https://github.com/rchatre/linux.git > branch: sgx/sgx2_submitted_v5_plus_rwx > > This SGX2 series also applies directly to v5.18-rc5 if done with a 3-way merge > since it and the shmem fixes both make changes to arch/x86/kernel/cpu/sgx/encl.h > but do not have direct conflicts. > > 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 | 62 + > 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 | 641 +++++++- > 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, 2627 insertions(+), 128 deletions(-) > > > base-commit: 672c0c5173427e6b3e2a9bbb7be51ceeec78093a > prerequisite-patch-id: 1a738c00922b0ec865f2674c6f4f8be9ff9b1aab > prerequisite-patch-id: 792889ea9bdfae8c150b1be5c16da697bc404422 > prerequisite-patch-id: 78ed2d6251ead724bcb96e0f058bb39dca9eba04 > prerequisite-patch-id: cbb715e565631a146eb3cd902455ebaa5d489872 > prerequisite-patch-id: 3e853bae87d94f8695a48c537ef32a516f415933 Is there something preventing to take this into v5.19 merge window? I don't think this can improve too much out-of-tree anymore. BR, Jarkko
