[common,Pal,LibOS] ASan: Enable stack sanitization

The current implementation is able to catch stack buffer overflows and
use-after-scope bugs (but not use-after-return).

The implementation requires Gramine to maintain proper hygiene when
handling stack memory. Otherwise, false positives are possible: if
Gramine stops using a stack (e.g. when returning from a syscall), but
doesn't unpoison it, the poisoned stack memory can incorrectly trigger
ASan the next time that stack is used.

Signed-off-by: Paweł Marczewski <[email protected]>

LibOS/shim/include/arch/x86_64/shim_internal-arch.h

@@ -3,31 +3,18 @@
 #ifndef _SHIM_INTERNAL_ARCH_H_
 #define _SHIM_INTERNAL_ARCH_H_
 
-#define __SWITCH_STACK(stack_top, func, arg)                    \
-    do {                                                        \
-        /* 16 Bytes align of stack */                           \
-        uintptr_t __stack_top = (uintptr_t)(stack_top);         \
-        __stack_top &= ~0xf;                                    \
-        __stack_top -= 8;                                       \
-        __asm__ volatile (                                      \
-            "movq %0, %%rsp\n"                                  \
-            "xorq %%rbp, %%rbp\n"                               \
-            "jmpq *%%rcx\n"                                     \
-            :                                                   \
-            : "r"(__stack_top), "c"(func), "D"(arg)             \
-            : "memory");                                        \
-        __builtin_unreachable();                                \
-    } while (0)
-
-#define CALL_ELF_ENTRY(ENTRY, ARGP)      \
-    __asm__ volatile(                    \
-        "pushq $0\r\n"                   \
-        "popfq\r\n"                      \
-        "movq %%rbx, %%rsp\r\n"          \
-        "jmp *%%rax\r\n"                 \
-        :                                \
-        : "a"(ENTRY), "b"(ARGP), "d"(0)  \
-        : "memory", "cc")
+#define CALL_ELF_ENTRY(ENTRY, ARGP)           \
+    do {                                      \
+        __asm__ volatile(                     \
+            "pushq $0\r\n"                    \
+            "popfq\r\n"                       \
+            "movq %%rbx, %%rsp\r\n"           \
+            "jmp *%%rax\r\n"                  \
+            :                                 \
+            : "a"(ENTRY), "b"(ARGP), "d"(0)   \
+            : "memory", "cc");                \
+        __builtin_unreachable();              \
+    } while(0)
 
 #define SHIM_ELF_HOST_MACHINE EM_X86_64
 

LibOS/shim/include/shim_internal.h

@@ -53,8 +53,14 @@ noreturn void shim_emulate_syscall(PAL_CONTEXT* context);
  * it does not need to be reentrant (there is no such thing as nested syscalls), but it cannot
  * assume that the CPU context is the same as at the entry to the syscall (e.g. sigreturn, or signal
  * handling may change it).
+ *
+ * When AddressSanitizer is enabled, it also unpoisons the LibOS stack. Note at this point, we could
+ * also be running from the initial PAL stack, but we unconditionally unpoison the LibOS stack for
+ * simplicity.
  */
 noreturn void return_from_syscall(PAL_CONTEXT* context);
+/* Platform-specific part of return_from_syscall (called after ASan unpoisoning). */
+noreturn void _return_from_syscall(PAL_CONTEXT* context);
 /*!
  * \brief Restore the context after clone/fork.
  *

LibOS/shim/src/arch/x86_64/syscallas.S

@@ -209,9 +209,9 @@ Lmorestack_for_gdb_bt:
 .size __morestack, .-__morestack
 #endif
 
-.global return_from_syscall
-.type return_from_syscall, @function
-return_from_syscall:
+.global _return_from_syscall
+.type _return_from_syscall, @function
+_return_from_syscall:
     # expects one argument (in `rdi`) - pointer to PAL_CONTEXT
     .cfi_startproc
 
@@ -271,4 +271,4 @@ return_from_syscall:
     jmp .Lrestore_context
 
     .cfi_endproc
-.size return_from_syscall, .-return_from_syscall
+.size _return_from_syscall, .-_return_from_syscall

LibOS/shim/src/bookkeep/shim_thread.c

@@ -9,6 +9,7 @@
  */
 
 #include "api.h"
+#include "asan.h"
 #include "assert.h"
 #include "cpu.h"
 #include "list.h"
@@ -337,8 +338,11 @@ void put_thread(struct shim_thread* thread) {
         assert(LIST_EMPTY(thread, list));
 
         if (thread->libos_stack_bottom) {
-            void* tmp_vma = NULL;
             char* addr = (char*)thread->libos_stack_bottom - SHIM_THREAD_LIBOS_STACK_SIZE;
+#ifdef ASAN
+            asan_unpoison_region((uintptr_t)addr, SHIM_THREAD_LIBOS_STACK_SIZE);
+#endif
+            void* tmp_vma = NULL;
             if (bkeep_munmap(addr, SHIM_THREAD_LIBOS_STACK_SIZE, /*is_internal=*/true, &tmp_vma) < 0) {
                 log_error("[put_thread] Failed to remove bookkeeped memory at %p-%p!",
                           addr, (char*)addr + SHIM_THREAD_LIBOS_STACK_SIZE);

LibOS/shim/src/shim_call.c

@@ -29,15 +29,27 @@ static int run_test_ubsan_int_overflow(void) {
 }
 #endif
 
-/* Test: allocate a buffer on heap, write past the end of buffer (ASan only) */
 #ifdef ASAN
-static int run_test_asan_buffer_overflow(void) {
+/* Test: allocate a buffer on heap, write past the end of buffer (ASan only) */
+__attribute__((no_sanitize("undefined")))
+static int run_test_asan_heap(void) {
     uint8_t* buf = malloc(30);
     buf[30] = 1;
     free(buf);
     return 0;
 }
-#endif
+
+/* Test: write past the end of a stack buffer (ASan only) */
+__attribute__((no_sanitize("undefined")))
+static int run_test_asan_stack(void) {
+    char buf[30];
+    /* Take a pointer: direct assignment such as `buf[30] = 1;` triggers a compiler warning */
+    char* c = buf + 30;
+    *c = 1;
+
+    return 0;
+}
+#endif /* ASAN */
 
 static const struct shim_test {
     const char* name;
@@ -48,7 +60,8 @@ static const struct shim_test {
     { "ubsan_int_overflow", &run_test_ubsan_int_overflow },
 #endif
 #ifdef ASAN
-    { "asan_buffer_overflow", &run_test_asan_buffer_overflow },
+    { "asan_heap", &run_test_asan_heap },
+    { "asan_stack", &run_test_asan_stack },
 #endif
     { NULL, NULL },
 };

LibOS/shim/src/shim_rtld.c

@@ -21,6 +21,7 @@
 #include <endian.h>
 #include <errno.h>
 
+#include "asan.h"
 #include "elf.h"
 #include "shim_checkpoint.h"
 #include "shim_entry.h"
@@ -819,6 +820,24 @@ int register_library(const char* name, unsigned long load_address) {
     return 0;
 }
 
+/*
+ * Helper function for unpoisoning the stack before jump: we need to do it from a function that is
+ * not instrumented by ASan, so that the stack is guaranteed to stay unpoisoned.
+ *
+ * Note that at this point, we could also be running from the initial PAL stack. However, we
+ * unconditionally unpoison the LibOS stack for simplicity.
+ */
+__attribute_no_sanitize_address
+noreturn static void cleanup_and_call_elf_entry(ElfW(Addr) entry, void* argp) {
+#ifdef ASAN
+    uintptr_t libos_stack_bottom = (uintptr_t)SHIM_TCB_GET(libos_stack_bottom);
+    asan_unpoison_region(libos_stack_bottom - SHIM_THREAD_LIBOS_STACK_SIZE,
+                         SHIM_THREAD_LIBOS_STACK_SIZE);
+
+#endif
+    CALL_ELF_ENTRY(entry, argp);
+}
+
 noreturn void execute_elf_object(struct link_map* exec_map, void* argp, ElfW(auxv_t)* auxp) {
     if (exec_map) {
         /* If a new map is provided, it means we have cleared the existing one by calling
@@ -891,9 +910,7 @@ noreturn void execute_elf_object(struct link_map* exec_map, void* argp, ElfW(aux
 
     ElfW(Addr) entry = g_interp_map ? g_interp_map->l_entry : g_exec_map->l_entry;
 
-    CALL_ELF_ENTRY(entry, argp);
-
-    die_or_inf_loop();
+    cleanup_and_call_elf_entry(entry, argp);
 }
 
 BEGIN_CP_FUNC(elf_object) {

LibOS/shim/src/shim_syscalls.c

@@ -3,6 +3,7 @@
  *                    Borys Popławski <[email protected]>
  */
 
+#include "asan.h"
 #include "shim_defs.h"
 #include "shim_internal.h"
 #include "shim_table.h"
@@ -62,3 +63,13 @@ noreturn void shim_emulate_syscall(PAL_CONTEXT* context) {
 
     return_from_syscall(context);
 }
+
+__attribute_no_sanitize_address
+noreturn void return_from_syscall(PAL_CONTEXT* context) {
+#ifdef ASAN
+    uintptr_t libos_stack_bottom = (uintptr_t)SHIM_TCB_GET(libos_stack_bottom);
+    asan_unpoison_region(libos_stack_bottom - SHIM_THREAD_LIBOS_STACK_SIZE,
+                         SHIM_THREAD_LIBOS_STACK_SIZE);
+#endif
+    _return_from_syscall(context);
+}

LibOS/shim/src/sys/shim_exec.c

@@ -35,14 +35,9 @@ static int close_cloexec_handle(struct shim_handle_map* map) {
     return walk_handle_map(&close_on_exec, map);
 }
 
-struct execve_rtld_arg {
-    void* new_argp;       /* pointer to beginning of first stack frame (argc, argv[0], ...) */
-    elf_auxv_t* new_auxv; /* pointer inside first stack frame (auxv[0], auxv[1], ...) */
-};
-
-noreturn static void __shim_do_execve_rtld(struct execve_rtld_arg* __arg) {
-    struct execve_rtld_arg arg = *__arg;
-
+/* new_argp: pointer to beginning of first stack frame (argc, argv[0], ...)
+ * new_auxv: pointer inside first stack frame (auxv[0], auxv[1], ...) */
+noreturn static void __shim_do_execve_rtld(void* new_argp, elf_auxv_t* new_auxv) {
     int ret = 0;
 
     set_default_tls();
@@ -100,7 +95,7 @@ noreturn static void __shim_do_execve_rtld(struct execve_rtld_arg* __arg) {
     put_handle(exec);
 
     log_debug("execve: start execution");
-    execute_elf_object(exec_map, arg.new_argp, arg.new_auxv);
+    execute_elf_object(exec_map, new_argp, new_auxv);
     /* NOTREACHED */
 
 error:
@@ -139,11 +134,7 @@ static int shim_do_execve_rtld(struct shim_handle* hdl, const char** argv, const
     /* We are done using this handle and we got the ownership from the caller. */
     put_handle(hdl);
 
-    struct execve_rtld_arg arg = {
-        .new_argp = new_argp,
-        .new_auxv = new_auxv
-    };
-    __SWITCH_STACK(new_argp, &__shim_do_execve_rtld, &arg);
+    __shim_do_execve_rtld(new_argp, new_auxv);
     /* UNREACHABLE */
 }
 

LibOS/shim/src/vdso/arch/x86_64/vdso.c

@@ -11,6 +11,10 @@
 #include "vdso.h"
 #include "vdso_syscall.h"
 
+#ifdef ASAN
+#error This code should be compiled without AddressSanitizer.
+#endif
+
 /*
  * The symbol below needs to be exported for libsysdb to inject those values,
  * but relocation (.rela.dyn section) isn't wanted in the code generation.

LibOS/shim/src/vdso/arch/x86_64/vdso_syscall.h

@@ -7,6 +7,10 @@
 
 #include "gramine_entry_api.h"
 
+#ifdef ASAN
+#error This code should be compiled without AddressSanitizer.
+#endif
+
 static inline long vdso_arch_syscall(long nr, long arg1, long arg2) {
     long ret;
     __asm__ volatile(

LibOS/shim/test/regression/test_libos.py

@@ -28,11 +28,18 @@ def test_020_ubsan(self):
         self._test_abort('ubsan_int_overflow', ['ubsan: overflow'])
 
     @unittest.skipUnless(os.environ.get('ASAN') == '1', 'test only enabled with ASAN=1')
-    def test_021_asan(self):
+    def test_021_asan_heap(self):
         expected_list = ['asan: heap-buffer-overflow']
         if self.has_debug():
-            expected_list.append('asan: location: run_test_asan_buffer_overflow at shim_call.c')
-        self._test_abort('asan_buffer_overflow', expected_list)
+            expected_list.append('asan: location: run_test_asan_heap at shim_call.c')
+        self._test_abort('asan_heap', expected_list)
+
+    @unittest.skipUnless(os.environ.get('ASAN') == '1', 'test only enabled with ASAN=1')
+    def test_022_asan_stack(self):
+        expected_list = ['asan: stack-buffer-overflow']
+        if self.has_debug():
+            expected_list.append('asan: location: run_test_asan_stack at shim_call.c')
+        self._test_abort('asan_stack', expected_list)
 
     def _test_abort(self, test_name, expected_list):
         try:

Pal/src/db_rtld.c

@@ -625,6 +625,9 @@ void pal_describe_location(uintptr_t addr, char* buf, size_t buf_size) {
     default_describe_location(addr, buf, buf_size);
 }
 
+/* Disable AddressSanitizer for this function: it uses the `stack_entries` array as the beginning of
+ * new stack, so we don't want any redzone around it, or ASan-specific stack frame handling. */
+__attribute_no_sanitize_address
 noreturn void start_execution(const char** arguments, const char** environs) {
     /* Our PAL loader invokes LibOS entrypoint with the following stack:
      *

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

@@ -11,6 +11,7 @@
 #include <linux/signal.h>
 
 #include "api.h"
+#include "asan.h"
 #include "cpu.h"
 #include "ecall_types.h"
 #include "pal.h"
@@ -26,9 +27,18 @@
 
 /* Restore an sgx_cpu_context_t as generated by .Lhandle_exception. Execution will
  * continue as specified by the rip in the context. */
+__attribute_no_sanitize_address
 noreturn static void restore_sgx_context(sgx_cpu_context_t* uc, PAL_XREGS_STATE* xregs_state) {
     if (xregs_state == NULL)
         xregs_state = (PAL_XREGS_STATE*)g_xsave_reset_state;
+
+#ifdef ASAN
+    /* Unpoison the signal stack before leaving it */
+    uintptr_t sig_stack_low = GET_ENCLAVE_TLS(sig_stack_low);
+    uintptr_t sig_stack_high = GET_ENCLAVE_TLS(sig_stack_high);
+    asan_unpoison_current_stack(sig_stack_low, sig_stack_high - sig_stack_low);
+#endif
+
     _restore_sgx_context(uc, xregs_state);
 }
 

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

@@ -27,6 +27,7 @@
 #include <stdint.h>
 
 #include "api.h"
+#include "asan.h"
 #include "cpu.h"
 #include "ecall_types.h"
 #include "ocall_types.h"
@@ -140,6 +141,7 @@ static long sgx_exitless_ocall(uint64_t code, void* ms) {
     return READ_ONCE(req->result);
 }
 
+__attribute_no_sanitize_address
 noreturn void ocall_exit(int exitcode, int is_exitgroup) {
     ms_ocall_exit_t* ms;
 
@@ -153,6 +155,16 @@ noreturn void ocall_exit(int exitcode, int is_exitgroup) {
     WRITE_ONCE(ms->ms_exitcode, exitcode);
     WRITE_ONCE(ms->ms_is_exitgroup, is_exitgroup);
 
+#ifdef ASAN
+    /* Unpoison the stacks allocated for this thread. They can be later used for a new thread. */
+    uintptr_t initial_stack_addr = GET_ENCLAVE_TLS(initial_stack_addr);
+    asan_unpoison_region(initial_stack_addr - ENCLAVE_STACK_SIZE, ENCLAVE_STACK_SIZE);
+
+    uintptr_t sig_stack_low = GET_ENCLAVE_TLS(sig_stack_low);
+    uintptr_t sig_stack_high = GET_ENCLAVE_TLS(sig_stack_high);
+    asan_unpoison_region(sig_stack_low, sig_stack_high - sig_stack_low);
+#endif
+
     // 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