Sfoglia il codice sorgente

[Pal/Linux-SGX] Use dedicate stack for vfork() child

With vfork(), parent and child share the same stack. The child must not
modify the parent's stack frame (otherwise parent's local variables are
corrupted). To circumvent this, this commit introduces a dedicated noinline
function to have a stack frame local to the child. (This bug was hit on
GCC 8 which reuses the same stack location for local variables with disjoint
lifetimes.)
Isaku Yamahata 6 anni fa
parent
commit
4077c5a816
1 ha cambiato i file con 35 aggiunte e 18 eliminazioni
  1. 35 18
      Pal/src/host/Linux-SGX/sgx_process.c

+ 35 - 18
Pal/src/host/Linux-SGX/sgx_process.c

@@ -46,6 +46,40 @@ struct proc_args {
     unsigned int    mcast_port;
 };
 
+/*
+ * vfork() shares stack between child and parent. Any stack modifications in
+ * child are reflected in parent's stack. Compiler may unwittingly modify
+ * child's stack for its own purposes and thus corrupt parent's stack
+ * (e.g., GCC re-uses the same stack area for local vars with non-overlapping
+ * lifetimes).
+ * Introduce noinline function with stack area used only by child.
+ * Make this function non-local to keep function signature.
+ * NOTE: more tricks may be needed to prevent unexpected optimization for
+ * future compiler.
+ */
+int __attribute_noinline
+vfork_exec(int pipe_input, int proc_fds[3], const char** argv)
+{
+    int ret = ARCH_VFORK();
+    if (ret)
+        return ret;
+
+    /* child */
+    for (int i = 0 ; i < 3 ; i++)
+        INLINE_SYSCALL(close, 1, proc_fds[i]);
+
+    ret = INLINE_SYSCALL(dup2, 2, pipe_input, PROC_INIT_FD);
+    if (!IS_ERR(ret)) {
+        extern char** environ;
+        ret = INLINE_SYSCALL(execve, 3, PAL_LOADER, argv, environ);
+
+        /* shouldn't get to here */
+        SGX_DBG(DBG_E, "unexpected failure of new process\n");
+    }
+    __asm__ volatile ("hlt");
+    return 0;
+}
+
 int sgx_create_process (const char * uri, int nargs, const char ** args,
                         int * retfds)
 {
@@ -71,27 +105,10 @@ int sgx_create_process (const char * uri, int nargs, const char ** args,
     memcpy(argv + 1, args, sizeof(const char *) * nargs);
     argv[nargs + 1] = NULL;
 
-    ret = ARCH_VFORK();
-
+    ret = vfork_exec(proc_fds[0][0], proc_fds[1], argv);
     if (IS_ERR(ret))
         goto out;
 
-    if (!ret) {
-        for (int i = 0 ; i < 3 ; i++)
-            INLINE_SYSCALL(close, 1, proc_fds[1][i]);
-
-        ret = INLINE_SYSCALL(dup2, 2, proc_fds[0][0], PROC_INIT_FD);
-        if (!IS_ERR(ret)) {
-            extern char** environ;
-            ret = INLINE_SYSCALL(execve, 3, PAL_LOADER, argv, environ);
-
-            /* shouldn't get to here */
-            SGX_DBG(DBG_E, "unexpected failure of new process\n");
-        }
-        __asm__ volatile ("hlt");
-        return 0;
-    }
-
     child = ret;
 
     for (int i = 0 ; i < 3 ; i++)