From 79ce80ff9a396f2fa489ad494fd77422504d6133 Mon Sep 17 00:00:00 2001 From: Dmitrii Kuvaiskii Date: Fri, 26 Jul 2024 03:12:42 -0700 Subject: [PATCH] [PAL/Linux-SGX] Read exitless-OCALL result before resetting the stack Previously, there was a data race that the exitless-OCALL logic read the result of the OCALL from the stack *after* the code reset the stack. However, this stack is shared with the AEX flows. As of now, the AEX flows use the stack only in debug mode, so this data race flew under the radar for a long time. What can happen is that right-before reading the result of the exitless OCALL, the enclave thread is interrupted, an AEX logic is executed and modifies the values on the stack, then the exitless-OCALL logic is resumed, and the enclave thread reads an AEX-modified OCALL result value. Future commits (e.g., AEX-Notify) will introduce more AEX flows and expose this data race. So let's fix it now. Signed-off-by: Dmitrii Kuvaiskii --- pal/src/host/linux-sgx/enclave_ocalls.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pal/src/host/linux-sgx/enclave_ocalls.c b/pal/src/host/linux-sgx/enclave_ocalls.c index c7259056e9..5e898bebc6 100644 --- a/pal/src/host/linux-sgx/enclave_ocalls.c +++ b/pal/src/host/linux-sgx/enclave_ocalls.c @@ -136,8 +136,12 @@ static long sgx_exitless_ocall(uint64_t code, void* ocall_args) { } } + /* important to copy req->result before resetting the stack, otherwise it may be overwritten; + * this enclave's stack is also used in AEX flows, see host_entry.S:async_exit_pointer() */ + long result = COPY_UNTRUSTED_VALUE(&req->result); sgx_reset_ustack(old_ustack); - return COPY_UNTRUSTED_VALUE(&req->result); + + return result; } __attribute_no_sanitize_address