Browse Source

[Pal/Linux-SGX] Distinguish between DkProcessExit() and DkThreadExit()

Previously, both DkProcessExit() and DkThreadExit() used SGX OCALL
ocall_exit(exitcode), which finally issued exit() syscall. This is
incorrect because DkProcessExit() must exit the whole process and not
just a single thread. This led to abandoned IPC/Async helper threads
in some Graphene-SGX corner cases. This commit forces DkProcessExit()
to result in exit_group() syscall, achieved by adding a new argument
to ocall_exit(exitcode, is_exitgroup).
Dmitrii Kuvaiskii 4 years ago
parent
commit
b5465d7b57

+ 3 - 3
Pal/src/host/Linux-SGX/db_main.c

@@ -302,7 +302,7 @@ void pal_linux_main(char * uptr_args, uint64_t args_size,
     rv = init_enclave();
     if (rv) {
         SGX_DBG(DBG_E, "Failed to initialize enclave properties: %d\n", rv);
-        ocall_exit(rv);
+        ocall_exit(rv, /*is_exitgroup=*/true);
     }
 
     if (args_size > MAX_ARGS_SIZE || env_size > MAX_ENV_SIZE) {
@@ -323,7 +323,7 @@ void pal_linux_main(char * uptr_args, uint64_t args_size,
     if (pal_sec.ppid) {
         if ((rv = init_child_process(&parent)) < 0) {
             SGX_DBG(DBG_E, "Failed to initialize child process: %d\n", rv);
-            ocall_exit(rv);
+            ocall_exit(rv, /*is_exitgroup=*/true);
         }
     }
 
@@ -366,7 +366,7 @@ void pal_linux_main(char * uptr_args, uint64_t args_size,
     const char * errstring = NULL;
     if ((rv = read_config(root_config, loader_filter, &errstring)) < 0) {
         SGX_DBG(DBG_E, "Can't read manifest: %s, error code %d\n", errstring, rv);
-        ocall_exit(rv);
+        ocall_exit(rv, /*is_exitgroup=*/true);
     }
 
     pal_state.root_config = root_config;

+ 1 - 1
Pal/src/host/Linux-SGX/db_process.c

@@ -327,7 +327,7 @@ noreturn void _DkProcessExit (int exitcode)
 #endif
     if (exitcode)
         SGX_DBG(DBG_I, "DkProcessExit: Returning exit code %d\n", exitcode);
-    ocall_exit(exitcode);
+    ocall_exit(exitcode, /*is_exitgroup=*/true);
     while (true) {
         /* nothing */;
     }

+ 1 - 1
Pal/src/host/Linux-SGX/db_threading.c

@@ -140,7 +140,7 @@ void _DkThreadYieldExecution (void)
 /* _DkThreadExit for internal use: Thread exiting */
 noreturn void _DkThreadExit (void)
 {
-    ocall_exit(0);
+    ocall_exit(0, /*is_exitgroup=*/false);
 }
 
 int _DkThreadResume (PAL_HANDLE threadHandle)

+ 1 - 1
Pal/src/host/Linux-SGX/enclave_framework.c

@@ -1236,7 +1236,7 @@ out:
 void restore_sgx_context(sgx_context_t *ctx) {
     if (((uint64_t) ctx) != ctx->rsp - (sizeof(sgx_context_t) + RED_ZONE_SIZE)) {
         SGX_DBG(DBG_E, "Invalid sgx_context_t pointer passed to restore_sgx_context!\n");
-        ocall_exit(1);
+        ocall_exit(1, /*is_exitgroup=*/false);
     }
 
     _restore_sgx_context(ctx);

+ 8 - 3
Pal/src/host/Linux-SGX/enclave_ocalls.c

@@ -11,15 +11,20 @@
 #include <api.h>
 #include <asm/errno.h>
 
-noreturn void ocall_exit(int exitcode)
+noreturn void ocall_exit(int exitcode, int is_exitgroup)
 {
-    int64_t code = exitcode;
+    ms_ocall_exit_t * ms;
+
+    ms = sgx_alloc_on_ustack(sizeof(*ms));
+    ms->ms_exitcode     = exitcode;
+    ms->ms_is_exitgroup = is_exitgroup;
+
     // There are two reasons for this loop:
     //  1. Ocalls can be interuppted.
     //  2. We can't trust the outside to actually exit, so we need to ensure
     //     that we never return even when the outside tries to trick us.
     while (true) {
-        sgx_ocall(OCALL_EXIT, (void *) code);
+        sgx_ocall(OCALL_EXIT, ms);
     }
 }
 

+ 1 - 1
Pal/src/host/Linux-SGX/enclave_ocalls.h

@@ -8,7 +8,7 @@
 #include <linux/socket.h>
 #include <linux/poll.h>
 
-noreturn void ocall_exit (int exitcode);
+noreturn void ocall_exit (int exitcode, int is_exitgroup);
 
 int ocall_print_string (const char * str, unsigned int length);
 

+ 1 - 1
Pal/src/host/Linux-SGX/enclave_pages.c

@@ -60,7 +60,7 @@ static void assert_vma_list (void)
             if (pal_sec.in_gdb)
                 __asm__ volatile ("int $3" ::: "memory");
 #endif
-            ocall_exit();
+            ocall_exit(1, /*is_exitgroup=*/true);
         }
         last_addr = vma->bottom;
     }

+ 5 - 0
Pal/src/host/Linux-SGX/ocall_types.h

@@ -58,6 +58,11 @@ enum {
 
 #define OCALL_NO_TIMEOUT   ((int64_t)-1)
 
+typedef struct {
+    int ms_exitcode;
+    int ms_is_exitgroup;
+} ms_ocall_exit_t;
+
 typedef struct {
     const char * ms_str;
     unsigned int ms_length;

+ 13 - 6
Pal/src/host/Linux-SGX/sgx_enclave.c

@@ -19,15 +19,22 @@
 
 #define ODEBUG(code, ms) do {} while (0)
 
-static int sgx_ocall_exit(void* prv)
+static int sgx_ocall_exit(void* pms)
 {
-    int64_t rv = (int64_t) prv;
+    ms_ocall_exit_t * ms = (ms_ocall_exit_t *) pms;
     ODEBUG(OCALL_EXIT, NULL);
-    if (rv != (int64_t) ((uint8_t) rv)) {
-        SGX_DBG(DBG_E, "Saturation error in exit code %ld, getting rounded down to %u\n", rv, (uint8_t) rv);
-        rv = 255;
+
+    if (ms->ms_exitcode != (int) ((uint8_t) ms->ms_exitcode)) {
+        SGX_DBG(DBG_E, "Saturation error in exit code %d, getting rounded down to %u\n",
+                ms->ms_exitcode, (uint8_t) ms->ms_exitcode);
+        ms->ms_exitcode = 255;
     }
-    INLINE_SYSCALL(exit, 1, (int)rv);
+
+    if (ms->ms_is_exitgroup)
+        INLINE_SYSCALL(exit_group, 1, (int)ms->ms_exitcode);
+    else
+        INLINE_SYSCALL(exit, 1, (int)ms->ms_exitcode);
+
     return 0;
 }