2022-05-10 20:18:25

by Reinette Chatre

[permalink] [raw]
Subject: [PATCH V5 00/31] x86/sgx and selftests/sgx: Support SGX2

V4: https://lore.kernel.org/lkml/[email protected]/

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/[email protected]/

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/[email protected]/

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/[email protected]/

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/[email protected]/
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
--
2.25.1



2022-05-10 20:37:40

by Reinette Chatre

[permalink] [raw]
Subject: [PATCH V5 20/31] x86/sgx: Free up EPC pages directly to support large page ranges

The page reclaimer ensures availability of EPC pages across all
enclaves. In support of this it runs independently from the
individual enclaves in order to take locks from the different
enclaves as it writes pages to swap.

When needing to load a page from swap an EPC page needs to be
available for its contents to be loaded into. Loading an existing
enclave page from swap does not reclaim EPC pages directly if
none are available, instead the reclaimer is woken when the
available EPC pages are found to be below a watermark.

When iterating over a large number of pages in an oversubscribed
environment there is a race between the reclaimer woken up and
EPC pages reclaimed fast enough for the page operations to proceed.

Ensure there are EPC pages available before attempting to load
a page that may potentially be pulled from swap into an available
EPC page.

Acked-by: Jarkko Sakkinen <[email protected]>
Signed-off-by: Reinette Chatre <[email protected]>
---
No changes since V4.

Changes since V3:
- Add Jarkko's Acked-by tag.
- Rename sgx_direct_reclaim() to sgx_reclaim_direct(). (Jarkko)
- Add Dave's provided comments to sgx_reclaim_direct(). (Dave)

Changes since v1:
- Reword commit message.

arch/x86/kernel/cpu/sgx/ioctl.c | 6 ++++++
arch/x86/kernel/cpu/sgx/main.c | 11 +++++++++++
arch/x86/kernel/cpu/sgx/sgx.h | 1 +
3 files changed, 18 insertions(+)

diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
index 1a2595f261d3..ebe79d60619f 100644
--- a/arch/x86/kernel/cpu/sgx/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -745,6 +745,8 @@ sgx_enclave_restrict_permissions(struct sgx_encl *encl,
for (c = 0 ; c < modp->length; c += PAGE_SIZE) {
addr = encl->base + modp->offset + c;

+ sgx_reclaim_direct();
+
mutex_lock(&encl->lock);

entry = sgx_encl_load_page(encl, addr);
@@ -910,6 +912,8 @@ static long sgx_enclave_modify_types(struct sgx_encl *encl,
for (c = 0 ; c < modt->length; c += PAGE_SIZE) {
addr = encl->base + modt->offset + c;

+ sgx_reclaim_direct();
+
mutex_lock(&encl->lock);

entry = sgx_encl_load_page(encl, addr);
@@ -1096,6 +1100,8 @@ static long sgx_encl_remove_pages(struct sgx_encl *encl,
for (c = 0 ; c < params->length; c += PAGE_SIZE) {
addr = encl->base + params->offset + c;

+ sgx_reclaim_direct();
+
mutex_lock(&encl->lock);

entry = sgx_encl_load_page(encl, addr);
diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
index bab2a2e13c70..1dc4a1516bf1 100644
--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -375,6 +375,17 @@ static bool sgx_should_reclaim(unsigned long watermark)
!list_empty(&sgx_active_page_list);
}

+/*
+ * sgx_reclaim_direct() should be called (without enclave's mutex held)
+ * in locations where SGX memory resources might be low and might be
+ * needed in order to make forward progress.
+ */
+void sgx_reclaim_direct(void)
+{
+ if (sgx_should_reclaim(SGX_NR_LOW_PAGES))
+ sgx_reclaim_pages();
+}
+
static int ksgxd(void *p)
{
set_freezable();
diff --git a/arch/x86/kernel/cpu/sgx/sgx.h b/arch/x86/kernel/cpu/sgx/sgx.h
index b30cee4de903..0f2020653fba 100644
--- a/arch/x86/kernel/cpu/sgx/sgx.h
+++ b/arch/x86/kernel/cpu/sgx/sgx.h
@@ -86,6 +86,7 @@ static inline void *sgx_get_epc_virt_addr(struct sgx_epc_page *page)
struct sgx_epc_page *__sgx_alloc_epc_page(void);
void sgx_free_epc_page(struct sgx_epc_page *page);

+void sgx_reclaim_direct(void);
void sgx_mark_page_reclaimable(struct sgx_epc_page *page);
int sgx_unmark_page_reclaimable(struct sgx_epc_page *page);
struct sgx_epc_page *sgx_alloc_epc_page(void *owner, bool reclaim);
--
2.25.1


2022-05-10 20:39:47

by Reinette Chatre

[permalink] [raw]
Subject: [PATCH V5 04/31] x86/sgx: Add wrapper for SGX2 EAUG function

Add a wrapper for the EAUG ENCLS leaf function used to
add a page to an initialized enclave.

EAUG:
1) Stores all properties of the new enclave page in the SGX
hardware's Enclave Page Cache Map (EPCM).
2) Sets the PENDING bit in the EPCM entry of the enclave page.
This bit is cleared by the enclave by invoking ENCLU leaf
function EACCEPT or EACCEPTCOPY.

Access from within the enclave to the new enclave page is not
possible until the PENDING bit is cleared.

Reviewed-by: Jarkko Sakkinen <[email protected]>
Signed-off-by: Reinette Chatre <[email protected]>
---
No changes since V4.

Changes since V3:
- Add Jarkko's Reviewed-by tag.

Changes since V1:
- Split original patch ("x86/sgx: Add wrappers for SGX2 functions")
in three to introduce the SGX2 functions separately (Jarkko).
- Rewrite commit message to include how the EPCM within the hardware
is changed by the SGX2 function as well as any calling
conditions (Jarkko).

arch/x86/kernel/cpu/sgx/encls.h | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/arch/x86/kernel/cpu/sgx/encls.h b/arch/x86/kernel/cpu/sgx/encls.h
index 7a1ecf704ec1..99004b02e2ed 100644
--- a/arch/x86/kernel/cpu/sgx/encls.h
+++ b/arch/x86/kernel/cpu/sgx/encls.h
@@ -227,4 +227,10 @@ static inline int __emodt(struct sgx_secinfo *secinfo, void *addr)
return __encls_ret_2(EMODT, secinfo, addr);
}

+/* Zero a page of EPC memory and add it to an initialized enclave. */
+static inline int __eaug(struct sgx_pageinfo *pginfo, void *addr)
+{
+ return __encls_2(EAUG, pginfo, addr);
+}
+
#endif /* _X86_ENCLS_H */
--
2.25.1


2022-05-10 20:40:51

by Reinette Chatre

[permalink] [raw]
Subject: [PATCH V5 22/31] selftests/sgx: Add test for EPCM permission changes

EPCM permission changes could be made from within (to relax
permissions) or out (to restrict permissions) the enclave. Kernel
support is needed when permissions are restricted to be able to
call the privileged ENCLS[EMODPR] instruction. EPCM permissions
can be relaxed via ENCLU[EMODPE] from within the enclave but the
enclave still depends on the kernel to install PTEs with the needed
permissions.

Add a test that exercises a few of the enclave page permission flows:
1) Test starts with a RW (from enclave and kernel perspective)
enclave page that is mapped via a RW VMA.
2) Use the SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS ioctl() to restrict
the enclave (EPCM) page permissions to read-only.
3) Run ENCLU[EACCEPT] from within the enclave to accept the new page
permissions.
4) Attempt to write to the enclave page from within the enclave - this
should fail with a page fault on the EPCM permissions since the page
table entry continues to allow RW access.
5) Restore EPCM permissions to RW by running ENCLU[EMODPE] from within
the enclave.
6) Attempt to write to the enclave page from within the enclave - this
should succeed since both EPCM and PTE permissions allow this access.

Acked-by: Jarkko Sakkinen <[email protected]>
Signed-off-by: Reinette Chatre <[email protected]>
---
No changes since V4.

Changes since V3:
- Add Jarkko's Acked-by tag.
- User provides only new permissions in
SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS ioctl(), replacing secinfo. (Jarkko)
- Use SGX page permission bits instead of VMA protection bits.

Changes since V2:
- Modify test to support separation between EPCM and PTE/VMA permissions
- Fix changelog and comments to reflect new relationship between
EPCM and PTE/VMA permissions.
- With EPCM permissions controlling access instead of PTE permissions,
check for SGX error code now encountered in page fault.
- Stop calling SGX_IOC_ENCLAVE_RELAX_PERMISSIONS and ensure that
only calling ENCLU[EMODPE] from within enclave is necessary to restore
access to the enclave page.
- Update to use new struct name struct sgx_enclave_restrict_perm -> struct
sgx_enclave_restrict_permissions. (Jarkko)

Changes since V1:
- Adapt test to the kernel interface changes: the ioctl() name change
and providing entire secinfo as parameter.
- Remove the ENCLU[EACCEPT] call after permissions are relaxed since
the new flow no longer results in the EPCM PR bit being set.
- Rewrite error path to reduce line lengths.

tools/testing/selftests/sgx/defines.h | 15 ++
tools/testing/selftests/sgx/main.c | 214 ++++++++++++++++++++++++
tools/testing/selftests/sgx/test_encl.c | 38 +++++
3 files changed, 267 insertions(+)

diff --git a/tools/testing/selftests/sgx/defines.h b/tools/testing/selftests/sgx/defines.h
index 02d775789ea7..b638eb98c80c 100644
--- a/tools/testing/selftests/sgx/defines.h
+++ b/tools/testing/selftests/sgx/defines.h
@@ -24,6 +24,8 @@ enum encl_op_type {
ENCL_OP_PUT_TO_ADDRESS,
ENCL_OP_GET_FROM_ADDRESS,
ENCL_OP_NOP,
+ ENCL_OP_EACCEPT,
+ ENCL_OP_EMODPE,
ENCL_OP_MAX,
};

@@ -53,4 +55,17 @@ struct encl_op_get_from_addr {
uint64_t addr;
};

+struct encl_op_eaccept {
+ struct encl_op_header header;
+ uint64_t epc_addr;
+ uint64_t flags;
+ uint64_t ret;
+};
+
+struct encl_op_emodpe {
+ struct encl_op_header header;
+ uint64_t epc_addr;
+ uint64_t flags;
+};
+
#endif /* DEFINES_H */
diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c
index dd74fa42302e..46eac09cd955 100644
--- a/tools/testing/selftests/sgx/main.c
+++ b/tools/testing/selftests/sgx/main.c
@@ -25,6 +25,18 @@ static const uint64_t MAGIC = 0x1122334455667788ULL;
static const uint64_t MAGIC2 = 0x8877665544332211ULL;
vdso_sgx_enter_enclave_t vdso_sgx_enter_enclave;

+/*
+ * Security Information (SECINFO) data structure needed by a few SGX
+ * instructions (eg. ENCLU[EACCEPT] and ENCLU[EMODPE]) holds meta-data
+ * about an enclave page. &enum sgx_secinfo_page_state specifies the
+ * secinfo flags used for page state.
+ */
+enum sgx_secinfo_page_state {
+ SGX_SECINFO_PENDING = (1 << 3),
+ SGX_SECINFO_MODIFIED = (1 << 4),
+ SGX_SECINFO_PR = (1 << 5),
+};
+
struct vdso_symtab {
Elf64_Sym *elf_symtab;
const char *elf_symstrtab;
@@ -555,4 +567,206 @@ TEST_F(enclave, pte_permissions)
EXPECT_EQ(self->run.exception_addr, 0);
}

+/*
+ * Enclave page permission test.
+ *
+ * Modify and restore enclave page's EPCM (enclave) permissions from
+ * outside enclave (ENCLS[EMODPR] via kernel) as well as from within
+ * enclave (via ENCLU[EMODPE]). Check for page fault if
+ * VMA allows access but EPCM permissions do not.
+ */
+TEST_F(enclave, epcm_permissions)
+{
+ struct sgx_enclave_restrict_permissions restrict_ioc;
+ struct encl_op_get_from_addr get_addr_op;
+ struct encl_op_put_to_addr put_addr_op;
+ struct encl_op_eaccept eaccept_op;
+ struct encl_op_emodpe emodpe_op;
+ unsigned long data_start;
+ int ret, errno_save;
+
+ ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata));
+
+ memset(&self->run, 0, sizeof(self->run));
+ self->run.tcs = self->encl.encl_base;
+
+ /*
+ * Ensure kernel supports needed ioctl() and system supports needed
+ * commands.
+ */
+ memset(&restrict_ioc, 0, sizeof(restrict_ioc));
+
+ ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS,
+ &restrict_ioc);
+ errno_save = ret == -1 ? errno : 0;
+
+ /*
+ * Invalid parameters were provided during sanity check,
+ * expect command to fail.
+ */
+ ASSERT_EQ(ret, -1);
+
+ /* ret == -1 */
+ if (errno_save == ENOTTY)
+ SKIP(return,
+ "Kernel does not support SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS ioctl()");
+ else if (errno_save == ENODEV)
+ SKIP(return, "System does not support SGX2");
+
+ /*
+ * Page that will have its permissions changed is the second data
+ * page in the .data segment. This forms part of the local encl_buffer
+ * within the enclave.
+ *
+ * At start of test @data_start should have EPCM as well as PTE and
+ * VMA permissions of RW.
+ */
+
+ data_start = self->encl.encl_base +
+ encl_get_data_offset(&self->encl) + PAGE_SIZE;
+
+ /*
+ * Sanity check that page at @data_start is writable before making
+ * any changes to page permissions.
+ *
+ * Start by writing MAGIC to test page.
+ */
+ put_addr_op.value = MAGIC;
+ put_addr_op.addr = data_start;
+ put_addr_op.header.type = ENCL_OP_PUT_TO_ADDRESS;
+
+ EXPECT_EQ(ENCL_CALL(&put_addr_op, &self->run, true), 0);
+
+ EXPECT_EEXIT(&self->run);
+ EXPECT_EQ(self->run.exception_vector, 0);
+ EXPECT_EQ(self->run.exception_error_code, 0);
+ EXPECT_EQ(self->run.exception_addr, 0);
+
+ /*
+ * Read memory that was just written to, confirming that
+ * page is writable.
+ */
+ get_addr_op.value = 0;
+ get_addr_op.addr = data_start;
+ get_addr_op.header.type = ENCL_OP_GET_FROM_ADDRESS;
+
+ EXPECT_EQ(ENCL_CALL(&get_addr_op, &self->run, true), 0);
+
+ EXPECT_EQ(get_addr_op.value, MAGIC);
+ EXPECT_EEXIT(&self->run);
+ EXPECT_EQ(self->run.exception_vector, 0);
+ EXPECT_EQ(self->run.exception_error_code, 0);
+ EXPECT_EQ(self->run.exception_addr, 0);
+
+ /*
+ * Change EPCM permissions to read-only. Kernel still considers
+ * the page writable.
+ */
+ memset(&restrict_ioc, 0, sizeof(restrict_ioc));
+
+ restrict_ioc.offset = encl_get_data_offset(&self->encl) + PAGE_SIZE;
+ restrict_ioc.length = PAGE_SIZE;
+ restrict_ioc.permissions = SGX_SECINFO_R;
+
+ ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS,
+ &restrict_ioc);
+ errno_save = ret == -1 ? errno : 0;
+
+ EXPECT_EQ(ret, 0);
+ EXPECT_EQ(errno_save, 0);
+ EXPECT_EQ(restrict_ioc.result, 0);
+ EXPECT_EQ(restrict_ioc.count, 4096);
+
+ /*
+ * EPCM permissions changed from kernel, need to EACCEPT from enclave.
+ */
+ eaccept_op.epc_addr = data_start;
+ eaccept_op.flags = SGX_SECINFO_R | SGX_SECINFO_REG | SGX_SECINFO_PR;
+ eaccept_op.ret = 0;
+ eaccept_op.header.type = ENCL_OP_EACCEPT;
+
+ EXPECT_EQ(ENCL_CALL(&eaccept_op, &self->run, true), 0);
+
+ EXPECT_EEXIT(&self->run);
+ EXPECT_EQ(self->run.exception_vector, 0);
+ EXPECT_EQ(self->run.exception_error_code, 0);
+ EXPECT_EQ(self->run.exception_addr, 0);
+ EXPECT_EQ(eaccept_op.ret, 0);
+
+ /*
+ * EPCM permissions of page is now read-only, expect #PF
+ * on EPCM when attempting to write to page from within enclave.
+ */
+ put_addr_op.value = MAGIC2;
+
+ EXPECT_EQ(ENCL_CALL(&put_addr_op, &self->run, true), 0);
+
+ EXPECT_EQ(self->run.function, ERESUME);
+ EXPECT_EQ(self->run.exception_vector, 14);
+ EXPECT_EQ(self->run.exception_error_code, 0x8007);
+ EXPECT_EQ(self->run.exception_addr, data_start);
+
+ self->run.exception_vector = 0;
+ self->run.exception_error_code = 0;
+ self->run.exception_addr = 0;
+
+ /*
+ * Received AEX but cannot return to enclave at same entrypoint,
+ * need different TCS from where EPCM permission can be made writable
+ * again.
+ */
+ self->run.tcs = self->encl.encl_base + PAGE_SIZE;
+
+ /*
+ * Enter enclave at new TCS to change EPCM permissions to be
+ * writable again and thus fix the page fault that triggered the
+ * AEX.
+ */
+
+ emodpe_op.epc_addr = data_start;
+ emodpe_op.flags = SGX_SECINFO_R | SGX_SECINFO_W;
+ emodpe_op.header.type = ENCL_OP_EMODPE;
+
+ EXPECT_EQ(ENCL_CALL(&emodpe_op, &self->run, true), 0);
+
+ EXPECT_EEXIT(&self->run);
+ EXPECT_EQ(self->run.exception_vector, 0);
+ EXPECT_EQ(self->run.exception_error_code, 0);
+ EXPECT_EQ(self->run.exception_addr, 0);
+
+ /*
+ * Attempt to return to main TCS to resume execution at faulting
+ * instruction, PTE should continue to allow writing to the page.
+ */
+ self->run.tcs = self->encl.encl_base;
+
+ /*
+ * Wrong page permissions that caused original fault has
+ * now been fixed via EPCM permissions.
+ * Resume execution in main TCS to re-attempt the memory access.
+ */
+ self->run.tcs = self->encl.encl_base;
+
+ EXPECT_EQ(vdso_sgx_enter_enclave((unsigned long)&put_addr_op, 0, 0,
+ ERESUME, 0, 0,
+ &self->run),
+ 0);
+
+ EXPECT_EEXIT(&self->run);
+ EXPECT_EQ(self->run.exception_vector, 0);
+ EXPECT_EQ(self->run.exception_error_code, 0);
+ EXPECT_EQ(self->run.exception_addr, 0);
+
+ get_addr_op.value = 0;
+
+ EXPECT_EQ(ENCL_CALL(&get_addr_op, &self->run, true), 0);
+
+ EXPECT_EQ(get_addr_op.value, MAGIC2);
+ EXPECT_EEXIT(&self->run);
+ EXPECT_EQ(self->run.user_data, 0);
+ EXPECT_EQ(self->run.exception_vector, 0);
+ EXPECT_EQ(self->run.exception_error_code, 0);
+ EXPECT_EQ(self->run.exception_addr, 0);
+}
+
TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/sgx/test_encl.c b/tools/testing/selftests/sgx/test_encl.c
index 4fca01cfd898..5b6c65331527 100644
--- a/tools/testing/selftests/sgx/test_encl.c
+++ b/tools/testing/selftests/sgx/test_encl.c
@@ -11,6 +11,42 @@
*/
static uint8_t encl_buffer[8192] = { 1 };

+enum sgx_enclu_function {
+ EACCEPT = 0x5,
+ EMODPE = 0x6,
+};
+
+static void do_encl_emodpe(void *_op)
+{
+ struct sgx_secinfo secinfo __aligned(sizeof(struct sgx_secinfo)) = {0};
+ struct encl_op_emodpe *op = _op;
+
+ secinfo.flags = op->flags;
+
+ asm volatile(".byte 0x0f, 0x01, 0xd7"
+ :
+ : "a" (EMODPE),
+ "b" (&secinfo),
+ "c" (op->epc_addr));
+}
+
+static void do_encl_eaccept(void *_op)
+{
+ struct sgx_secinfo secinfo __aligned(sizeof(struct sgx_secinfo)) = {0};
+ struct encl_op_eaccept *op = _op;
+ int rax;
+
+ secinfo.flags = op->flags;
+
+ asm volatile(".byte 0x0f, 0x01, 0xd7"
+ : "=a" (rax)
+ : "a" (EACCEPT),
+ "b" (&secinfo),
+ "c" (op->epc_addr));
+
+ op->ret = rax;
+}
+
static void *memcpy(void *dest, const void *src, size_t n)
{
size_t i;
@@ -62,6 +98,8 @@ void encl_body(void *rdi, void *rsi)
do_encl_op_put_to_addr,
do_encl_op_get_from_addr,
do_encl_op_nop,
+ do_encl_eaccept,
+ do_encl_emodpe,
};

struct encl_op_header *op = (struct encl_op_header *)rdi;
--
2.25.1


2022-05-10 20:49:19

by Reinette Chatre

[permalink] [raw]
Subject: [PATCH V5 21/31] Documentation/x86: Introduce enclave runtime management section

Enclave runtime management is introduced following the pattern
of the section describing enclave building. Provide a brief
summary of enclave runtime management, pointing to the functions
implementing the ioctl()s that will contain details within their
kernel-doc.

Reviewed-by: Jarkko Sakkinen <[email protected]>
Signed-off-by: Reinette Chatre <[email protected]>
---
Changes since V4:
- Rename sgx_ioc_enclave_modify_type -> sgx_ioc_enclave_modify_types.
(Jarkko)
- Add Jarkko's Reviewed-by tag.

Changes since V2:
- Remove references to ioctl() to relax permissions and update to reflect
function renaming sgx_ioc_enclave_restrict_perm() ->
sgx_ioc_enclave_restrict_permissions().
- Rename sgx_ioc_enclave_modt -> sgx_ioc_enclave_modify_type

Changes since V1:
- New patch.

Documentation/x86/sgx.rst | 15 +++++++++++++++
1 file changed, 15 insertions(+)

diff --git a/Documentation/x86/sgx.rst b/Documentation/x86/sgx.rst
index 265568a9292c..2bcbffacbed5 100644
--- a/Documentation/x86/sgx.rst
+++ b/Documentation/x86/sgx.rst
@@ -100,6 +100,21 @@ pages and establish enclave page permissions.
sgx_ioc_enclave_init
sgx_ioc_enclave_provision

+Enclave runtime management
+--------------------------
+
+Systems supporting SGX2 additionally support changes to initialized
+enclaves: modifying enclave page permissions and type, and dynamically
+adding and removing of enclave pages. When an enclave accesses an address
+within its address range that does not have a backing page then a new
+regular page will be dynamically added to the enclave. The enclave is
+still required to run EACCEPT on the new page before it can be used.
+
+.. kernel-doc:: arch/x86/kernel/cpu/sgx/ioctl.c
+ :functions: sgx_ioc_enclave_restrict_permissions
+ sgx_ioc_enclave_modify_types
+ sgx_ioc_enclave_remove_pages
+
Enclave vDSO
------------

--
2.25.1


2022-05-10 20:49:26

by Reinette Chatre

[permalink] [raw]
Subject: [PATCH V5 02/31] x86/sgx: Add wrapper for SGX2 EMODPR function

Add a wrapper for the EMODPR ENCLS leaf function used to
restrict enclave page permissions as maintained in the
SGX hardware's Enclave Page Cache Map (EPCM).

EMODPR:
1) Updates the EPCM permissions of an enclave page by treating
the new permissions as a mask. Supplying a value that attempts
to relax EPCM permissions has no effect on EPCM permissions
(PR bit, see below, is changed).
2) Sets the PR bit in the EPCM entry of the enclave page to
indicate that permission restriction is in progress. The bit
is reset by the enclave by invoking ENCLU leaf function
EACCEPT or EACCEPTCOPY.

The enclave may access the page throughout the entire process
if conforming to the EPCM permissions for the enclave page.

After performing the permission restriction by issuing EMODPR
the kernel needs to collaborate with the hardware to ensure that
all logical processors sees the new restricted permissions. This
is required for the enclave's EACCEPT/EACCEPTCOPY to succeed and
is accomplished with the ETRACK flow.

Expand enum sgx_return_code with the possible EMODPR return
values.

Reviewed-by: Jarkko Sakkinen <[email protected]>
Signed-off-by: Reinette Chatre <[email protected]>
---
No changes since V4.

Changes since V3:
- Add Jarkko's Reviewed-by tag.

Changes since V2:
- Add detail to changelog that PR bit is set when EPCM permissions
not changed when relaxing of permissions using EMODPR attempted.

Changes since V1:
- Split original patch ("x86/sgx: Add wrappers for SGX2 functions")
in three to introduce the SGX2 functions separately (Jarkko).
- Rewrite commit message to include how the EPCM within the hardware
is changed by the SGX2 function as well as the calling
conditions (Jarkko).
- Make short description more specific to which permissions (EPCM
permissions) the function modifies.

arch/x86/include/asm/sgx.h | 5 +++++
arch/x86/kernel/cpu/sgx/encls.h | 6 ++++++
2 files changed, 11 insertions(+)

diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
index 3f9334ef67cd..d67810b50a81 100644
--- a/arch/x86/include/asm/sgx.h
+++ b/arch/x86/include/asm/sgx.h
@@ -65,17 +65,22 @@ enum sgx_encls_function {

/**
* enum sgx_return_code - The return code type for ENCLS, ENCLU and ENCLV
+ * %SGX_EPC_PAGE_CONFLICT: Page is being written by other ENCLS function.
* %SGX_NOT_TRACKED: Previous ETRACK's shootdown sequence has not
* been completed yet.
* %SGX_CHILD_PRESENT SECS has child pages present in the EPC.
* %SGX_INVALID_EINITTOKEN: EINITTOKEN is invalid and enclave signer's
* public key does not match IA32_SGXLEPUBKEYHASH.
+ * %SGX_PAGE_NOT_MODIFIABLE: The EPC page cannot be modified because it
+ * is in the PENDING or MODIFIED state.
* %SGX_UNMASKED_EVENT: An unmasked event, e.g. INTR, was received
*/
enum sgx_return_code {
+ SGX_EPC_PAGE_CONFLICT = 7,
SGX_NOT_TRACKED = 11,
SGX_CHILD_PRESENT = 13,
SGX_INVALID_EINITTOKEN = 16,
+ SGX_PAGE_NOT_MODIFIABLE = 20,
SGX_UNMASKED_EVENT = 128,
};

diff --git a/arch/x86/kernel/cpu/sgx/encls.h b/arch/x86/kernel/cpu/sgx/encls.h
index 0e22fa8f77c5..2b091912f038 100644
--- a/arch/x86/kernel/cpu/sgx/encls.h
+++ b/arch/x86/kernel/cpu/sgx/encls.h
@@ -215,4 +215,10 @@ static inline int __ewb(struct sgx_pageinfo *pginfo, void *addr,
return __encls_ret_3(EWB, pginfo, addr, va);
}

+/* Restrict the EPCM permissions of an EPC page. */
+static inline int __emodpr(struct sgx_secinfo *secinfo, void *addr)
+{
+ return __encls_ret_2(EMODPR, secinfo, addr);
+}
+
#endif /* _X86_ENCLS_H */
--
2.25.1


2022-05-10 20:56:35

by Reinette Chatre

[permalink] [raw]
Subject: [PATCH V5 27/31] selftests/sgx: Test complete changing of page type flow

Support for changing an enclave page's type enables an initialized
enclave to be expanded with support for more threads by changing the
type of a regular enclave page to that of a Thread Control Structure
(TCS). Additionally, being able to change a TCS or regular enclave
page's type to be trimmed (SGX_PAGE_TYPE_TRIM) initiates the removal
of the page from the enclave.

Test changing page type to TCS as well as page removal flows
in two phases: In the first phase support for a new thread is
dynamically added to an initialized enclave and in the second phase
the pages associated with the new thread are removed from the enclave.
As an additional sanity check after the second phase the page used as
a TCS page during the first phase is added back as a regular page and
ensured that it can be written to (which is not possible if it was a
TCS page).

Acked-by: Jarkko Sakkinen <[email protected]>
Signed-off-by: Reinette Chatre <[email protected]>
---
Changes since V4:
- Rename struct sgx_enclave_modify_type ->
struct sgx_enclave_modify_types. (Jarkko)

Changes since V3:
- Add Jarkko's Acked-by tag.
- Rename SGX_IOC_ENCLAVE_MODIFY_TYPE to
SGX_IOC_ENCLAVE_MODIFY_TYPES. (Jarkko)
- User provides just page type to SGX_IOC_ENCLAVE_MODIFY_TYPES ioctl(),
replacing secinfo. (Jarkko)
- Let the SKIP() call involving SGX_IOC_ENCLAVE_MODIFY_TYPES span
two lines to address checkpatch.pl warning triggered by new longer
name.

Changes since V2:
- Rename struct sgx_enclave_modt -> struct sgx_enclave_modify_type

Changes since V1:
- Update to support ioctl() name change (SGX_IOC_PAGE_MODT ->
SGX_IOC_ENCLAVE_MODIFY_TYPE) and provide secinfo as parameter instead
of just page type (Jarkko).
- Update test to reflect page removal ioctl() and struct name change:
SGX_IOC_PAGE_REMOVE->SGX_IOC_ENCLAVE_REMOVE_PAGES,
struct sgx_page_remove -> struct sgx_enclave_remove_pages (Jarkko).
- Use ioctl() instead of ioctl (Dave).

tools/testing/selftests/sgx/load.c | 41 ++++
tools/testing/selftests/sgx/main.c | 343 +++++++++++++++++++++++++++++
tools/testing/selftests/sgx/main.h | 1 +
3 files changed, 385 insertions(+)

diff --git a/tools/testing/selftests/sgx/load.c b/tools/testing/selftests/sgx/load.c
index 006b464c8fc9..94bdeac1cf04 100644
--- a/tools/testing/selftests/sgx/load.c
+++ b/tools/testing/selftests/sgx/load.c
@@ -130,6 +130,47 @@ static bool encl_ioc_add_pages(struct encl *encl, struct encl_segment *seg)
return true;
}

+/*
+ * Parse the enclave code's symbol table to locate and return address of
+ * the provided symbol
+ */
+uint64_t encl_get_entry(struct encl *encl, const char *symbol)
+{
+ Elf64_Shdr *sections;
+ Elf64_Sym *symtab;
+ Elf64_Ehdr *ehdr;
+ char *sym_names;
+ int num_sym;
+ int i;
+
+ ehdr = encl->bin;
+ sections = encl->bin + ehdr->e_shoff;
+
+ for (i = 0; i < ehdr->e_shnum; i++) {
+ if (sections[i].sh_type == SHT_SYMTAB) {
+ symtab = (Elf64_Sym *)((char *)encl->bin + sections[i].sh_offset);
+ num_sym = sections[i].sh_size / sections[i].sh_entsize;
+ break;
+ }
+ }
+
+ for (i = 0; i < ehdr->e_shnum; i++) {
+ if (sections[i].sh_type == SHT_STRTAB) {
+ sym_names = (char *)encl->bin + sections[i].sh_offset;
+ break;
+ }
+ }
+
+ for (i = 0; i < num_sym; i++) {
+ Elf64_Sym *sym = &symtab[i];
+
+ if (!strcmp(symbol, sym_names + sym->st_name))
+ return (uint64_t)sym->st_value;
+ }
+
+ return 0;
+}
+
bool encl_load(const char *path, struct encl *encl, unsigned long heap_size)
{
const char device_path[] = "/dev/sgx_enclave";
diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c
index 79c08e347112..8bf43646e0bb 100644
--- a/tools/testing/selftests/sgx/main.c
+++ b/tools/testing/selftests/sgx/main.c
@@ -1090,4 +1090,347 @@ TEST_F(enclave, augment_via_eaccept)
munmap(addr, PAGE_SIZE);
}

+/*
+ * SGX2 page type modification test in two phases:
+ * Phase 1:
+ * Create a new TCS, consisting out of three new pages (stack page with regular
+ * page type, SSA page with regular page type, and TCS page with TCS page
+ * type) in an initialized enclave and run a simple workload within it.
+ * Phase 2:
+ * Remove the three pages added in phase 1, add a new regular page at the
+ * same address that previously hosted the TCS page and verify that it can
+ * be modified.
+ */
+TEST_F(enclave, tcs_create)
+{
+ struct encl_op_init_tcs_page init_tcs_page_op;
+ struct sgx_enclave_remove_pages remove_ioc;
+ struct encl_op_get_from_addr get_addr_op;
+ struct sgx_enclave_modify_types modt_ioc;
+ struct encl_op_put_to_addr put_addr_op;
+ struct encl_op_get_from_buf get_buf_op;
+ struct encl_op_put_to_buf put_buf_op;
+ void *addr, *tcs, *stack_end, *ssa;
+ struct encl_op_eaccept eaccept_op;
+ size_t total_size = 0;
+ uint64_t val_64;
+ int errno_save;
+ int ret, i;
+
+ ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl,
+ _metadata));
+
+ memset(&self->run, 0, sizeof(self->run));
+ self->run.tcs = self->encl.encl_base;
+
+ /*
+ * Hardware (SGX2) and kernel support is needed for this test. Start
+ * with check that test has a chance of succeeding.
+ */
+ memset(&modt_ioc, 0, sizeof(modt_ioc));
+ ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_MODIFY_TYPES, &modt_ioc);
+
+ if (ret == -1) {
+ if (errno == ENOTTY)
+ SKIP(return,
+ "Kernel does not support SGX_IOC_ENCLAVE_MODIFY_TYPES ioctl()");
+ else if (errno == ENODEV)
+ SKIP(return, "System does not support SGX2");
+ }
+
+ /*
+ * Invalid parameters were provided during sanity check,
+ * expect command to fail.
+ */
+ EXPECT_EQ(ret, -1);
+
+ /*
+ * Add three regular pages via EAUG: one will be the TCS stack, one
+ * will be the TCS SSA, and one will be the new TCS. The stack and
+ * SSA will remain as regular pages, the TCS page will need its
+ * type changed after populated with needed data.
+ */
+ for (i = 0; i < self->encl.nr_segments; i++) {
+ struct encl_segment *seg = &self->encl.segment_tbl[i];
+
+ total_size += seg->size;
+ }
+
+ /*
+ * Actual enclave size is expected to be larger than the loaded
+ * test enclave since enclave size must be a power of 2 in bytes while
+ * test_encl does not consume it all.
+ */
+ EXPECT_LT(total_size + 3 * PAGE_SIZE, self->encl.encl_size);
+
+ /*
+ * mmap() three pages at end of existing enclave to be used for the
+ * three new pages.
+ */
+ addr = mmap((void *)self->encl.encl_base + total_size, 3 * PAGE_SIZE,
+ PROT_READ | PROT_WRITE, MAP_SHARED | MAP_FIXED,
+ self->encl.fd, 0);
+ EXPECT_NE(addr, MAP_FAILED);
+
+ self->run.exception_vector = 0;
+ self->run.exception_error_code = 0;
+ self->run.exception_addr = 0;
+
+ stack_end = (void *)self->encl.encl_base + total_size;
+ tcs = (void *)self->encl.encl_base + total_size + PAGE_SIZE;
+ ssa = (void *)self->encl.encl_base + total_size + 2 * PAGE_SIZE;
+
+ /*
+ * Run EACCEPT on each new page to trigger the
+ * EACCEPT->(#PF)->EAUG->EACCEPT(again without a #PF) flow.
+ */
+
+ eaccept_op.epc_addr = (unsigned long)stack_end;
+ eaccept_op.flags = SGX_SECINFO_R | SGX_SECINFO_W | SGX_SECINFO_REG | SGX_SECINFO_PENDING;
+ eaccept_op.ret = 0;
+ eaccept_op.header.type = ENCL_OP_EACCEPT;
+
+ EXPECT_EQ(ENCL_CALL(&eaccept_op, &self->run, true), 0);
+
+ if (self->run.exception_vector == 14 &&
+ self->run.exception_error_code == 4 &&
+ self->run.exception_addr == (unsigned long)stack_end) {
+ munmap(addr, 3 * PAGE_SIZE);
+ SKIP(return, "Kernel does not support adding pages to initialized enclave");
+ }
+
+ EXPECT_EEXIT(&self->run);
+ EXPECT_EQ(self->run.exception_vector, 0);
+ EXPECT_EQ(self->run.exception_error_code, 0);
+ EXPECT_EQ(self->run.exception_addr, 0);
+ EXPECT_EQ(eaccept_op.ret, 0);
+
+ eaccept_op.epc_addr = (unsigned long)ssa;
+
+ EXPECT_EQ(ENCL_CALL(&eaccept_op, &self->run, true), 0);
+
+ EXPECT_EEXIT(&self->run);
+ EXPECT_EQ(self->run.exception_vector, 0);
+ EXPECT_EQ(self->run.exception_error_code, 0);
+ EXPECT_EQ(self->run.exception_addr, 0);
+ EXPECT_EQ(eaccept_op.ret, 0);
+
+ eaccept_op.epc_addr = (unsigned long)tcs;
+
+ EXPECT_EQ(ENCL_CALL(&eaccept_op, &self->run, true), 0);
+
+ EXPECT_EEXIT(&self->run);
+ EXPECT_EQ(self->run.exception_vector, 0);
+ EXPECT_EQ(self->run.exception_error_code, 0);
+ EXPECT_EQ(self->run.exception_addr, 0);
+ EXPECT_EQ(eaccept_op.ret, 0);
+
+ /*
+ * Three new pages added to enclave. Now populate the TCS page with
+ * needed data. This should be done from within enclave. Provide
+ * the function that will do the actual data population with needed
+ * data.
+ */
+
+ /*
+ * New TCS will use the "encl_dyn_entry" entrypoint that expects
+ * stack to begin in page before TCS page.
+ */
+ val_64 = encl_get_entry(&self->encl, "encl_dyn_entry");
+ EXPECT_NE(val_64, 0);
+
+ init_tcs_page_op.tcs_page = (unsigned long)tcs;
+ init_tcs_page_op.ssa = (unsigned long)total_size + 2 * PAGE_SIZE;
+ init_tcs_page_op.entry = val_64;
+ init_tcs_page_op.header.type = ENCL_OP_INIT_TCS_PAGE;
+
+ EXPECT_EQ(ENCL_CALL(&init_tcs_page_op, &self->run, true), 0);
+
+ EXPECT_EEXIT(&self->run);
+ EXPECT_EQ(self->run.exception_vector, 0);
+ EXPECT_EQ(self->run.exception_error_code, 0);
+ EXPECT_EQ(self->run.exception_addr, 0);
+
+ /* Change TCS page type to TCS. */
+ memset(&modt_ioc, 0, sizeof(modt_ioc));
+
+ modt_ioc.offset = total_size + PAGE_SIZE;
+ modt_ioc.length = PAGE_SIZE;
+ modt_ioc.page_type = SGX_PAGE_TYPE_TCS;
+
+ ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_MODIFY_TYPES, &modt_ioc);
+ errno_save = ret == -1 ? errno : 0;
+
+ EXPECT_EQ(ret, 0);
+ EXPECT_EQ(errno_save, 0);
+ EXPECT_EQ(modt_ioc.result, 0);
+ EXPECT_EQ(modt_ioc.count, 4096);
+
+ /* EACCEPT new TCS page from enclave. */
+ eaccept_op.epc_addr = (unsigned long)tcs;
+ eaccept_op.flags = SGX_SECINFO_TCS | SGX_SECINFO_MODIFIED;
+ eaccept_op.ret = 0;
+ eaccept_op.header.type = ENCL_OP_EACCEPT;
+
+ EXPECT_EQ(ENCL_CALL(&eaccept_op, &self->run, true), 0);
+
+ EXPECT_EEXIT(&self->run);
+ EXPECT_EQ(self->run.exception_vector, 0);
+ EXPECT_EQ(self->run.exception_error_code, 0);
+ EXPECT_EQ(self->run.exception_addr, 0);
+ EXPECT_EQ(eaccept_op.ret, 0);
+
+ /* Run workload from new TCS. */
+ self->run.tcs = (unsigned long)tcs;
+
+ /*
+ * Simple workload to write to data buffer and read value back.
+ */
+ put_buf_op.header.type = ENCL_OP_PUT_TO_BUFFER;
+ put_buf_op.value = MAGIC;
+
+ EXPECT_EQ(ENCL_CALL(&put_buf_op, &self->run, true), 0);
+
+ EXPECT_EEXIT(&self->run);
+ EXPECT_EQ(self->run.exception_vector, 0);
+ EXPECT_EQ(self->run.exception_error_code, 0);
+ EXPECT_EQ(self->run.exception_addr, 0);
+
+ get_buf_op.header.type = ENCL_OP_GET_FROM_BUFFER;
+ get_buf_op.value = 0;
+
+ EXPECT_EQ(ENCL_CALL(&get_buf_op, &self->run, true), 0);
+
+ EXPECT_EQ(get_buf_op.value, MAGIC);
+ EXPECT_EEXIT(&self->run);
+ EXPECT_EQ(self->run.exception_vector, 0);
+ EXPECT_EQ(self->run.exception_error_code, 0);
+ EXPECT_EQ(self->run.exception_addr, 0);
+
+ /*
+ * Phase 2 of test:
+ * Remove pages associated with new TCS, create a regular page
+ * where TCS page used to be and verify it can be used as a regular
+ * page.
+ */
+
+ /* Start page removal by requesting change of page type to PT_TRIM. */
+ memset(&modt_ioc, 0, sizeof(modt_ioc));
+
+ modt_ioc.offset = total_size;
+ modt_ioc.length = 3 * PAGE_SIZE;
+ modt_ioc.page_type = SGX_PAGE_TYPE_TRIM;
+
+ ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_MODIFY_TYPES, &modt_ioc);
+ errno_save = ret == -1 ? errno : 0;
+
+ EXPECT_EQ(ret, 0);
+ EXPECT_EQ(errno_save, 0);
+ EXPECT_EQ(modt_ioc.result, 0);
+ EXPECT_EQ(modt_ioc.count, 3 * PAGE_SIZE);
+
+ /*
+ * Enter enclave via TCS #1 and approve page removal by sending
+ * EACCEPT for each of three removed pages.
+ */
+ self->run.tcs = self->encl.encl_base;
+
+ eaccept_op.epc_addr = (unsigned long)stack_end;
+ eaccept_op.flags = SGX_SECINFO_TRIM | SGX_SECINFO_MODIFIED;
+ eaccept_op.ret = 0;
+ eaccept_op.header.type = ENCL_OP_EACCEPT;
+
+ EXPECT_EQ(ENCL_CALL(&eaccept_op, &self->run, true), 0);
+
+ EXPECT_EEXIT(&self->run);
+ EXPECT_EQ(self->run.exception_vector, 0);
+ EXPECT_EQ(self->run.exception_error_code, 0);
+ EXPECT_EQ(self->run.exception_addr, 0);
+ EXPECT_EQ(eaccept_op.ret, 0);
+
+ eaccept_op.epc_addr = (unsigned long)tcs;
+ eaccept_op.ret = 0;
+
+ EXPECT_EQ(ENCL_CALL(&eaccept_op, &self->run, true), 0);
+
+ EXPECT_EEXIT(&self->run);
+ EXPECT_EQ(self->run.exception_vector, 0);
+ EXPECT_EQ(self->run.exception_error_code, 0);
+ EXPECT_EQ(self->run.exception_addr, 0);
+ EXPECT_EQ(eaccept_op.ret, 0);
+
+ eaccept_op.epc_addr = (unsigned long)ssa;
+ eaccept_op.ret = 0;
+
+ EXPECT_EQ(ENCL_CALL(&eaccept_op, &self->run, true), 0);
+
+ EXPECT_EEXIT(&self->run);
+ EXPECT_EQ(self->run.exception_vector, 0);
+ EXPECT_EQ(self->run.exception_error_code, 0);
+ EXPECT_EQ(self->run.exception_addr, 0);
+ EXPECT_EQ(eaccept_op.ret, 0);
+
+ /* Send final ioctl() to complete page removal. */
+ memset(&remove_ioc, 0, sizeof(remove_ioc));
+
+ remove_ioc.offset = total_size;
+ remove_ioc.length = 3 * PAGE_SIZE;
+
+ ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_REMOVE_PAGES, &remove_ioc);
+ errno_save = ret == -1 ? errno : 0;
+
+ EXPECT_EQ(ret, 0);
+ EXPECT_EQ(errno_save, 0);
+ EXPECT_EQ(remove_ioc.count, 3 * PAGE_SIZE);
+
+ /*
+ * Enter enclave via TCS #1 and access location where TCS #3 was to
+ * trigger dynamic add of regular page at that location.
+ */
+ eaccept_op.epc_addr = (unsigned long)tcs;
+ eaccept_op.flags = SGX_SECINFO_R | SGX_SECINFO_W | SGX_SECINFO_REG | SGX_SECINFO_PENDING;
+ eaccept_op.ret = 0;
+ eaccept_op.header.type = ENCL_OP_EACCEPT;
+
+ EXPECT_EQ(ENCL_CALL(&eaccept_op, &self->run, true), 0);
+
+ EXPECT_EEXIT(&self->run);
+ EXPECT_EQ(self->run.exception_vector, 0);
+ EXPECT_EQ(self->run.exception_error_code, 0);
+ EXPECT_EQ(self->run.exception_addr, 0);
+ EXPECT_EQ(eaccept_op.ret, 0);
+
+ /*
+ * New page should be accessible from within enclave - write to it.
+ */
+ put_addr_op.value = MAGIC;
+ put_addr_op.addr = (unsigned long)tcs;
+ put_addr_op.header.type = ENCL_OP_PUT_TO_ADDRESS;
+
+ EXPECT_EQ(ENCL_CALL(&put_addr_op, &self->run, true), 0);
+
+ EXPECT_EEXIT(&self->run);
+ EXPECT_EQ(self->run.exception_vector, 0);
+ EXPECT_EQ(self->run.exception_error_code, 0);
+ EXPECT_EQ(self->run.exception_addr, 0);
+
+ /*
+ * Read memory from newly added page that was just written to,
+ * confirming that data previously written (MAGIC) is present.
+ */
+ get_addr_op.value = 0;
+ get_addr_op.addr = (unsigned long)tcs;
+ get_addr_op.header.type = ENCL_OP_GET_FROM_ADDRESS;
+
+ EXPECT_EQ(ENCL_CALL(&get_addr_op, &self->run, true), 0);
+
+ EXPECT_EQ(get_addr_op.value, MAGIC);
+ EXPECT_EEXIT(&self->run);
+ EXPECT_EQ(self->run.exception_vector, 0);
+ EXPECT_EQ(self->run.exception_error_code, 0);
+ EXPECT_EQ(self->run.exception_addr, 0);
+
+ munmap(addr, 3 * PAGE_SIZE);
+}
+
TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/sgx/main.h b/tools/testing/selftests/sgx/main.h
index b45c52ec7ab3..fc585be97e2f 100644
--- a/tools/testing/selftests/sgx/main.h
+++ b/tools/testing/selftests/sgx/main.h
@@ -38,6 +38,7 @@ void encl_delete(struct encl *ctx);
bool encl_load(const char *path, struct encl *encl, unsigned long heap_size);
bool encl_measure(struct encl *encl);
bool encl_build(struct encl *encl);
+uint64_t encl_get_entry(struct encl *encl, const char *symbol);

int sgx_enter_enclave(void *rdi, void *rsi, long rdx, u32 function, void *r8, void *r9,
struct sgx_enclave_run *run);
--
2.25.1


2022-05-10 21:10:13

by Reinette Chatre

[permalink] [raw]
Subject: [PATCH V5 07/31] x86/sgx: Rename sgx_encl_ewb_cpumask() as sgx_encl_cpumask()

sgx_encl_ewb_cpumask() is no longer unique to the reclaimer where it
is used during the EWB ENCLS leaf function when EPC pages are written
out to main memory and sgx_encl_ewb_cpumask() is used to learn which
CPUs might have executed the enclave to ensure that TLBs are cleared.

Upcoming SGX2 enabling will use sgx_encl_ewb_cpumask() during the
EMODPR and EMODT ENCLS leaf functions that make changes to enclave
pages. The function is needed for the same reason it is used now: to
learn which CPUs might have executed the enclave to ensure that TLBs
no longer point to the changed pages.

Rename sgx_encl_ewb_cpumask() to sgx_encl_cpumask() to reflect the
broader usage.

Reviewed-by: Jarkko Sakkinen <[email protected]>
Signed-off-by: Reinette Chatre <[email protected]>
---
No changes since V4.

Changes since V3:
- Add Jarkko's Reviewed-by tag.

Changes since V1:
- New patch split from original "x86/sgx: Use more generic name for
enclave cpumask function" (Jarkko).

arch/x86/kernel/cpu/sgx/encl.c | 6 +++---
arch/x86/kernel/cpu/sgx/encl.h | 2 +-
arch/x86/kernel/cpu/sgx/main.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c
index 04d8212ce400..1bbd76d91554 100644
--- a/arch/x86/kernel/cpu/sgx/encl.c
+++ b/arch/x86/kernel/cpu/sgx/encl.c
@@ -710,7 +710,7 @@ int sgx_encl_mm_add(struct sgx_encl *encl, struct mm_struct *mm)
}

/**
- * sgx_encl_ewb_cpumask() - Query which CPUs might be accessing the enclave
+ * sgx_encl_cpumask() - Query which CPUs might be accessing the enclave
* @encl: the enclave
*
* Some SGX functions require that no cached linear-to-physical address
@@ -735,7 +735,7 @@ int sgx_encl_mm_add(struct sgx_encl *encl, struct mm_struct *mm)
* The following flow is used to support SGX functions that require that
* no cached linear-to-physical address mappings are present:
* 1) Execute ENCLS[ETRACK] to initiate hardware tracking.
- * 2) Use this function (sgx_encl_ewb_cpumask()) to query which CPUs might be
+ * 2) Use this function (sgx_encl_cpumask()) to query which CPUs might be
* accessing the enclave.
* 3) Send IPI to identified CPUs, kicking them out of the enclave and
* thus flushing all locally cached linear-to-physical address mappings.
@@ -752,7 +752,7 @@ int sgx_encl_mm_add(struct sgx_encl *encl, struct mm_struct *mm)
*
* Return: cpumask of CPUs that might be accessing @encl
*/
-const cpumask_t *sgx_encl_ewb_cpumask(struct sgx_encl *encl)
+const cpumask_t *sgx_encl_cpumask(struct sgx_encl *encl)
{
cpumask_t *cpumask = &encl->cpumask;
struct sgx_encl_mm *encl_mm;
diff --git a/arch/x86/kernel/cpu/sgx/encl.h b/arch/x86/kernel/cpu/sgx/encl.h
index c6afa58ea3e6..ef8cf106904b 100644
--- a/arch/x86/kernel/cpu/sgx/encl.h
+++ b/arch/x86/kernel/cpu/sgx/encl.h
@@ -105,7 +105,7 @@ int sgx_encl_may_map(struct sgx_encl *encl, unsigned long start,

void sgx_encl_release(struct kref *ref);
int sgx_encl_mm_add(struct sgx_encl *encl, struct mm_struct *mm);
-const cpumask_t *sgx_encl_ewb_cpumask(struct sgx_encl *encl);
+const cpumask_t *sgx_encl_cpumask(struct sgx_encl *encl);
int sgx_encl_get_backing(struct sgx_encl *encl, unsigned long page_index,
struct sgx_backing *backing);
void sgx_encl_put_backing(struct sgx_backing *backing);
diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
index d1af5828635d..736ca652caab 100644
--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -251,7 +251,7 @@ static void sgx_encl_ewb(struct sgx_epc_page *epc_page,
* miss cpus that entered the enclave between
* generating the mask and incrementing epoch.
*/
- on_each_cpu_mask(sgx_encl_ewb_cpumask(encl),
+ on_each_cpu_mask(sgx_encl_cpumask(encl),
sgx_ipi_cb, NULL, 1);
ret = __sgx_encl_ewb(epc_page, va_slot, backing);
}
--
2.25.1


2022-05-10 21:13:34

by Reinette Chatre

[permalink] [raw]
Subject: [PATCH V5 10/31] x86/sgx: Create utility to validate user provided offset and length

User provided offset and length is validated when parsing the parameters
of the SGX_IOC_ENCLAVE_ADD_PAGES ioctl(). Extract this validation
(with consistent use of IS_ALIGNED) into a utility that can be used
by the SGX2 ioctl()s that will also provide these values.

Reviewed-by: Jarkko Sakkinen <[email protected]>
Signed-off-by: Reinette Chatre <[email protected]>
---
No changes since V4.

Changes since V3:
- Add Jarkko's Reviewed-by tag.
- Consistently use IS_ALIGNED(). (Jarkko)

Changes since V1:
- New patch

arch/x86/kernel/cpu/sgx/ioctl.c | 28 ++++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
index 83df20e3e633..a66795e0b685 100644
--- a/arch/x86/kernel/cpu/sgx/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -372,6 +372,26 @@ static int sgx_encl_add_page(struct sgx_encl *encl, unsigned long src,
return ret;
}

+/*
+ * Ensure user provided offset and length values are valid for
+ * an enclave.
+ */
+static int sgx_validate_offset_length(struct sgx_encl *encl,
+ unsigned long offset,
+ unsigned long length)
+{
+ if (!IS_ALIGNED(offset, PAGE_SIZE))
+ return -EINVAL;
+
+ if (!length || !IS_ALIGNED(length, PAGE_SIZE))
+ return -EINVAL;
+
+ if (offset + length - PAGE_SIZE >= encl->size)
+ return -EINVAL;
+
+ return 0;
+}
+
/**
* sgx_ioc_enclave_add_pages() - The handler for %SGX_IOC_ENCLAVE_ADD_PAGES
* @encl: an enclave pointer
@@ -425,14 +445,10 @@ static long sgx_ioc_enclave_add_pages(struct sgx_encl *encl, void __user *arg)
if (copy_from_user(&add_arg, arg, sizeof(add_arg)))
return -EFAULT;

- if (!IS_ALIGNED(add_arg.offset, PAGE_SIZE) ||
- !IS_ALIGNED(add_arg.src, PAGE_SIZE))
- return -EINVAL;
-
- if (!add_arg.length || add_arg.length & (PAGE_SIZE - 1))
+ if (!IS_ALIGNED(add_arg.src, PAGE_SIZE))
return -EINVAL;

- if (add_arg.offset + add_arg.length - PAGE_SIZE >= encl->size)
+ if (sgx_validate_offset_length(encl, add_arg.offset, add_arg.length))
return -EINVAL;

if (copy_from_user(&secinfo, (void __user *)add_arg.secinfo,
--
2.25.1


2022-05-10 21:17:36

by Reinette Chatre

[permalink] [raw]
Subject: [PATCH V5 03/31] x86/sgx: Add wrapper for SGX2 EMODT function

Add a wrapper for the EMODT ENCLS leaf function used to
change the type of an enclave page as maintained in the
SGX hardware's Enclave Page Cache Map (EPCM).

EMODT:
1) Updates the EPCM page type of the enclave page.
2) Sets the MODIFIED bit in the EPCM entry of the enclave page.
This bit is reset by the enclave by invoking ENCLU leaf
function EACCEPT or EACCEPTCOPY.

Access from within the enclave to the enclave page is not possible
while the MODIFIED bit is set.

After changing the enclave page type by issuing EMODT the kernel
needs to collaborate with the hardware to ensure that no logical
processor continues to hold a reference to the changed page. This
is required to ensure no required security checks are circumvented
and is required for the enclave's EACCEPT/EACCEPTCOPY to succeed.
Ensuring that no references to the changed page remain is
accomplished with the ETRACK flow.

Reviewed-by: Jarkko Sakkinen <[email protected]>
Signed-off-by: Reinette Chatre <[email protected]>
---
No changes since V4.

Changes since V3:
- Add Jarkko's Reviewed-by tag.

Changes since V1:
- Split original patch ("x86/sgx: Add wrappers for SGX2 functions")
in three to introduce the SGX2 functions separately (Jarkko).
- Rewrite commit message to include how the EPCM within the hardware
is changed by the SGX2 function as well as the calling
conditions (Jarkko).

arch/x86/kernel/cpu/sgx/encls.h | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/arch/x86/kernel/cpu/sgx/encls.h b/arch/x86/kernel/cpu/sgx/encls.h
index 2b091912f038..7a1ecf704ec1 100644
--- a/arch/x86/kernel/cpu/sgx/encls.h
+++ b/arch/x86/kernel/cpu/sgx/encls.h
@@ -221,4 +221,10 @@ static inline int __emodpr(struct sgx_secinfo *secinfo, void *addr)
return __encls_ret_2(EMODPR, secinfo, addr);
}

+/* Change the type of an EPC page. */
+static inline int __emodt(struct sgx_secinfo *secinfo, void *addr)
+{
+ return __encls_ret_2(EMODT, secinfo, addr);
+}
+
#endif /* _X86_ENCLS_H */
--
2.25.1


2022-05-10 21:40:47

by Reinette Chatre

[permalink] [raw]
Subject: [PATCH V5 14/31] x86/sgx: Support VA page allocation without reclaiming

struct sgx_encl should be protected with the mutex
sgx_encl->lock. One exception is sgx_encl->page_cnt that
is incremented (in sgx_encl_grow()) when an enclave page
is added to the enclave. The reason the mutex is not held
is to allow the reclaimer to be called directly if there are
no EPC pages (in support of a new VA page) available at the time.

Incrementing sgx_encl->page_cnt without sgc_encl->lock held
is currently (before SGX2) safe from concurrent updates because
all paths in which sgx_encl_grow() is called occur before
enclave initialization and are protected with an atomic
operation on SGX_ENCL_IOCTL.

SGX2 includes support for dynamically adding pages after
enclave initialization where the protection of SGX_ENCL_IOCTL
is not available.

Make direct reclaim of EPC pages optional when new VA pages
are added to the enclave. Essentially the existing "reclaim"
flag used when regular EPC pages are added to an enclave
becomes available to the caller when used to allocate VA pages
instead of always being "true".

When adding pages without invoking the reclaimer it is possible
to do so with sgx_encl->lock held, gaining its protection against
concurrent updates to sgx_encl->page_cnt after enclave
initialization.

No functional change.

Reported-by: Haitao Huang <[email protected]>
Reviewed-by: Jarkko Sakkinen <[email protected]>
Signed-off-by: Reinette Chatre <[email protected]>
---
Changes since V4:
- Move Haitao's "Tested-by" tag to "x86/sgx: Support adding of pages
to an initialized enclave". (Jarkko)
- Add Jarkko's Reviewed-by tag.

Changes since V3:
- New patch prompted by Haitao encountering the
WARN_ON_ONCE(encl->page_cnt % SGX_VA_SLOT_COUNT)
within sgx_encl_grow() during his SGX2 multi-threaded
unit tests.

arch/x86/kernel/cpu/sgx/encl.c | 6 ++++--
arch/x86/kernel/cpu/sgx/encl.h | 4 ++--
arch/x86/kernel/cpu/sgx/ioctl.c | 8 ++++----
3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c
index 2f053af9ce3e..5b5affe1a538 100644
--- a/arch/x86/kernel/cpu/sgx/encl.c
+++ b/arch/x86/kernel/cpu/sgx/encl.c
@@ -959,6 +959,8 @@ void sgx_zap_enclave_ptes(struct sgx_encl *encl, unsigned long addr)

/**
* sgx_alloc_va_page() - Allocate a Version Array (VA) page
+ * @reclaim: Reclaim EPC pages directly if none available. Enclave
+ * mutex should not be held if this is set.
*
* Allocate a free EPC page and convert it to a Version Array (VA) page.
*
@@ -966,12 +968,12 @@ void sgx_zap_enclave_ptes(struct sgx_encl *encl, unsigned long addr)
* a VA page,
* -errno otherwise
*/
-struct sgx_epc_page *sgx_alloc_va_page(void)
+struct sgx_epc_page *sgx_alloc_va_page(bool reclaim)
{
struct sgx_epc_page *epc_page;
int ret;

- epc_page = sgx_alloc_epc_page(NULL, true);
+ epc_page = sgx_alloc_epc_page(NULL, reclaim);
if (IS_ERR(epc_page))
return ERR_CAST(epc_page);

diff --git a/arch/x86/kernel/cpu/sgx/encl.h b/arch/x86/kernel/cpu/sgx/encl.h
index 2cb58ab868e5..3d0e0ba3edf5 100644
--- a/arch/x86/kernel/cpu/sgx/encl.h
+++ b/arch/x86/kernel/cpu/sgx/encl.h
@@ -116,14 +116,14 @@ struct sgx_encl_page *sgx_encl_page_alloc(struct sgx_encl *encl,
unsigned long offset,
u64 secinfo_flags);
void sgx_zap_enclave_ptes(struct sgx_encl *encl, unsigned long addr);
-struct sgx_epc_page *sgx_alloc_va_page(void);
+struct sgx_epc_page *sgx_alloc_va_page(bool reclaim);
unsigned int sgx_alloc_va_slot(struct sgx_va_page *va_page);
void sgx_free_va_slot(struct sgx_va_page *va_page, unsigned int offset);
bool sgx_va_page_full(struct sgx_va_page *va_page);
void sgx_encl_free_epc_page(struct sgx_epc_page *page);
struct sgx_encl_page *sgx_encl_load_page(struct sgx_encl *encl,
unsigned long addr);
-struct sgx_va_page *sgx_encl_grow(struct sgx_encl *encl);
+struct sgx_va_page *sgx_encl_grow(struct sgx_encl *encl, bool reclaim);
void sgx_encl_shrink(struct sgx_encl *encl, struct sgx_va_page *va_page);

#endif /* _X86_ENCL_H */
diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
index bb8cdb2ad0d1..5d41aa204761 100644
--- a/arch/x86/kernel/cpu/sgx/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -17,7 +17,7 @@
#include "encl.h"
#include "encls.h"

-struct sgx_va_page *sgx_encl_grow(struct sgx_encl *encl)
+struct sgx_va_page *sgx_encl_grow(struct sgx_encl *encl, bool reclaim)
{
struct sgx_va_page *va_page = NULL;
void *err;
@@ -30,7 +30,7 @@ struct sgx_va_page *sgx_encl_grow(struct sgx_encl *encl)
if (!va_page)
return ERR_PTR(-ENOMEM);

- va_page->epc_page = sgx_alloc_va_page();
+ va_page->epc_page = sgx_alloc_va_page(reclaim);
if (IS_ERR(va_page->epc_page)) {
err = ERR_CAST(va_page->epc_page);
kfree(va_page);
@@ -64,7 +64,7 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs)
struct file *backing;
long ret;

- va_page = sgx_encl_grow(encl);
+ va_page = sgx_encl_grow(encl, true);
if (IS_ERR(va_page))
return PTR_ERR(va_page);
else if (va_page)
@@ -275,7 +275,7 @@ static int sgx_encl_add_page(struct sgx_encl *encl, unsigned long src,
return PTR_ERR(epc_page);
}

- va_page = sgx_encl_grow(encl);
+ va_page = sgx_encl_grow(encl, true);
if (IS_ERR(va_page)) {
ret = PTR_ERR(va_page);
goto err_out_free;
--
2.25.1


2022-05-10 21:46:48

by Reinette Chatre

[permalink] [raw]
Subject: [PATCH V5 31/31] selftests/sgx: Page removal stress test

Create enclave with additional heap that consumes all physical SGX
memory and then remove it.

Depending on the available SGX memory this test could take a
significant time to run (several minutes) as it (1) creates the
enclave, (2) changes the type of every page to be trimmed,
(3) enters the enclave once per page to run EACCEPT, before
(4) the pages are finally removed.

Acked-by: Jarkko Sakkinen <[email protected]>
Signed-off-by: Reinette Chatre <[email protected]>
---
Changes since V4:
- Rename struct sgx_enclave_modify_type ->
struct sgx_enclave_modify_types. (Jarkko)

Changes since V3:
- Add Jarkko's Acked-by tag.
- Rename SGX_IOC_ENCLAVE_MODIFY_TYPE to
SGX_IOC_ENCLAVE_MODIFY_TYPES. (Jarkko)
- User provides just page type to SGX_IOC_ENCLAVE_MODIFY_TYPES ioctl(),
replacing secinfo. (Jarkko)
- Let the SKIP() call involving SGX_IOC_ENCLAVE_MODIFY_TYPES span
two lines to address checkpatch.pl warning triggered by new longer
name.

Changes since V2:
- Rename struct sgx_enclave_modt -> struct sgx_enclave_modify_type

Changes since V1:
- Exit test completely on first failure of EACCEPT of a removed page. Since
this is an oversubscribed test the number of pages on which this is
attempted can be significant and in case of failure the per-page
error logging would overwhelm the system.
- Update test to call renamed ioctl() (SGX_IOC_PAGE_MODT ->
SGX_IOC_ENCLAVE_MODIFY_TYPE) and provide secinfo as parameter (Jarkko).
- Fixup definitions to be reverse xmas tree.
- Update test to reflect page removal ioctl() and struct name change:
SGX_IOC_PAGE_REMOVE->SGX_IOC_ENCLAVE_REMOVE_PAGES,
struct sgx_page_remove -> struct sgx_enclave_remove_pages (Jarkko).
- Ensure test is skipped when SGX2 not supported by kernel.
- Cleanup comments.

tools/testing/selftests/sgx/main.c | 120 +++++++++++++++++++++++++++++
1 file changed, 120 insertions(+)

diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c
index ba16671aef79..9820b3809c69 100644
--- a/tools/testing/selftests/sgx/main.c
+++ b/tools/testing/selftests/sgx/main.c
@@ -378,7 +378,127 @@ TEST_F(enclave, unclobbered_vdso_oversubscribed)
EXPECT_EQ(get_op.value, MAGIC);
EXPECT_EEXIT(&self->run);
EXPECT_EQ(self->run.user_data, 0);
+}
+
+TEST_F_TIMEOUT(enclave, unclobbered_vdso_oversubscribed_remove, 900)
+{
+ struct sgx_enclave_remove_pages remove_ioc;
+ struct sgx_enclave_modify_types modt_ioc;
+ struct encl_op_get_from_buf get_op;
+ struct encl_op_eaccept eaccept_op;
+ struct encl_op_put_to_buf put_op;
+ struct encl_segment *heap;
+ unsigned long total_mem;
+ int ret, errno_save;
+ unsigned long addr;
+ unsigned long i;
+
+ /*
+ * Create enclave with additional heap that is as big as all
+ * available physical SGX memory.
+ */
+ total_mem = get_total_epc_mem();
+ ASSERT_NE(total_mem, 0);
+ TH_LOG("Creating an enclave with %lu bytes heap may take a while ...",
+ total_mem);
+ ASSERT_TRUE(setup_test_encl(total_mem, &self->encl, _metadata));
+
+ /*
+ * Hardware (SGX2) and kernel support is needed for this test. Start
+ * with check that test has a chance of succeeding.
+ */
+ memset(&modt_ioc, 0, sizeof(modt_ioc));
+ ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_MODIFY_TYPES, &modt_ioc);
+
+ if (ret == -1) {
+ if (errno == ENOTTY)
+ SKIP(return,
+ "Kernel does not support SGX_IOC_ENCLAVE_MODIFY_TYPES ioctl()");
+ else if (errno == ENODEV)
+ SKIP(return, "System does not support SGX2");
+ }
+
+ /*
+ * Invalid parameters were provided during sanity check,
+ * expect command to fail.
+ */
+ EXPECT_EQ(ret, -1);
+
+ /* SGX2 is supported by kernel and hardware, test can proceed. */
+ memset(&self->run, 0, sizeof(self->run));
+ self->run.tcs = self->encl.encl_base;
+
+ heap = &self->encl.segment_tbl[self->encl.nr_segments - 1];
+
+ put_op.header.type = ENCL_OP_PUT_TO_BUFFER;
+ put_op.value = MAGIC;
+
+ EXPECT_EQ(ENCL_CALL(&put_op, &self->run, false), 0);
+
+ EXPECT_EEXIT(&self->run);
+ EXPECT_EQ(self->run.user_data, 0);
+
+ get_op.header.type = ENCL_OP_GET_FROM_BUFFER;
+ get_op.value = 0;
+
+ EXPECT_EQ(ENCL_CALL(&get_op, &self->run, false), 0);
+
+ EXPECT_EQ(get_op.value, MAGIC);
+ EXPECT_EEXIT(&self->run);
+ EXPECT_EQ(self->run.user_data, 0);

+ /* Trim entire heap. */
+ memset(&modt_ioc, 0, sizeof(modt_ioc));
+
+ modt_ioc.offset = heap->offset;
+ modt_ioc.length = heap->size;
+ modt_ioc.page_type = SGX_PAGE_TYPE_TRIM;
+
+ TH_LOG("Changing type of %zd bytes to trimmed may take a while ...",
+ heap->size);
+ ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_MODIFY_TYPES, &modt_ioc);
+ errno_save = ret == -1 ? errno : 0;
+
+ EXPECT_EQ(ret, 0);
+ EXPECT_EQ(errno_save, 0);
+ EXPECT_EQ(modt_ioc.result, 0);
+ EXPECT_EQ(modt_ioc.count, heap->size);
+
+ /* EACCEPT all removed pages. */
+ addr = self->encl.encl_base + heap->offset;
+
+ eaccept_op.flags = SGX_SECINFO_TRIM | SGX_SECINFO_MODIFIED;
+ eaccept_op.header.type = ENCL_OP_EACCEPT;
+
+ TH_LOG("Entering enclave to run EACCEPT for each page of %zd bytes may take a while ...",
+ heap->size);
+ for (i = 0; i < heap->size; i += 4096) {
+ eaccept_op.epc_addr = addr + i;
+ eaccept_op.ret = 0;
+
+ EXPECT_EQ(ENCL_CALL(&eaccept_op, &self->run, true), 0);
+
+ EXPECT_EQ(self->run.exception_vector, 0);
+ EXPECT_EQ(self->run.exception_error_code, 0);
+ EXPECT_EQ(self->run.exception_addr, 0);
+ ASSERT_EQ(eaccept_op.ret, 0);
+ ASSERT_EQ(self->run.function, EEXIT);
+ }
+
+ /* Complete page removal. */
+ memset(&remove_ioc, 0, sizeof(remove_ioc));
+
+ remove_ioc.offset = heap->offset;
+ remove_ioc.length = heap->size;
+
+ TH_LOG("Removing %zd bytes from enclave may take a while ...",
+ heap->size);
+ ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_REMOVE_PAGES, &remove_ioc);
+ errno_save = ret == -1 ? errno : 0;
+
+ EXPECT_EQ(ret, 0);
+ EXPECT_EQ(errno_save, 0);
+ EXPECT_EQ(remove_ioc.count, heap->size);
}

TEST_F(enclave, clobbered_vdso)
--
2.25.1


2022-05-10 22:06:54

by Reinette Chatre

[permalink] [raw]
Subject: [PATCH V5 18/31] x86/sgx: Support modifying SGX page type

Every enclave contains one or more Thread Control Structures (TCS). The
TCS contains meta-data used by the hardware to save and restore thread
specific information when entering/exiting the enclave. With SGX1 an
enclave needs to be created with enough TCSs to support the largest
number of threads expecting to use the enclave and enough enclave pages
to meet all its anticipated memory demands. In SGX1 all pages remain in
the enclave until the enclave is unloaded.

SGX2 introduces a new function, ENCLS[EMODT], that is used to change
the type of an enclave page from a regular (SGX_PAGE_TYPE_REG) enclave
page to a TCS (SGX_PAGE_TYPE_TCS) page or change the type from a
regular (SGX_PAGE_TYPE_REG) or TCS (SGX_PAGE_TYPE_TCS)
page to a trimmed (SGX_PAGE_TYPE_TRIM) page (setting it up for later
removal).

With the existing support of dynamically adding regular enclave pages
to an initialized enclave and changing the page type to TCS it is
possible to dynamically increase the number of threads supported by an
enclave.

Changing the enclave page type to SGX_PAGE_TYPE_TRIM is the first step
of dynamically removing pages from an initialized enclave. The complete
page removal flow is:
1) Change the type of the pages to be removed to SGX_PAGE_TYPE_TRIM
using the SGX_IOC_ENCLAVE_MODIFY_TYPES ioctl() introduced here.
2) Approve the page removal by running ENCLU[EACCEPT] from within
the enclave.
3) Initiate actual page removal using the ioctl() introduced in the
following patch.

Add ioctl() SGX_IOC_ENCLAVE_MODIFY_TYPES to support changing SGX
enclave page types within an initialized enclave. With
SGX_IOC_ENCLAVE_MODIFY_TYPES the user specifies a page range and the
enclave page type to be applied to all pages in the provided range.
The ioctl() itself can return an error code based on failures
encountered by the kernel. It is also possible for SGX specific
failures to be encountered. Add a result output parameter to
communicate the SGX return code. It is possible for the enclave page
type change request to fail on any page within the provided range.
Support partial success by returning the number of pages that were
successfully changed.

After the page type is changed the page continues to be accessible
from the kernel perspective with page table entries and internal
state. The page may be moved to swap. Any access until ENCLU[EACCEPT]
will encounter a page fault with SGX flag set in error code.

Reviewed-by: Jarkko Sakkinen <[email protected]>
Tested-by: Jarkko Sakkinen <[email protected]>
Tested-by: Haitao Huang <[email protected]>
Tested-by: Vijay Dhanraj <[email protected]>
Signed-off-by: Reinette Chatre <[email protected]>
---
Changes since V4:
- Renames: (Jarkko)
struct sgx_enclave_modify_type -> struct sgx_enclave_modify_types
sgx_enclave_modify_type() -> sgx_enclave_modify_types()
sgx_ioc_enclave_modify_type() -> sgx_ioc_enclave_modify_types()
- Add Jarkko's Reviewed-by and Tested-by tags.
- Add Haitao's Tested-by tag.
- Add Vijay's Tested-by tag.

Changes since V3:
- Rename ioctl() SGX_IOC_ENCLAVE_MODIFY_TYPE to
SGX_IOC_ENCLAVE_MODIFY_TYPES. (Jarkko)
- User provides just page type, replacing secinfo. (Jarkko)

Changes since V2:
- Adjust ioctl number after removal of SGX_IOC_ENCLAVE_RELAX_PERMISSIONS.
- Remove attempt at runtime tracking of EPCM permissions
(sgx_encl_page->vm_run_prot_bits). (Jarkko)
- Change names to follow guidance of using detailed names (Jarkko):
struct sgx_enclave_modt -> struct sgx_enclave_modify_type
sgx_enclave_modt() -> sgx_enclave_modify_type()
sgx_ioc_enclave_modt() -> sgx_ioc_enclave_modify_type()

Changes since V1:
- Remove the "Earlier changes ..." paragraph (Jarkko).
- Change "new ioctl" text to "Add SGX_IOC_ENCLAVE_MOD_TYPE" (Jarkko).
- Discussion about EPCM interaction and the EPCM MODIFIED bit is moved
to new patch that introduces the ENCLS[EMODT] wrapper while keeping
the higher level discussion on page accessibility in
this commit log (Jarkko).
- Rename SGX_IOC_PAGE_MODT ioctl() to SGX_IOC_ENCLAVE_MODIFY_TYPE
(Jarkko).
- Rename struct sgx_page_modt to struct sgx_enclave_modt in support
of ioctl() rename.
- Rename sgx_page_modt() to sgx_enclave_modt() and sgx_ioc_page_modt()
to sgx_ioc_enclave_modt() in support of ioctl() rename.
- Provide secinfo as parameter to ioctl() instead of just
page type (Jarkko).
- Update comments to refer to new ioctl() names.
- Use new SGX2 checking helper().
- Use ETRACK flow utility.
- Move kernel-doc to function that provides documentation for
Documentation/x86/sgx.rst.
- Remove redundant comment.
- Use offset/length validation utility.
- Make explicit which members of struct sgx_enclave_modt are for
output (Dave).

arch/x86/include/uapi/asm/sgx.h | 20 ++++
arch/x86/kernel/cpu/sgx/ioctl.c | 202 ++++++++++++++++++++++++++++++++
2 files changed, 222 insertions(+)

diff --git a/arch/x86/include/uapi/asm/sgx.h b/arch/x86/include/uapi/asm/sgx.h
index 82648c006470..567f6166c24a 100644
--- a/arch/x86/include/uapi/asm/sgx.h
+++ b/arch/x86/include/uapi/asm/sgx.h
@@ -31,6 +31,8 @@ enum sgx_page_flags {
_IO(SGX_MAGIC, 0x04)
#define SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS \
_IOWR(SGX_MAGIC, 0x05, struct sgx_enclave_restrict_permissions)
+#define SGX_IOC_ENCLAVE_MODIFY_TYPES \
+ _IOWR(SGX_MAGIC, 0x06, struct sgx_enclave_modify_types)

/**
* struct sgx_enclave_create - parameter structure for the
@@ -97,6 +99,24 @@ struct sgx_enclave_restrict_permissions {
__u64 count;
};

+/**
+ * struct sgx_enclave_modify_types - parameters for ioctl
+ * %SGX_IOC_ENCLAVE_MODIFY_TYPES
+ * @offset: starting page offset (page aligned relative to enclave base
+ * address defined in SECS)
+ * @length: length of memory (multiple of the page size)
+ * @page_type: new type for pages in range described by @offset and @length
+ * @result: (output) SGX result code of ENCLS[EMODT] function
+ * @count: (output) bytes successfully changed (multiple of page size)
+ */
+struct sgx_enclave_modify_types {
+ __u64 offset;
+ __u64 length;
+ __u64 page_type;
+ __u64 result;
+ __u64 count;
+};
+
struct sgx_enclave_run;

/**
diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
index 720188d86ed4..9ccafbfc4811 100644
--- a/arch/x86/kernel/cpu/sgx/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -872,6 +872,205 @@ static long sgx_ioc_enclave_restrict_permissions(struct sgx_encl *encl,
return ret;
}

+/**
+ * sgx_enclave_modify_types() - Modify type of SGX enclave pages
+ * @encl: Enclave to which the pages belong.
+ * @modt: Checked parameters from user about which pages need modifying
+ * and their new page type.
+ *
+ * Return:
+ * - 0: Success
+ * - -errno: Otherwise
+ */
+static long sgx_enclave_modify_types(struct sgx_encl *encl,
+ struct sgx_enclave_modify_types *modt)
+{
+ unsigned long max_prot_restore;
+ enum sgx_page_type page_type;
+ struct sgx_encl_page *entry;
+ struct sgx_secinfo secinfo;
+ unsigned long prot;
+ unsigned long addr;
+ unsigned long c;
+ void *epc_virt;
+ int ret;
+
+ page_type = modt->page_type & SGX_PAGE_TYPE_MASK;
+
+ /*
+ * The only new page types allowed by hardware are PT_TCS and PT_TRIM.
+ */
+ if (page_type != SGX_PAGE_TYPE_TCS && page_type != SGX_PAGE_TYPE_TRIM)
+ return -EINVAL;
+
+ memset(&secinfo, 0, sizeof(secinfo));
+
+ secinfo.flags = page_type << 8;
+
+ for (c = 0 ; c < modt->length; c += PAGE_SIZE) {
+ addr = encl->base + modt->offset + c;
+
+ mutex_lock(&encl->lock);
+
+ entry = sgx_encl_load_page(encl, addr);
+ if (IS_ERR(entry)) {
+ ret = PTR_ERR(entry) == -EBUSY ? -EAGAIN : -EFAULT;
+ goto out_unlock;
+ }
+
+ /*
+ * Borrow the logic from the Intel SDM. Regular pages
+ * (SGX_PAGE_TYPE_REG) can change type to SGX_PAGE_TYPE_TCS
+ * or SGX_PAGE_TYPE_TRIM but TCS pages can only be trimmed.
+ * CET pages not supported yet.
+ */
+ if (!(entry->type == SGX_PAGE_TYPE_REG ||
+ (entry->type == SGX_PAGE_TYPE_TCS &&
+ page_type == SGX_PAGE_TYPE_TRIM))) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
+ max_prot_restore = entry->vm_max_prot_bits;
+
+ /*
+ * Once a regular page becomes a TCS page it cannot be
+ * changed back. So the maximum allowed protection reflects
+ * the TCS page that is always RW from kernel perspective but
+ * will be inaccessible from within enclave. Before doing
+ * so, do make sure that the new page type continues to
+ * respect the originally vetted page permissions.
+ */
+ if (entry->type == SGX_PAGE_TYPE_REG &&
+ page_type == SGX_PAGE_TYPE_TCS) {
+ if (~entry->vm_max_prot_bits & (VM_READ | VM_WRITE)) {
+ ret = -EPERM;
+ goto out_unlock;
+ }
+ prot = PROT_READ | PROT_WRITE;
+ entry->vm_max_prot_bits = calc_vm_prot_bits(prot, 0);
+
+ /*
+ * Prevent page from being reclaimed while mutex
+ * is released.
+ */
+ if (sgx_unmark_page_reclaimable(entry->epc_page)) {
+ ret = -EAGAIN;
+ goto out_entry_changed;
+ }
+
+ /*
+ * Do not keep encl->lock because of dependency on
+ * mmap_lock acquired in sgx_zap_enclave_ptes().
+ */
+ mutex_unlock(&encl->lock);
+
+ sgx_zap_enclave_ptes(encl, addr);
+
+ mutex_lock(&encl->lock);
+
+ sgx_mark_page_reclaimable(entry->epc_page);
+ }
+
+ /* Change EPC type */
+ epc_virt = sgx_get_epc_virt_addr(entry->epc_page);
+ ret = __emodt(&secinfo, epc_virt);
+ if (encls_faulted(ret)) {
+ /*
+ * All possible faults should be avoidable:
+ * parameters have been checked, will only change
+ * valid page types, and no concurrent
+ * SGX1/SGX2 ENCLS instructions since these are
+ * protected with mutex.
+ */
+ pr_err_once("EMODT encountered exception %d\n",
+ ENCLS_TRAPNR(ret));
+ ret = -EFAULT;
+ goto out_entry_changed;
+ }
+ if (encls_failed(ret)) {
+ modt->result = ret;
+ ret = -EFAULT;
+ goto out_entry_changed;
+ }
+
+ ret = sgx_enclave_etrack(encl);
+ if (ret) {
+ ret = -EFAULT;
+ goto out_unlock;
+ }
+
+ entry->type = page_type;
+
+ mutex_unlock(&encl->lock);
+ }
+
+ ret = 0;
+ goto out;
+
+out_entry_changed:
+ entry->vm_max_prot_bits = max_prot_restore;
+out_unlock:
+ mutex_unlock(&encl->lock);
+out:
+ modt->count = c;
+
+ return ret;
+}
+
+/**
+ * sgx_ioc_enclave_modify_types() - handler for %SGX_IOC_ENCLAVE_MODIFY_TYPES
+ * @encl: an enclave pointer
+ * @arg: userspace pointer to a &struct sgx_enclave_modify_types instance
+ *
+ * Ability to change the enclave page type supports the following use cases:
+ *
+ * * It is possible to add TCS pages to an enclave by changing the type of
+ * regular pages (%SGX_PAGE_TYPE_REG) to TCS (%SGX_PAGE_TYPE_TCS) pages.
+ * With this support the number of threads supported by an initialized
+ * enclave can be increased dynamically.
+ *
+ * * Regular or TCS pages can dynamically be removed from an initialized
+ * enclave by changing the page type to %SGX_PAGE_TYPE_TRIM. Changing the
+ * page type to %SGX_PAGE_TYPE_TRIM marks the page for removal with actual
+ * removal done by handler of %SGX_IOC_ENCLAVE_REMOVE_PAGES ioctl() called
+ * after ENCLU[EACCEPT] is run on %SGX_PAGE_TYPE_TRIM page from within the
+ * enclave.
+ *
+ * Return:
+ * - 0: Success
+ * - -errno: Otherwise
+ */
+static long sgx_ioc_enclave_modify_types(struct sgx_encl *encl,
+ void __user *arg)
+{
+ struct sgx_enclave_modify_types params;
+ long ret;
+
+ ret = sgx_ioc_sgx2_ready(encl);
+ if (ret)
+ return ret;
+
+ if (copy_from_user(&params, arg, sizeof(params)))
+ return -EFAULT;
+
+ if (sgx_validate_offset_length(encl, params.offset, params.length))
+ return -EINVAL;
+
+ if (params.page_type & ~SGX_PAGE_TYPE_MASK)
+ return -EINVAL;
+
+ if (params.result || params.count)
+ return -EINVAL;
+
+ ret = sgx_enclave_modify_types(encl, &params);
+
+ if (copy_to_user(arg, &params, sizeof(params)))
+ return -EFAULT;
+
+ return ret;
+}
+
long sgx_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
{
struct sgx_encl *encl = filep->private_data;
@@ -897,6 +1096,9 @@ long sgx_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
ret = sgx_ioc_enclave_restrict_permissions(encl,
(void __user *)arg);
break;
+ case SGX_IOC_ENCLAVE_MODIFY_TYPES:
+ ret = sgx_ioc_enclave_modify_types(encl, (void __user *)arg);
+ break;
default:
ret = -ENOIOCTLCMD;
break;
--
2.25.1


2022-05-10 23:15:08

by Reinette Chatre

[permalink] [raw]
Subject: [PATCH V5 09/31] x86/sgx: Make sgx_ipi_cb() available internally

The ETRACK function followed by an IPI to all CPUs within an enclave
is a common pattern with more frequent use in support of SGX2.

Make the (empty) IPI callback function available internally in
preparation for usage by SGX2.

Reviewed-by: Jarkko Sakkinen <[email protected]>
Signed-off-by: Reinette Chatre <[email protected]>
---
No changes since V4.

Changes since V3:
- Add Jarkko's Reviewed-by tag.

Changes since V1:
- Replace "for more usages" by "for usage by SGX2" (Jarkko)

arch/x86/kernel/cpu/sgx/main.c | 2 +-
arch/x86/kernel/cpu/sgx/sgx.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
index bb0d5e8905be..bab2a2e13c70 100644
--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -174,7 +174,7 @@ static int __sgx_encl_ewb(struct sgx_epc_page *epc_page, void *va_slot,
return ret;
}

-static void sgx_ipi_cb(void *info)
+void sgx_ipi_cb(void *info)
{
}

diff --git a/arch/x86/kernel/cpu/sgx/sgx.h b/arch/x86/kernel/cpu/sgx/sgx.h
index 0f17def9fe6f..b30cee4de903 100644
--- a/arch/x86/kernel/cpu/sgx/sgx.h
+++ b/arch/x86/kernel/cpu/sgx/sgx.h
@@ -90,6 +90,8 @@ void sgx_mark_page_reclaimable(struct sgx_epc_page *page);
int sgx_unmark_page_reclaimable(struct sgx_epc_page *page);
struct sgx_epc_page *sgx_alloc_epc_page(void *owner, bool reclaim);

+void sgx_ipi_cb(void *info);
+
#ifdef CONFIG_X86_SGX_KVM
int __init sgx_vepc_init(void);
#else
--
2.25.1


2022-05-10 23:47:30

by Reinette Chatre

[permalink] [raw]
Subject: [PATCH V5 25/31] selftests/sgx: Introduce dynamic entry point

The test enclave (test_encl.elf) is built with two initialized
Thread Control Structures (TCS) included in the binary. Both TCS are
initialized with the same entry point, encl_entry, that correctly
computes the absolute address of the stack based on the stack of each
TCS that is also built into the binary.

A new TCS can be added dynamically to the enclave and requires to be
initialized with an entry point used to enter the enclave. Since the
existing entry point, encl_entry, assumes that the TCS and its stack
exists at particular offsets within the binary it is not able to handle
a dynamically added TCS and its stack.

Introduce a new entry point, encl_dyn_entry, that initializes the
absolute address of that thread's stack to the address immediately
preceding the TCS itself. It is now possible to dynamically add a
contiguous memory region to the enclave with the new stack preceding
the new TCS. With the new TCS initialized with encl_dyn_entry as entry
point the absolute address of the stack is computed correctly on entry.

Acked-by: Jarkko Sakkinen <[email protected]>
Signed-off-by: Reinette Chatre <[email protected]>
---
No changes since V4.

Changes since V3:
- Add Jarkko's Acked-by tag.

tools/testing/selftests/sgx/test_encl_bootstrap.S | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/tools/testing/selftests/sgx/test_encl_bootstrap.S b/tools/testing/selftests/sgx/test_encl_bootstrap.S
index 82fb0dfcbd23..03ae0f57e29d 100644
--- a/tools/testing/selftests/sgx/test_encl_bootstrap.S
+++ b/tools/testing/selftests/sgx/test_encl_bootstrap.S
@@ -45,6 +45,12 @@ encl_entry:
# TCS #2. By adding the value of encl_stack to it, we get
# the absolute address for the stack.
lea (encl_stack)(%rbx), %rax
+ jmp encl_entry_core
+encl_dyn_entry:
+ # Entry point for dynamically created TCS page expected to follow
+ # its stack directly.
+ lea -1(%rbx), %rax
+encl_entry_core:
xchg %rsp, %rax
push %rax

--
2.25.1


2022-05-11 00:09:05

by Reinette Chatre

[permalink] [raw]
Subject: [PATCH V5 23/31] selftests/sgx: Add test for TCS page permission changes

Kernel should not allow permission changes on TCS pages. Add test to
confirm this behavior.

Acked-by: Jarkko Sakkinen <[email protected]>
Signed-off-by: Reinette Chatre <[email protected]>
---
No changes since V4.

Changes since V3:
- Add Jarkko's Acked-by tag.
- User provides only new permissions in
SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS ioctl(), replacing secinfo. (Jarkko)
- Use SGX page permission bits instead of VMA protection bits.

Changes since V2:
- Update to use new struct name struct sgx_enclave_restrict_perm -> struct
sgx_enclave_restrict_permissions. (Jarkko)

Changes since V1:
- Adapt test to the kernel interface changes: the ioctl() name change
and providing entire secinfo as parameter.
- Rewrite error path to reduce line lengths.

tools/testing/selftests/sgx/main.c | 71 ++++++++++++++++++++++++++++++
1 file changed, 71 insertions(+)

diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c
index 46eac09cd955..016ae3e5f398 100644
--- a/tools/testing/selftests/sgx/main.c
+++ b/tools/testing/selftests/sgx/main.c
@@ -121,6 +121,24 @@ static Elf64_Sym *vdso_symtab_get(struct vdso_symtab *symtab, const char *name)
return NULL;
}

+/*
+ * Return the offset in the enclave where the TCS segment can be found.
+ * The first RW segment loaded is the TCS.
+ */
+static off_t encl_get_tcs_offset(struct encl *encl)
+{
+ int i;
+
+ for (i = 0; i < encl->nr_segments; i++) {
+ struct encl_segment *seg = &encl->segment_tbl[i];
+
+ if (i == 0 && seg->prot == (PROT_READ | PROT_WRITE))
+ return seg->offset;
+ }
+
+ return -1;
+}
+
/*
* Return the offset in the enclave where the data segment can be found.
* The first RW segment loaded is the TCS, skip that to get info on the
@@ -567,6 +585,59 @@ TEST_F(enclave, pte_permissions)
EXPECT_EQ(self->run.exception_addr, 0);
}

+/*
+ * Modifying permissions of TCS page should not be possible.
+ */
+TEST_F(enclave, tcs_permissions)
+{
+ struct sgx_enclave_restrict_permissions ioc;
+ int ret, errno_save;
+
+ ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata));
+
+ memset(&self->run, 0, sizeof(self->run));
+ self->run.tcs = self->encl.encl_base;
+
+ memset(&ioc, 0, sizeof(ioc));
+
+ /*
+ * Ensure kernel supports needed ioctl() and system supports needed
+ * commands.
+ */
+
+ ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS, &ioc);
+ errno_save = ret == -1 ? errno : 0;
+
+ /*
+ * Invalid parameters were provided during sanity check,
+ * expect command to fail.
+ */
+ ASSERT_EQ(ret, -1);
+
+ /* ret == -1 */
+ if (errno_save == ENOTTY)
+ SKIP(return,
+ "Kernel does not support SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS ioctl()");
+ else if (errno_save == ENODEV)
+ SKIP(return, "System does not support SGX2");
+
+ /*
+ * Attempt to make TCS page read-only. This is not allowed and
+ * should be prevented by the kernel.
+ */
+ ioc.offset = encl_get_tcs_offset(&self->encl);
+ ioc.length = PAGE_SIZE;
+ ioc.permissions = SGX_SECINFO_R;
+
+ ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS, &ioc);
+ errno_save = ret == -1 ? errno : 0;
+
+ EXPECT_EQ(ret, -1);
+ EXPECT_EQ(errno_save, EINVAL);
+ EXPECT_EQ(ioc.result, 0);
+ EXPECT_EQ(ioc.count, 0);
+}
+
/*
* Enclave page permission test.
*
--
2.25.1


2022-05-11 06:32:51

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH V5 00/31] x86/sgx and selftests/sgx: Support SGX2

On Tue, May 10, 2022 at 11:08:36AM -0700, Reinette Chatre wrote:
> V4: https://lore.kernel.org/lkml/[email protected]/
>
> 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/[email protected]/
>
> 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/[email protected]/
>
> 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/[email protected]/
>
> 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/[email protected]/
> 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
> --
> 2.25.1
>

If there is any patch that does not have my reviewed-by, please put it
there. I was totally happy with v4 already. I went through these, and
did not see anything worth of complaining about.

Great job, thank you for doing this.

I can also add my tag separely to each patch, which have not have it on
request if that makes things easier in any possible way on request.

BR, Jarkko

2022-05-11 07:59:18

by Reinette Chatre

[permalink] [raw]
Subject: [PATCH V5 26/31] selftests/sgx: Introduce TCS initialization enclave operation

The Thread Control Structure (TCS) contains meta-data used by the
hardware to save and restore thread specific information when
entering/exiting the enclave. A TCS can be added to an initialized
enclave by first adding a new regular enclave page, initializing the
content of the new page from within the enclave, and then changing that
page's type to a TCS.

Support the initialization of a TCS from within the enclave.
The variable information needed that should be provided from outside
the enclave is the address of the TCS, address of the State Save Area
(SSA), and the entry point that the thread should use to enter the
enclave. With this information provided all needed fields of a TCS
can be initialized.

Acked-by: Jarkko Sakkinen <[email protected]>
Signed-off-by: Reinette Chatre <[email protected]>
---
No changes since V4.

Changes since V3:
- Add Jarkko's Acked-by tag.

tools/testing/selftests/sgx/defines.h | 8 +++++++
tools/testing/selftests/sgx/test_encl.c | 30 +++++++++++++++++++++++++
2 files changed, 38 insertions(+)

diff --git a/tools/testing/selftests/sgx/defines.h b/tools/testing/selftests/sgx/defines.h
index b638eb98c80c..d8587c971941 100644
--- a/tools/testing/selftests/sgx/defines.h
+++ b/tools/testing/selftests/sgx/defines.h
@@ -26,6 +26,7 @@ enum encl_op_type {
ENCL_OP_NOP,
ENCL_OP_EACCEPT,
ENCL_OP_EMODPE,
+ ENCL_OP_INIT_TCS_PAGE,
ENCL_OP_MAX,
};

@@ -68,4 +69,11 @@ struct encl_op_emodpe {
uint64_t flags;
};

+struct encl_op_init_tcs_page {
+ struct encl_op_header header;
+ uint64_t tcs_page;
+ uint64_t ssa;
+ uint64_t entry;
+};
+
#endif /* DEFINES_H */
diff --git a/tools/testing/selftests/sgx/test_encl.c b/tools/testing/selftests/sgx/test_encl.c
index 5b6c65331527..c0d6397295e3 100644
--- a/tools/testing/selftests/sgx/test_encl.c
+++ b/tools/testing/selftests/sgx/test_encl.c
@@ -57,6 +57,35 @@ static void *memcpy(void *dest, const void *src, size_t n)
return dest;
}

+static void *memset(void *dest, int c, size_t n)
+{
+ size_t i;
+
+ for (i = 0; i < n; i++)
+ ((char *)dest)[i] = c;
+
+ return dest;
+}
+
+static void do_encl_init_tcs_page(void *_op)
+{
+ struct encl_op_init_tcs_page *op = _op;
+ void *tcs = (void *)op->tcs_page;
+ uint32_t val_32;
+
+ memset(tcs, 0, 16); /* STATE and FLAGS */
+ memcpy(tcs + 16, &op->ssa, 8); /* OSSA */
+ memset(tcs + 24, 0, 4); /* CSSA */
+ val_32 = 1;
+ memcpy(tcs + 28, &val_32, 4); /* NSSA */
+ memcpy(tcs + 32, &op->entry, 8); /* OENTRY */
+ memset(tcs + 40, 0, 24); /* AEP, OFSBASE, OGSBASE */
+ val_32 = 0xFFFFFFFF;
+ memcpy(tcs + 64, &val_32, 4); /* FSLIMIT */
+ memcpy(tcs + 68, &val_32, 4); /* GSLIMIT */
+ memset(tcs + 72, 0, 4024); /* Reserved */
+}
+
static void do_encl_op_put_to_buf(void *op)
{
struct encl_op_put_to_buf *op2 = op;
@@ -100,6 +129,7 @@ void encl_body(void *rdi, void *rsi)
do_encl_op_nop,
do_encl_eaccept,
do_encl_emodpe,
+ do_encl_init_tcs_page,
};

struct encl_op_header *op = (struct encl_op_header *)rdi;
--
2.25.1


2022-05-12 19:15:02

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH V5 00/31] x86/sgx and selftests/sgx: Support SGX2

Hi Jarkko,

On 5/10/2022 3:22 PM, Jarkko Sakkinen wrote:
> If there is any patch that does not have my reviewed-by, please put it
> there. I was totally happy with v4 already. I went through these, and
> did not see anything worth of complaining about.
>
> Great job, thank you for doing this.
>
> I can also add my tag separely to each patch, which have not have it on
> request if that makes things easier in any possible way on request.

Thank you very much. I do appreciate all the feedback and testing.

All patches in this series have some tag from you, a few have "Acked-by"
instead of "Reviewed-by".

Patch 20/31 "x86/sgx: Free up EPC pages directly to support large
page ranges" is the only x86/sgx patch that has an "Acked-by" from you
instead of a "Reviewed-by". All selftests/sgx patches have an "Acked-by"
from you.

Here is a summary of your tags if you would like to make changes:

[PATCH V5 01/31] x86/sgx: Add short descriptions to ENCLS wrappers
Reviewed-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 02/31] x86/sgx: Add wrapper for SGX2 EMODPR function
Reviewed-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 03/31] x86/sgx: Add wrapper for SGX2 EMODT function
Reviewed-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 04/31] x86/sgx: Add wrapper for SGX2 EAUG function
Reviewed-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 05/31] x86/sgx: Support loading enclave page without VMA
Reviewed-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 06/31] x86/sgx: Export sgx_encl_ewb_cpumask()
Reviewed-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 07/31] x86/sgx: Rename sgx_encl_ewb_cpumask() as sgx_encl_cpumask()
Reviewed-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 08/31] x86/sgx: Move PTE zap code to new sgx_zap_enclave_ptes()
Reviewed-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 09/31] x86/sgx: Make sgx_ipi_cb() available internally
Reviewed-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 10/31] x86/sgx: Create utility to validate user provided offset and length
Reviewed-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 11/31] x86/sgx: Keep record of SGX page type
Reviewed-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 12/31] x86/sgx: Export sgx_encl_{grow,shrink}()
Suggested-by: Jarkko Sakkinen <[email protected]>
Reviewed-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 13/31] x86/sgx: Export sgx_encl_page_alloc()
Signed-off-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 14/31] x86/sgx: Support VA page allocation without reclaiming
Reviewed-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 15/31] x86/sgx: Support restricting of enclave page permissions
Reviewed-by: Jarkko Sakkinen <[email protected]>
Tested-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 16/31] x86/sgx: Support adding of pages to an initialized enclave
Reviewed-by: Jarkko Sakkinen <[email protected]>
Tested-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 17/31] x86/sgx: Tighten accessible memory range after enclave initialization
Reviewed-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 18/31] x86/sgx: Support modifying SGX page type
Reviewed-by: Jarkko Sakkinen <[email protected]>
Tested-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 19/31] x86/sgx: Support complete page removal
Reviewed-by: Jarkko Sakkinen <[email protected]>
Tested-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 20/31] x86/sgx: Free up EPC pages directly to support large page ranges
Acked-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 21/31] Documentation/x86: Introduce enclave runtime management section
Reviewed-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 22/31] selftests/sgx: Add test for EPCM permission changes
Acked-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 23/31] selftests/sgx: Add test for TCS page permission changes
Acked-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 24/31] selftests/sgx: Test two different SGX2 EAUG flows
Acked-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 25/31] selftests/sgx: Introduce dynamic entry point
Acked-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 26/31] selftests/sgx: Introduce TCS initialization enclave operation
Acked-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 27/31] selftests/sgx: Test complete changing of page type flow
Acked-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 28/31] selftests/sgx: Test faulty enclave behavior
Acked-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 29/31] selftests/sgx: Test invalid access to removed enclave page
Acked-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 30/31] selftests/sgx: Test reclaiming of untouched page
Acked-by: Jarkko Sakkinen <[email protected]>

[PATCH V5 31/31] selftests/sgx: Page removal stress test
Acked-by: Jarkko Sakkinen <[email protected]>


Reinette

2022-05-14 01:05:27

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH V5 00/31] x86/sgx and selftests/sgx: Support SGX2

On Wed, May 11, 2022 at 11:47:31AM -0700, Reinette Chatre wrote:
> Hi Jarkko,
>
> On 5/10/2022 3:22 PM, Jarkko Sakkinen wrote:
> > If there is any patch that does not have my reviewed-by, please put it
> > there. I was totally happy with v4 already. I went through these, and
> > did not see anything worth of complaining about.
> >
> > Great job, thank you for doing this.
> >
> > I can also add my tag separely to each patch, which have not have it on
> > request if that makes things easier in any possible way on request.
>
> Thank you very much. I do appreciate all the feedback and testing.
>
> All patches in this series have some tag from you, a few have "Acked-by"
> instead of "Reviewed-by".
>
> Patch 20/31 "x86/sgx: Free up EPC pages directly to support large
> page ranges" is the only x86/sgx patch that has an "Acked-by" from you
> instead of a "Reviewed-by". All selftests/sgx patches have an "Acked-by"
> from you.
>
> Here is a summary of your tags if you would like to make changes:
>
> [PATCH V5 01/31] x86/sgx: Add short descriptions to ENCLS wrappers
> Reviewed-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 02/31] x86/sgx: Add wrapper for SGX2 EMODPR function
> Reviewed-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 03/31] x86/sgx: Add wrapper for SGX2 EMODT function
> Reviewed-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 04/31] x86/sgx: Add wrapper for SGX2 EAUG function
> Reviewed-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 05/31] x86/sgx: Support loading enclave page without VMA
> Reviewed-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 06/31] x86/sgx: Export sgx_encl_ewb_cpumask()
> Reviewed-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 07/31] x86/sgx: Rename sgx_encl_ewb_cpumask() as sgx_encl_cpumask()
> Reviewed-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 08/31] x86/sgx: Move PTE zap code to new sgx_zap_enclave_ptes()
> Reviewed-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 09/31] x86/sgx: Make sgx_ipi_cb() available internally
> Reviewed-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 10/31] x86/sgx: Create utility to validate user provided offset and length
> Reviewed-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 11/31] x86/sgx: Keep record of SGX page type
> Reviewed-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 12/31] x86/sgx: Export sgx_encl_{grow,shrink}()
> Suggested-by: Jarkko Sakkinen <[email protected]>
> Reviewed-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 13/31] x86/sgx: Export sgx_encl_page_alloc()
> Signed-off-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 14/31] x86/sgx: Support VA page allocation without reclaiming
> Reviewed-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 15/31] x86/sgx: Support restricting of enclave page permissions
> Reviewed-by: Jarkko Sakkinen <[email protected]>
> Tested-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 16/31] x86/sgx: Support adding of pages to an initialized enclave
> Reviewed-by: Jarkko Sakkinen <[email protected]>
> Tested-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 17/31] x86/sgx: Tighten accessible memory range after enclave initialization
> Reviewed-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 18/31] x86/sgx: Support modifying SGX page type
> Reviewed-by: Jarkko Sakkinen <[email protected]>
> Tested-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 19/31] x86/sgx: Support complete page removal
> Reviewed-by: Jarkko Sakkinen <[email protected]>
> Tested-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 20/31] x86/sgx: Free up EPC pages directly to support large page ranges
> Acked-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 21/31] Documentation/x86: Introduce enclave runtime management section
> Reviewed-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 22/31] selftests/sgx: Add test for EPCM permission changes
> Acked-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 23/31] selftests/sgx: Add test for TCS page permission changes
> Acked-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 24/31] selftests/sgx: Test two different SGX2 EAUG flows
> Acked-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 25/31] selftests/sgx: Introduce dynamic entry point
> Acked-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 26/31] selftests/sgx: Introduce TCS initialization enclave operation
> Acked-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 27/31] selftests/sgx: Test complete changing of page type flow
> Acked-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 28/31] selftests/sgx: Test faulty enclave behavior
> Acked-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 29/31] selftests/sgx: Test invalid access to removed enclave page
> Acked-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 30/31] selftests/sgx: Test reclaiming of untouched page
> Acked-by: Jarkko Sakkinen <[email protected]>
>
> [PATCH V5 31/31] selftests/sgx: Page removal stress test
> Acked-by: Jarkko Sakkinen <[email protected]>
>
>
> Reinette

It looks good. And yeah, I've been running different versions of this patch
set since April with zero issues, about a month, in our platform. No high
doubts that anything would wrong that could not be later fixed, if problems
arise.

BR, Jarkko

2022-06-01 19:51:22

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH V5 00/31] x86/sgx and selftests/sgx: Support SGX2

On Tue, 2022-05-10 at 11:08 -0700, Reinette Chatre wrote:
> V4: https://lore.kernel.org/lkml/[email protected]/
>
> 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/[email protected]/
>
> 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/[email protected]/
>
> 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/[email protected]/
>
> 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/[email protected]/
> 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