Error propagation bugfix in PAL initialization (#114)

Fix a bug where configuration error ends up doing a huge allocation, rather than catching the error.  Add some documentation to the slabmgr code.

Fix a stack corruption on signal delivery in a PAL call.  Basically, some important code that saves registers and other state, in case we need to jump out of the PAL, was getting optimized out.  Added some documentation about how the exception handling code works.

A bugfix for getsockname()

LibOS/shim/src/sys/shim_socket.c

@@ -93,6 +93,10 @@ static int init_port_rebase (void)
     return 0;
 }
 
+static int inet_parse_addr (int domain, int type, const char * uri,
+                            struct addr_inet * bind,
+                            struct addr_inet * conn);
+
 static int __process_pending_options (struct shim_handle * hdl);
 
 int shim_do_socket (int family, int type, int protocol)
@@ -522,6 +526,22 @@ int shim_do_bind (int sockfd, struct sockaddr * addr, socklen_t addrlen)
         dent->data = sock->addr.un.data;
     }
 
+    if (sock->domain == AF_INET || sock->domain == AF_INET6) {
+        char uri[SOCK_URI_SIZE];
+
+        if (!DkStreamGetName(pal_hdl, uri, SOCK_URI_SIZE)) {
+            ret = -PAL_ERRNO;
+            goto out;
+        }
+
+        if ((ret = inet_parse_addr(sock->domain, sock->sock_type, uri,
+                                   &sock->addr.in.bind, NULL)) < 0)
+            goto out;
+
+        inet_rebase_port(true, sock->domain, &sock->addr.in.bind, true);
+    }
+
+
     hdl->pal_handle = pal_hdl;
     __process_pending_options(hdl);
     ret = 0;

Pal/lib/slabmgr.h

@@ -116,7 +116,7 @@ struct slab_debug {
 
 #define SLAB_HDR_SIZE \
     ROUND_UP((sizeof(SLAB_OBJ_TYPE) - sizeof(LIST_TYPE(slab_obj)) +     \
-              SLAB_DEBUG_SIZE + SLAB_CANARY_SIZE), \
+              SLAB_DEBUG_SIZE + SLAB_CANARY_SIZE),                      \
              MIN_MALLOC_ALIGNMENT)
 
 #ifndef SLAB_LEVEL
@@ -226,7 +226,7 @@ static inline void __set_free_slab_area (SLAB_AREA area, SLAB_MGR mgr,
 {
     int slab_size = slab_levels[level] + SLAB_HDR_SIZE;
     mgr->addr[level] = (void *) area->raw;
-    mgr->addr_top[level] = (void *) area->raw + area->size * slab_size;
+    mgr->addr_top[level] = (void *) area->raw + (area->size * slab_size);
     mgr->size[level] += area->size;
 }
 
@@ -293,11 +293,20 @@ static inline SLAB_MGR enlarge_slab_mgr (SLAB_MGR mgr, int level)
     SLAB_AREA area;
     int size;
 
-    if (level >= SLAB_LEVEL) {
+    /* DEP 11/24/17: I don't see how this case is possible.
+     * Either way, we should be consistent with whether to
+     * return with system_lock held or not.
+     * Commenting for now and replacing with an assert */
+    /*if (level >= SLAB_LEVEL) {
         system_lock();
         goto out;
-    }
+        }*/
+    assert(level < SLAB_LEVEL);
 
+    /* DEP 11/24/17: This strategy basically doubles a level's size 
+     * every time it grows.  The assumption if we get this far is that
+     * mgr->addr == mgr->top_addr */
+    assert (mgr->addr[level] == mgr->addr_top[level]);
     size = mgr->size[level];
     area = (SLAB_AREA) system_malloc(__MAX_MEM_SIZE(slab_levels[level], size));
     if (area <= 0)
@@ -310,7 +319,7 @@ static inline SLAB_MGR enlarge_slab_mgr (SLAB_MGR mgr, int level)
     __set_free_slab_area(area, mgr, level);
     system_unlock();
 
-out:
+//out:
     return mgr;
 }
 
@@ -339,6 +348,7 @@ static inline void * slab_alloc (SLAB_MGR mgr, int size)
     }
 
     system_lock();
+    assert(mgr->addr[level] <= mgr->addr_top[level]);
     if (mgr->addr[level] == mgr->addr_top[level] &&
         listp_empty(&mgr->free_list[level])) {
         system_unlock();
@@ -353,6 +363,7 @@ static inline void * slab_alloc (SLAB_MGR mgr, int size)
         mobj = (void *) mgr->addr[level];
         mgr->addr[level] += slab_levels[level] + SLAB_HDR_SIZE;
     }
+    assert(mgr->addr[level] <= mgr->addr_top[level]);
     OBJ_LEVEL(mobj) = level;
     system_unlock();
 

Pal/src/host/Linux-SGX/db_pipes.c

@@ -69,9 +69,10 @@ static int pipe_listen (PAL_HANDLE * handle, PAL_NUM pipeid, int options)
     if ((ret = pipe_addr(pipeid, &addr)) < 0)
         return ret;
 
+    unsigned int addrlen = sizeof(struct sockaddr_un);
     struct sockopt sock_options;
     ret = ocall_sock_listen(AF_UNIX, pipe_type(options), 0,
-                            (void *) &addr, sizeof(struct sockaddr_un),
+                            (struct sockaddr *) &addr, &addrlen,
                             &sock_options);
     if (ret < 0)
         return ret;

Pal/src/host/Linux-SGX/db_sockets.c

@@ -345,14 +345,14 @@ static int tcp_listen (PAL_HANDLE * handle, char * uri, int options)
 #if ALLOW_BIND_ANY == 0
     /* the socket need to have a binding address, a null address or an
        any address is not allowed */
-    if (addr_check_any(bind_addr))
+    if (check_any_addr(bind_addr))
        return -PAL_ERROR_INVAL;
 #endif
 
     struct sockopt sock_options;
     ret = ocall_sock_listen(bind_addr->sa_family,
                             sock_type(SOCK_STREAM, options), 0,
-                            bind_addr, bind_addrlen,
+                            bind_addr, &bind_addrlen,
                             &sock_options);
     if (ret < 0)
         return ret;
@@ -550,7 +550,7 @@ static int udp_bind (PAL_HANDLE * handle, char * uri, int options)
     struct sockopt sock_options;
     ret = ocall_sock_listen(bind_addr->sa_family,
                             sock_type(SOCK_DGRAM, options), 0,
-                            bind_addr, bind_addrlen, &sock_options);
+                            bind_addr, &bind_addrlen, &sock_options);
     if (ret < 0)
         return ret;
 

Pal/src/host/Linux-SGX/enclave_ocalls.c

@@ -467,22 +467,36 @@ int ocall_socketpair (int domain, int type, int protocol,
 }
 
 int ocall_sock_listen (int domain, int type, int protocol,
-                       const struct sockaddr * addr, unsigned int addrlen,
+                       struct sockaddr * addr, unsigned int * addrlen,
                        struct sockopt * sockopt)
 {
     int retval = 0;
+    unsigned int bind_len = *addrlen;
     ms_ocall_sock_listen_t * ms;
     OCALLOC(ms, ms_ocall_sock_listen_t *, sizeof(*ms));
 
     ms->ms_domain = domain;
     ms->ms_type = type;
     ms->ms_protocol = protocol;
-    ms->ms_addr = COPY_TO_USER(addr, addrlen);
-    ms->ms_addrlen = addrlen;
+    ms->ms_addr = COPY_TO_USER(addr, bind_len);
+    ms->ms_addrlen = bind_len;
 
     retval = SGX_OCALL(OCALL_SOCK_LISTEN, ms);
-    if (retval >= 0)
-        *sockopt = ms->ms_sockopt;
+    if (retval >= 0) {
+        if (addrlen && (
+            sgx_is_within_enclave(ms->ms_addr, bind_len) ||
+            ms->ms_addrlen > bind_len)) {
+            OCALL_EXIT();
+            return -PAL_ERROR_DENIED;
+        }
+
+        if (addr) {
+            COPY_FROM_USER(addr, ms->ms_addr, ms->ms_addrlen);
+            *addrlen = ms->ms_addrlen;
+        }
+        if (sockopt)
+            *sockopt = ms->ms_sockopt;
+    }
     OCALL_EXIT();
     return retval;
 }

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

@@ -53,7 +53,7 @@ int ocall_mkdir (const char *pathname, unsigned short mode);
 int ocall_getdents (int fd, struct linux_dirent64 *dirp, unsigned int size);
 
 int ocall_sock_listen (int domain, int type, int protocol,
-                       const struct sockaddr * addr, unsigned int addrlen,
+                       struct sockaddr * addr, unsigned int * addrlen,
                        struct sockopt * opt);
 
 int ocall_sock_accept (int sockfd, struct sockaddr * addr,

Pal/src/host/Linux-SGX/pal_host.h

@@ -242,8 +242,14 @@ struct pal_frame {
     struct arch_frame           arch;
 };
 
+/* DEP 12/25/17: This frame storage thing is important to mark volatile.
+ * The compiler should not optimize out any of these changes, and 
+ * because some accesses can happen during an exception, these are not
+ * visible to the compiler in an otherwise stack-local variable (so the
+ * compiler will try to optimize out these assignments.
+ */
 static inline
-void __store_frame (struct pal_frame * frame,
+void __store_frame (volatile struct pal_frame * frame,
                     void * func, const char * funcname)
 {
     arch_store_frame(&frame->arch)
@@ -259,7 +265,7 @@ void __store_frame (struct pal_frame * frame,
 
 
 static inline
-void __clear_frame (struct pal_frame * frame)
+void __clear_frame (volatile struct pal_frame * frame)
 {
     if (frame->identifier == PAL_FRAME_IDENTIFIER) {
         asm volatile ("nop" ::: "memory");

Pal/src/host/Linux-SGX/sgx_enclave.c

@@ -297,6 +297,16 @@ static int sgx_ocall_sock_listen(void * pms)
         goto err_fd;
     }
 
+    if (ms->ms_addr) {
+        socklen_t addrlen;
+        ret = INLINE_SYSCALL(getsockname, 3, fd, ms->ms_addr, &addrlen);
+        if (IS_ERR(ret)) {
+            ret = -PAL_ERROR_DENIED;
+            goto err_fd;
+        }
+        ms->ms_addrlen = addrlen;
+    }
+
     if (ms->ms_type & SOCK_STREAM) {
         ret = INLINE_SYSCALL(listen, 2, fd, DEFAULT_BACKLOG);
         if (IS_ERR(ret)) {

Pal/src/host/Linux/db_exception.c

@@ -193,6 +193,18 @@ static bool _DkGenericSignalHandle (int event_num, siginfo_t * info,
 #define ADDR_IN_PAL(addr) \
         ((void *) (addr) > TEXT_START && (void *) (addr) < TEXT_END)
 
+/* This function walks the stack to find the PAL_FRAME
+ * that was saved upon entry to the PAL, if an exception/interrupt
+ * comes in during a PAL call.  This is needed to support the behavior that an
+ * exception in the PAL has Unix-style, EAGAIN semantics.
+ * 
+ * The PAL_FRAME is supposed to be in the first PAL frame, and we look for 
+ * it by matching a special magic number, that should only appear on the stack
+ * once.
+ * 
+ * If an exception comes in while we are not in the PAL, this PAL_FRAME won't
+ * exist, and it is ok to return NULL.
+ */
 static struct pal_frame * get_frame (ucontext_t * uc)
 {
     unsigned long rip = uc->uc_mcontext.gregs[REG_RIP];

Pal/src/host/Linux/db_sockets.c

@@ -290,7 +290,6 @@ PAL_HANDLE socket_create_handle (int type, int fd, int options,
     return hdl;
 }
 
-#if ALLOW_BIND_ANY == 0
 static bool check_zero (void * mem, size_t size)
 {
     void * p = mem, * q = mem + size;
@@ -339,7 +338,6 @@ static bool check_any_addr (struct sockaddr * addr)
 
     return false;
 }
-#endif
 
 /* listen on a tcp socket */
 static int tcp_listen (PAL_HANDLE * handle, char * uri, int options)
@@ -358,7 +356,7 @@ static int tcp_listen (PAL_HANDLE * handle, char * uri, int options)
 #if ALLOW_BIND_ANY == 0
     /* the socket need to have a binding address, a null address or an
        any address is not allowed */
-    if (addr_check_any(bind_addr))
+    if (check_any_addr(bind_addr))
        return -PAL_ERROR_INVAL;
 #endif
 
@@ -395,6 +393,13 @@ static int tcp_listen (PAL_HANDLE * handle, char * uri, int options)
         }
     }
 
+    if (check_any_addr(bind_addr)) {
+        /* call getsockname to get socket address */
+        if ((ret = INLINE_SYSCALL(getsockname, 3, fd,
+                                  bind_addr, &bind_addrlen)) < 0)
+            goto failed;
+    }
+
     ret = INLINE_SYSCALL(listen, 2, fd, DEFAULT_BACKLOG);
     if (IS_ERR(ret))
         return -PAL_ERROR_DENIED;
@@ -668,7 +673,7 @@ static int udp_bind (PAL_HANDLE * handle, char * uri, int options)
 #if ALLOW_BIND_ANY == 0
     /* the socket need to have a binding address, a null address or an
        any address is not allowed */
-    if (addr_check_any(bind_addr))
+    if (check_any_addr(bind_addr))
        return -PAL_ERROR_INVAL;
 #endif
 
@@ -730,7 +735,7 @@ static int udp_connect (PAL_HANDLE * handle, char * uri, int options)
 #if ALLOW_BIND_ANY == 0
     /* the socket need to have a binding address, a null address or an
        any address is not allowed */
-    if (bind_addr && addr_check_any(bind_addr))
+    if (bind_addr && check_any_addr(bind_addr))
        return -PAL_ERROR_INVAL;
 #endif
 

Pal/src/host/Linux/pal_host.h

@@ -185,7 +185,7 @@ typedef struct pal_handle
 
 struct arch_frame {
 #ifdef __x86_64__
-    unsigned long rsp, rbp, rbx, rsi, rdi, r12, r13, r14, r15;
+    uint64_t rsp, rbp, rbx, rsi, rdi, r12, r13, r14, r15;
 #else
 # error "unsupported architecture"
 #endif
@@ -238,8 +238,27 @@ struct pal_frame {
     struct arch_frame           arch;
 };
 
+/* When a PAL call is issued, a special PAL_FRAME is placed on the stack.
+ * This stores both a magic identifier, debugging information, 
+ * as well as callee-saved state.  This is used as a way to deal
+ * with PAL-internal failures where the goal is to exit the PAL and return a
+ * failure.
+ * 
+ * Arguably, an alternative is to unwind the stack and handle error cases at
+ * each stage.  In general, this is probably more robust, but would take work
+ * in the short term.  The one exception where the current strategy is
+ * probably better is when the PAL gets in a state where the code is
+ * unrecoverable, but ideally, this shouldn't happen.
+ */
+
+/* DEP 12/25/17: This frame storage thing is important to mark volatile.
+ * The compiler should not optimize out any of these changes, and 
+ * because some accesses can happen during an exception, these are not
+ * visible to the compiler in an otherwise stack-local variable (so the
+ * compiler will try to optimize out these assignments.
+ */
 static inline
-void __store_frame (struct pal_frame * frame,
+void __store_frame (volatile struct pal_frame * frame,
                     void * func, const char * funcname)
 {
     arch_store_frame(&frame->arch)
@@ -255,7 +274,7 @@ void __store_frame (struct pal_frame * frame,
 
 
 static inline
-void __clear_frame (struct pal_frame * frame)
+void __clear_frame (volatile struct pal_frame * frame)
 {
     if (frame->identifier == PAL_FRAME_IDENTIFIER) {
         asm volatile ("nop" ::: "memory");