From b1b93564b9c3aa50c12e21828a981febb07d3be6 Mon Sep 17 00:00:00 2001 From: Ming-Wei Shih Date: Tue, 30 Nov 2021 00:53:48 +0000 Subject: [PATCH] Do not update td state in the case of direct return Signed-off-by: Ming-Wei Shih --- enclave/core/sgx/exit.S | 23 +++++-- host/sgx/enclavemanager.c | 2 +- tests/sgx/thread_interrupt/enc/enc.c | 61 ++++++++++++++++--- tests/sgx/thread_interrupt/host/host.c | 11 ++-- .../sgx/thread_interrupt/thread_interrupt.edl | 2 +- 5 files changed, 78 insertions(+), 21 deletions(-) diff --git a/enclave/core/sgx/exit.S b/enclave/core/sgx/exit.S index ede93b1d88..68f9223444 100644 --- a/enclave/core/sgx/exit.S +++ b/enclave/core/sgx/exit.S @@ -58,8 +58,9 @@ oe_asm_exit: mov %rdx, %r11 .determine_exit_type: - // Check if the argument direct_return is set - cmp $1, %rcx + // Save the direct_return argument in r13 and check if it is set + mov %rcx, %r13 + cmp $1, %r13 je .return // Check the depth of the ECALL stack (zero for clean exit) @@ -117,23 +118,33 @@ oe_asm_exit: mov SGX_SSA_URSP_OFFSET(%r12), %rsp mov SGX_SSA_URBP_OFFSET(%r12), %rbp + // Bypass update_td_state if direct_return is set + cmp $1, %r13 + je .execute_eexit + +.update_td_state: + lfence + // Do not update the state if the enclave exits in the middle // the exception handling (e.g., exiting from the first-level - // exception handler) or exits after an illegal instruction - // emulation + // exception handler), exits after an illegal instruction + // emulation, or the enclave is aborted cmpq $TD_STATE_SECOND_LEVEL_EXCEPTION_HANDLING, td_state(%r11) je .execute_eexit cmpq $TD_STATE_FIRST_LEVEL_EXCEPTION_HANDLING, td_previous_state(%r11) je .execute_eexit + cmpq $TD_STATE_ABORTED, td_state(%r11) + je .execute_eexit + + lfence // Update the state to indicate that the enclave is returning // to the host movq $TD_STATE_EXITED, td_state(%r11) +.execute_eexit: oe_cleanup_registers -.execute_eexit: - lfence // Check oe_sgx_td_t.simulate flag // simulate-flag-check. mov td_simulate(%r11), %rax diff --git a/host/sgx/enclavemanager.c b/host/sgx/enclavemanager.c index b0933ba29c..76aa29752c 100644 --- a/host/sgx/enclavemanager.c +++ b/host/sgx/enclavemanager.c @@ -201,7 +201,7 @@ oe_enclave_t* oe_query_enclave_instance(void* tcs) } if (!ret) - OE_TRACE_ERROR("tcs=0x%x\n", tcs); + OE_TRACE_ERROR("tcs=0x%lx\n", tcs); return ret; } diff --git a/tests/sgx/thread_interrupt/enc/enc.c b/tests/sgx/thread_interrupt/enc/enc.c index 1077c1f272..7b9fd5caf4 100644 --- a/tests/sgx/thread_interrupt/enc/enc.c +++ b/tests/sgx/thread_interrupt/enc/enc.c @@ -232,6 +232,26 @@ void enc_thread_interrupt_nonblocking(void) host_join_thread(); } +uint64_t thread_terminate_handler(oe_exception_record_t* exception_record) +{ + int self_tid = 0; + + OE_TEST(exception_record->code == OE_EXCEPTION_UNKNOWN); + + host_get_tid(&self_tid); + OE_TEST(_thread_info_blocking.tid == self_tid); + + printf("(tid=%d) thread is interrupted...\n", self_tid); + + OE_TEST( + _thread_info_blocking.td->state == + OE_TD_STATE_SECOND_LEVEL_EXCEPTION_HANDLING); + + __atomic_store_n(&_thread_info_blocking.lock, 2, __ATOMIC_RELEASE); + + return OE_EXCEPTION_CONTINUE_EXECUTION; +} + void enc_run_thread_blocking(int tid) { oe_result_t result = OE_OK; @@ -242,7 +262,7 @@ void enc_run_thread_blocking(int tid) printf("(tid=%d) blocking thread is running...\n", self_tid); OE_CHECK( - oe_add_vectored_exception_handler(false, thread_interrupt_handler)); + oe_add_vectored_exception_handler(false, thread_terminate_handler)); _thread_info_blocking.tid = tid; _thread_info_blocking.td = oe_sgx_get_td(); @@ -268,7 +288,7 @@ void enc_run_thread_blocking(int tid) return; } -void enc_thread_interrupt_blocking(void) +void enc_thread_interrupt_blocking(int* ret) { oe_result_t result; int tid = 0; @@ -295,9 +315,9 @@ void enc_thread_interrupt_blocking(void) _handler_entered = 0; while (!_handler_entered) { - OE_TEST( - oe_sgx_td_register_host_signal(_thread_info_blocking.td, SIGUSR1) == - true); + if (oe_sgx_td_register_host_signal(_thread_info_blocking.td, SIGUSR1) != + true) + break; printf( "(tid=%d) Sending registered signal (SIGUSR1) to (td=0x%lx, " @@ -307,6 +327,9 @@ void enc_thread_interrupt_blocking(void) _thread_info_blocking.tid, ++retry); + if (_thread_info_blocking.td->state != OE_TD_STATE_RUNNING) + break; + host_send_interrupt(_thread_info_blocking.tid, SIGUSR1); if (retry == 10) @@ -320,7 +343,8 @@ void enc_thread_interrupt_blocking(void) host_sleep_msec(30); } - OE_TEST(retry == 10); + if (retry != 10) + goto done; retry = 0; _handler_entered = 0; @@ -339,6 +363,9 @@ void enc_thread_interrupt_blocking(void) _thread_info_blocking.tid, ++retry); + if (_thread_info_blocking.td->state != OE_TD_STATE_RUNNING) + break; + host_send_interrupt(_thread_info_blocking.tid, SIGUSR2); if (retry == 10) @@ -352,9 +379,27 @@ void enc_thread_interrupt_blocking(void) host_sleep_msec(30); } - OE_TEST(retry == 10); + if (retry != 10) + goto done; - oe_abort(); + *ret = 1; + +done: + /* unblock and shutdown the thread */ + OE_TEST( + oe_sgx_td_register_host_signal(_thread_info_blocking.td, SIGUSR2) == + true); + + printf( + "(tid=%d) Shutting down (td=0x%lx, " + "tid=%d)\n", + tid, + (uint64_t)_thread_info_blocking.td, + _thread_info_blocking.tid); + + host_send_interrupt(_thread_info_blocking.tid, SIGUSR2); + + host_join_thread(); } OE_SET_ENCLAVE_SGX( diff --git a/tests/sgx/thread_interrupt/host/host.c b/tests/sgx/thread_interrupt/host/host.c index e6c3f196bd..d61ef3a8d5 100644 --- a/tests/sgx/thread_interrupt/host/host.c +++ b/tests/sgx/thread_interrupt/host/host.c @@ -98,11 +98,12 @@ int main(int argc, const char* argv[]) } else if (!strcmp(argv[2], "blocking")) { - result = enc_thread_interrupt_blocking(enclave); - OE_TEST(result == OE_ENCLAVE_ABORTING); - /* Expcet a non-OE_OK result. The error code may be different - * between debug and release build. */ - OE_TEST(oe_terminate_enclave(enclave) != OE_OK); + int ret = 0; + result = enc_thread_interrupt_blocking(enclave, &ret); + OE_TEST(ret == 1); + if (result != OE_OK) + oe_put_err("oe_call_enclave() failed: result=%u", result); + OE_TEST(oe_terminate_enclave(enclave) == OE_OK); } else { diff --git a/tests/sgx/thread_interrupt/thread_interrupt.edl b/tests/sgx/thread_interrupt/thread_interrupt.edl index 819b95029c..47a67efbc4 100644 --- a/tests/sgx/thread_interrupt/thread_interrupt.edl +++ b/tests/sgx/thread_interrupt/thread_interrupt.edl @@ -8,7 +8,7 @@ enclave { trusted { public void enc_thread_interrupt_nonblocking(); - public void enc_thread_interrupt_blocking(); + public void enc_thread_interrupt_blocking([in, out] int* ret); public void enc_run_thread_nonblocking(int tid); public void enc_run_thread_blocking(int tid); };