Fix a regression on exec in trying to repro #59. Remove an assertion that doesn't seem to be doing much good, given that the size is always statically determined in hex2str.

Fixing the bound of a debug buffer in SGX PAL

Rewrite bytes2hexstr; Add two new macros, alloca_bytes2hexstr() and malloc_bytes2hexstr().

Remove HASHBUF_SIZE

Add comments for the bytes2hexstr() macros.

Add a compile-time assertion

LibOS/shim/test/native/exec_victim.c

@@ -1,24 +0,0 @@
-/* -*- mode:c; c-file-style:"k&r"; c-basic-offset: 4; tab-width:4; indent-tabs-mode:nil; mode:auto-fill; fill-column:78; -*- */
-/* vim: set ts=4 sw=4 et tw=78 fo=cqt wm=0: */
-
-#include <stdio.h>
-#include <stdlib.h>
-
-int main(int argc, char ** argv, const char ** envp)
-{
-    FILE * out = stdout;
-
-    if (argc > 1) {
-        int fd = atoi(argv[argc - 1]);
-        printf("inherited file descriptor %d\n", fd);
-        out = fdopen(fd, "a");
-        if (!out) {
-            perror("fdopen");
-            exit(1);
-        }
-    }
-
-    fprintf(out, "Hello World (%s)!\n", argv[0]);
-    fprintf(out, "envp[\'IN_EXECVE\'] = %s\n", getenv("IN_EXECVE"));
-    return 0;
-}

LibOS/shim/test/regression/exec_victim.c

@@ -0,0 +1,41 @@
+/* -*- mode:c; c-file-style:"k&r"; c-basic-offset: 4; tab-width:4; indent-tabs-mode:nil; mode:auto-fill; fill-column:78; -*- */
+/* vim: set ts=4 sw=4 et tw=78 fo=cqt wm=0: */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+int main(int argc, char ** argv, const char ** envp)
+{
+    FILE * out = stdout;
+
+    if (argc > 1) {
+        int fd = atoi(argv[argc - 1]);
+        printf("inherited file descriptor %d\n", fd);
+        out = fdopen(fd, "a");
+        if (!out) {
+            perror("fdopen");
+            exit(1);
+        }
+    }
+
+    fprintf(out, "Hello World (%s)!\n", argv[0]);
+    fprintf(out, "envp[\'IN_EXECVE\'] = %s\n", getenv("IN_EXECVE"));
+    fprintf(out, "00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
+000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
+000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
+000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
+000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
+000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
+000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
+000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
+000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
+000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
+000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
+000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
+000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
+000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
+000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
+000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 \
+\n");    
+    return 0;
+}

LibOS/shim/test/native/exec_victim.manifest.template → LibOS/shim/test/regression/exec_victim.manifest.template

@@ -1,6 +1,6 @@
 loader.preload = file:$(SHIMPATH)
 loader.env.LD_LIBRARY_PATH = /lib
-loader.debug_type = inline
+loader.debug_type = none
 loader.syscall_symbol = syscalldb
 
 fs.mount.lib.type = chroot

LibOS/shim/test/regression/manifest.template

@@ -18,3 +18,5 @@ net.rules.2 = 0.0.0.0:0-65535:127.0.0.1:8000
 
 sgx.trusted_files.ld = file:../../../../Runtime/ld-linux-x86-64.so.2
 sgx.trusted_files.libc = file:../../../../Runtime/libc.so.6
+sgx.trusted_files.victim = file:exec_victim
+sgx.trusted_children.victim = file:exec_victim.sig

Pal/lib/assert.h

@@ -3,17 +3,19 @@
 
 /*
  * assert.h
- * 
- * Define a common interface for assertions that builds for both the PAL 
+ *
+ * Define a common interface for assertions that builds for both the PAL
  * and libOS.
- * 
+ *
  */
 
 #ifndef ASSERT_H
 #define ASSERT_H
 
+#define COMPILE_TIME_ASSERT(pred) switch(0){case 0:case pred:;}
+
 /* All environments should implement warn, which prints a non-optional debug
- * message. All environments should also implement __abort, which 
+ * message. All environments should also implement __abort, which
  * terminates the process.
  */
 

Pal/lib/hex.h

@@ -21,13 +21,13 @@
 #ifndef HEX_H
 #define HEX_H
 
-/* This function is a helper for debug printing. 
- * It accepts a pointer to a numerical value, and 
+/* This function is a helper for debug printing.
+ * It accepts a pointer to a numerical value, and
  * formats it as a hex string, for printing.
  * size is the number of bytes pointed to by hex.
  * str is the caller-provided buffer, len is the length of the buffer.
  * The len must be at least (size * 2)+1.
- * 
+ *
  * Note that it does not normalize for endianness, and pads to the
  * size the compiler things the string is.
  */
@@ -51,7 +51,21 @@ char * __bytes2hexstr(void * hex, size_t size, char *str, size_t len)
 #define IS_INDEXABLE(arg) (sizeof((arg)[0]))
 #define IS_ARRAY(arg) (IS_INDEXABLE(arg) > 0 && (((void *) &(arg)) == ((void *) (arg))))
 
-#define bytes2hexstr(array, str, len) (IS_ARRAY(array) ?                \
-                                       __bytes2hexstr((array), sizeof(array), str, len) \
-                                       : NULL)
+
+/*
+ * bytes2hexstr converts an array into a hexadecimal string and fills into a
+ * given buffer. The buffer size is given as an extra argument.
+ */
+#define bytes2hexstr(array, str, len) ({             \
+            COMPILE_TIME_ASSERT(IS_ARRAY(array));    \
+            __bytes2hexstr((array), sizeof(array), str, len);})
+
+/*
+ * alloca_bytes2hexstr uses __alloca to allocate a buffer on the current frame
+ * and then fills the hexadecimal string into the buffer.
+ * This buffer can only be used within the caller frame (function).
+ */
+#define alloca_bytes2hexstr(array) \
+    (bytes2hexstr((array), __alloca(sizeof(array) * 2 + 1), sizeof(array) * 2 + 1))
+
 #endif // HEX_H

Pal/regression/Hex.c

@@ -9,8 +9,7 @@
 int main() {
     char x[] = {0xde, 0xad, 0xbe, 0xef};
     char y[] = {0xcd, 0xcd, 0xcd, 0xcd, 0xcd, 0xcd, 0xcd, 0xcd};
-    char buf[(sizeof(y) * 2) + 1];
-    pal_printf("Hex test 1 is %s\n", bytes2hexstr(x, buf, 17));
-    pal_printf("Hex test 2 is %s\n", bytes2hexstr(y, buf, 17));
+    pal_printf("Hex test 1 is %s\n", alloca_bytes2hexstr(x));
+    pal_printf("Hex test 2 is %s\n", alloca_bytes2hexstr(y));
     return 0;
 }

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

@@ -236,8 +236,8 @@ int _DkProcessCreate (PAL_HANDLE * handle, const char * uri,
                 sizeof(pal_enclave_state.enclave_keyhash),
                 data.keyhash_mac, sizeof data.keyhash_mac);
 
-    char mac_buf[MACBUF_SIZE];
-    SGX_DBG(DBG_P|DBG_S, "Attestation data: %s\n", bytes2hexstr(data.keyhash_mac, mac_buf, MACBUF_SIZE));
+    SGX_DBG(DBG_P|DBG_S, "Attestation data: %s\n",
+            alloca_bytes2hexstr(data.keyhash_mac));
 
     ret = _DkStreamAttestationRequest(proc, &data,
                                       &check_child_mrenclave, &param);
@@ -306,8 +306,8 @@ int init_child_process (PAL_HANDLE * parent_handle)
                 sizeof(pal_enclave_state.enclave_keyhash),
                 data.keyhash_mac, sizeof data.keyhash_mac);
 
-    char mac_buf[MACBUF_SIZE];
-    SGX_DBG(DBG_P|DBG_S, "Attestation data: %s\n", bytes2hexstr(data.keyhash_mac, mac_buf, MACBUF_SIZE));
+    SGX_DBG(DBG_P|DBG_S, "Attestation data: %s\n",
+            alloca_bytes2hexstr(data.keyhash_mac));
 
     ret = _DkStreamAttestationRespond(parent, &data,
                                       &check_parent_mrenclave,

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

@@ -36,7 +36,6 @@ void sgx_ocfree (void)
     SET_ENCLAVE_TLS(ustack, GET_ENCLAVE_TLS(ustack_top));
 }
 
-#define HASHBUF_SIZE ((sizeof(sgx_arch_hash_t)*2)+1)
 int sgx_get_report (sgx_arch_hash_t * mrenclave,
                     sgx_arch_attributes_t * attributes,
                     void * enclave_data,
@@ -56,21 +55,18 @@ int sgx_get_report (sgx_arch_hash_t * mrenclave,
     if (ret)
         return -PAL_ERROR_DENIED;
 
-    char hash_buf[HASHBUF_SIZE];
-    char mac_buf[MACBUF_SIZE];
-
     SGX_DBG(DBG_S, "Generated report:\n");
     SGX_DBG(DBG_S, "    cpusvn:           %08x %08x\n", report->cpusvn[0],
                                                 report->cpusvn[1]);
-    SGX_DBG(DBG_S, "    mrenclave:        %s\n",        bytes2hexstr(report->mrenclave, hash_buf, HASHBUF_SIZE));
-    SGX_DBG(DBG_S, "    mrsigner:         %s\n",        bytes2hexstr(report->mrsigner, hash_buf, HASHBUF_SIZE));
+    SGX_DBG(DBG_S, "    mrenclave:        %s\n",        alloca_bytes2hexstr(report->mrenclave));
+    SGX_DBG(DBG_S, "    mrsigner:         %s\n",        alloca_bytes2hexstr(report->mrsigner));
     SGX_DBG(DBG_S, "    attributes.flags: %016lx\n",    report->attributes.flags);
     SGX_DBG(DBG_S, "    sttributes.xfrm:  %016lx\n",    report->attributes.xfrm);
 
     SGX_DBG(DBG_S, "    isvprodid:        %02x\n",      report->isvprodid);
     SGX_DBG(DBG_S, "    isvsvn:           %02x\n",      report->isvsvn);
-    SGX_DBG(DBG_S, "    keyid:            %s\n",        bytes2hexstr(report->keyid, hash_buf, HASHBUF_SIZE));
-    SGX_DBG(DBG_S, "    mac:              %s\n",        bytes2hexstr(report->mac, mac_buf, MACBUF_SIZE));
+    SGX_DBG(DBG_S, "    keyid:            %s\n",        alloca_bytes2hexstr(report->keyid));
+    SGX_DBG(DBG_S, "    mac:              %s\n",        alloca_bytes2hexstr(report->mac));
 
     return 0;
 }
@@ -91,9 +87,9 @@ int sgx_verify_report (sgx_arch_report_t * report)
         return -PAL_ERROR_DENIED;
     }
 
-    char key_buf[KEYBUF_SIZE];
+    SGX_DBG(DBG_S, "Get report key for verification: %s\n",
+            alloca_bytes2hexstr(enclave_key));
 
-    SGX_DBG(DBG_S, "Get report key for verification: %s\n", bytes2hexstr(enclave_key, key_buf, KEYBUF_SIZE));
     return 0;
 }
 
@@ -109,8 +105,7 @@ int init_enclave_key (void)
         return -PAL_ERROR_DENIED;
     }
 
-    char key_buf[KEYBUF_SIZE];
-    SGX_DBG(DBG_S, "Get sealing key: %s\n", bytes2hexstr(enclave_key, key_buf, KEYBUF_SIZE));
+    SGX_DBG(DBG_S, "Get sealing key: %s\n", alloca_bytes2hexstr(enclave_key));
     return 0;
 }
 
@@ -840,10 +835,10 @@ void test_dh (void)
     FreeDhKey(&key1);
     FreeDhKey(&key2);
 
-    SGX_DBG(DBG_S, "key exchange(side A): %s (%d)\n", __bytes2hexstr(agree1, agreesz1, scratch, (agreesz1 * 2) + 1),
-           agreesz1);
-    SGX_DBG(DBG_S, "key exchange(side B): %s (%d)\n", __bytes2hexstr(agree2, agreesz2, scratch, (agreesz2 * 2) + 1),
-           agreesz2);
+    SGX_DBG(DBG_S, "key exchange(side A): %s\n",
+            __bytes2hexstr(agree1, agreesz1, scratch, agreesz1 * 2 + 1));
+    SGX_DBG(DBG_S, "key exchange(side B): %s\n",
+            __bytes2hexstr(agree2, agreesz2, scratch, agreesz2 * 2 + 1));
 }
 #endif
 
@@ -887,9 +882,8 @@ int init_enclave (void)
 
     pal_enclave_config.enclave_key = rsa;
 
-    char hash_buf[HASHBUF_SIZE];
     SGX_DBG(DBG_S, "enclave (software) key hash: %s\n",
-            bytes2hexstr(pal_enclave_state.enclave_keyhash, hash_buf, HASHBUF_SIZE));
+            alloca_bytes2hexstr(pal_enclave_state.enclave_keyhash));
 
     return 0;
 
@@ -901,7 +895,8 @@ out_free:
 
 int _DkStreamKeyExchange (PAL_HANDLE stream, PAL_SESSION_KEY * keyptr)
 {
-    unsigned char session_key[32] __attribute__((aligned(32)));
+    uint8_t session_key[sizeof(PAL_SESSION_KEY)]
+        __attribute__((aligned(sizeof(PAL_SESSION_KEY))));
     uint8_t pub[DH_SIZE]   __attribute__((aligned(DH_SIZE)));
     uint8_t agree[DH_SIZE] __attribute__((aligned(DH_SIZE)));
     PAL_NUM pubsz, agreesz;
@@ -956,9 +951,8 @@ int _DkStreamKeyExchange (PAL_HANDLE stream, PAL_SESSION_KEY * keyptr)
     for (int i = 0 ; i < agreesz ; i++)
         session_key[i % sizeof(session_key)] ^= agree[i];
 
-    char key_buf[KEYBUF_SIZE];
     SGX_DBG(DBG_S, "key exchange: (%p) %s\n", session_key,
-            bytes2hexstr(session_key, key_buf, KEYBUF_SIZE));
+            alloca_bytes2hexstr(session_key));
 
     if (keyptr)
         memcpy(keyptr, session_key, sizeof(PAL_SESSION_KEY));
@@ -994,9 +988,8 @@ int _DkStreamAttestationRequest (PAL_HANDLE stream, void * data,
     memcpy(&req.attributes, &pal_sec.enclave_attributes,
            sizeof(sgx_arch_attributes_t));
 
-    char hash_buf[HASHBUF_SIZE];
     SGX_DBG(DBG_S, "Sending attestation request ... (mrenclave = %s)\n",\
-            bytes2hexstr(req.mrenclave, hash_buf, HASHBUF_SIZE));
+            alloca_bytes2hexstr(req.mrenclave));
 
     for (bytes = 0, ret = 0 ; bytes < sizeof(req) ; bytes += ret) {
         ret = _DkStreamWrite(stream, 0, sizeof(req) - bytes,
@@ -1017,7 +1010,7 @@ int _DkStreamAttestationRequest (PAL_HANDLE stream, void * data,
     }
 
     SGX_DBG(DBG_S, "Received attestation (mrenclave = %s)\n",
-            bytes2hexstr(att.mrenclave, hash_buf, HASHBUF_SIZE));
+            alloca_bytes2hexstr(att.mrenclave));
 
     ret = sgx_verify_report(&att.report);
     if (ret < 0) {
@@ -1040,7 +1033,7 @@ int _DkStreamAttestationRequest (PAL_HANDLE stream, void * data,
 
     if (ret == 1) {
         SGX_DBG(DBG_S, "Not an allowed encalve (mrenclave = %s)\n",
-                bytes2hexstr(att.mrenclave, hash_buf, HASHBUF_SIZE));
+                alloca_bytes2hexstr(att.mrenclave));
         ret = -PAL_ERROR_DENIED;
         goto out;
     }
@@ -1058,7 +1051,7 @@ int _DkStreamAttestationRequest (PAL_HANDLE stream, void * data,
            sizeof(sgx_arch_attributes_t));
 
     SGX_DBG(DBG_S, "Sending attestation ... (mrenclave = %s)\n",
-            bytes2hexstr(att.mrenclave, hash_buf, HASHBUF_SIZE));
+            alloca_bytes2hexstr(att.mrenclave));
 
     for (bytes = 0, ret = 0 ; bytes < sizeof(att) ; bytes += ret) {
         ret = _DkStreamWrite(stream, 0, sizeof(att) - bytes,
@@ -1094,9 +1087,8 @@ int _DkStreamAttestationRespond (PAL_HANDLE stream, void * data,
         }
     }
 
-    char hash_buf[HASHBUF_SIZE];
     SGX_DBG(DBG_S, "Received attestation request ... (mrenclave = %s)\n",
-            bytes2hexstr(req.mrenclave, hash_buf, HASHBUF_SIZE));
+            alloca_bytes2hexstr(req.mrenclave));
 
     ret = sgx_get_report(&req.mrenclave, &req.attributes, data, &att.report);
     if (ret < 0) {
@@ -1109,7 +1101,7 @@ int _DkStreamAttestationRespond (PAL_HANDLE stream, void * data,
            sizeof(sgx_arch_attributes_t));
 
     SGX_DBG(DBG_S, "Sending attestation ... (mrenclave = %s)\n",
-            bytes2hexstr(att.mrenclave, hash_buf, HASHBUF_SIZE));
+            alloca_bytes2hexstr(att.mrenclave));
 
     for (bytes = 0, ret = 0 ; bytes < sizeof(att) ; bytes += ret) {
         ret = _DkStreamWrite(stream, 0, sizeof(att) - bytes,
@@ -1130,7 +1122,7 @@ int _DkStreamAttestationRespond (PAL_HANDLE stream, void * data,
     }
 
     SGX_DBG(DBG_S, "Received attestation (mrenclave = %s)\n",
-            bytes2hexstr(att.mrenclave, hash_buf, HASHBUF_SIZE));
+            alloca_bytes2hexstr(att.mrenclave));
 
     ret = sgx_verify_report(&att.report);
     if (ret < 0) {
@@ -1152,7 +1144,7 @@ int _DkStreamAttestationRespond (PAL_HANDLE stream, void * data,
 
     if (ret == 1) {
         SGX_DBG(DBG_S, "Not an allowed enclave (mrenclave = %s)\n",
-                bytes2hexstr(att.mrenclave, hash_buf, HASHBUF_SIZE));
+                alloca_bytes2hexstr(att.mrenclave));
         ret = -PAL_ERROR_DENIED;
         goto out;
     }