diff --git a/LibOS/shim/src/shim_call.c b/LibOS/shim/src/shim_call.c index 855701e0b5..5866538313 100644 --- a/LibOS/shim/src/shim_call.c +++ b/LibOS/shim/src/shim_call.c @@ -49,6 +49,17 @@ static int run_test_asan_stack(void) { return 0; } + +/* Test: write past the end of a global (static local) buffer (ASan only) */ +__attribute__((no_sanitize("undefined"))) +static int run_test_asan_global(void) { + static char buf[30]; + char* c = buf + 30; + *c = 1; + + return 0; +} + #endif /* ASAN */ static const struct shim_test { @@ -62,6 +73,7 @@ static const struct shim_test { #ifdef ASAN { "asan_heap", &run_test_asan_heap }, { "asan_stack", &run_test_asan_stack }, + { "asan_global", &run_test_asan_global }, #endif { NULL, NULL }, }; diff --git a/LibOS/shim/src/shim_checkpoint.c b/LibOS/shim/src/shim_checkpoint.c index 229e111e22..9c163838ff 100644 --- a/LibOS/shim/src/shim_checkpoint.c +++ b/LibOS/shim/src/shim_checkpoint.c @@ -168,7 +168,10 @@ BEGIN_CP_FUNC(migratable) { size_t len = &__migratable_end - &__migratable[0]; size_t off = ADD_CP_OFFSET(len); - memcpy((char*)base + off, &__migratable[0], len); + /* Use `_real_memcpy` to bypass ASan: we are accessing the whole `.migratable` section, + * including the redzones after global variables, and using normal `memcpy` would cause ASan to + * complain. */ + _real_memcpy((char*)base + off, &__migratable[0], len); ADD_CP_FUNC_ENTRY(off); } END_CP_FUNC(migratable) @@ -178,7 +181,8 @@ BEGIN_RS_FUNC(migratable) { __UNUSED(rebase); const char* data = (char*)base + GET_CP_FUNC_ENTRY(); - memcpy(&__migratable[0], data, &__migratable_end - &__migratable[0]); + /* Use `_real_memcpy` to bypass ASan (same as above). */ + _real_memcpy(&__migratable[0], data, &__migratable_end - &__migratable[0]); } END_RS_FUNC(migratable) diff --git a/LibOS/shim/test/regression/test_libos.py b/LibOS/shim/test/regression/test_libos.py index 0e2b778392..2172f5ddfb 100644 --- a/LibOS/shim/test/regression/test_libos.py +++ b/LibOS/shim/test/regression/test_libos.py @@ -27,19 +27,21 @@ def test_010_gramine_run_test(self): 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_heap(self): - expected_list = ['asan: heap-buffer-overflow'] - if self.has_debug(): - expected_list.append('asan: location: run_test_asan_heap at shim_call.c') - self._test_abort('asan_heap', expected_list) + self._test_asan('heap', 'heap-buffer-overflow') - @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'] + self._test_asan('stack', 'stack-buffer-overflow') + + def test_023_asan_stack(self): + self._test_asan('global', 'global-buffer-overflow') + + @unittest.skipUnless(os.environ.get('ASAN') == '1', 'test only enabled with ASAN=1') + def _test_asan(self, case, desc): + expected_list = [f'asan: {desc}'] if self.has_debug(): - expected_list.append('asan: location: run_test_asan_stack at shim_call.c') - self._test_abort('asan_stack', expected_list) + expected_list.append(f'asan: location: run_test_asan_{case} at shim_call.c') + self._test_abort(f'asan_{case}', expected_list) def _test_abort(self, test_name, expected_list): try: diff --git a/Pal/src/host/Linux-SGX/sgx_main.c b/Pal/src/host/Linux-SGX/sgx_main.c index c80413ab2d..405c496a01 100644 --- a/Pal/src/host/Linux-SGX/sgx_main.c +++ b/Pal/src/host/Linux-SGX/sgx_main.c @@ -1074,6 +1074,16 @@ noreturn static void print_usage_and_exit(const char* argv_0) { } #ifdef ASAN +/* + * HACK: `setup_asan` is not called inside `main`, but defined as a constructor with a priority of + * 0, which is normally reserved. This is because we need to run it before module constructors + * generated by ASan itself (which have a priority of 1). + * + * Note that this is necessary only because we're in an executable linked against Glibc. In other + * cases, we invoke the constructors from `.init_array` directly, so we have full control over + * initialization. + */ +__attribute((constructor(0))) __attribute_no_sanitize_address static void setup_asan(void) { int prot = PROT_READ | PROT_WRITE; @@ -1096,10 +1106,6 @@ int main(int argc, char* argv[], char* envp[]) { bool need_gsgx = true; char* manifest = NULL; -#ifdef ASAN - setup_asan(); -#endif - #ifdef DEBUG ret = debug_map_init_from_proc_maps(); if (ret < 0) { diff --git a/common/include/asan.h b/common/include/asan.h index 8ea5e6044d..4e0ce95433 100644 --- a/common/include/asan.h +++ b/common/include/asan.h @@ -49,6 +49,10 @@ * unpoisoned before unmapping (in case ASan-unaware code uses this part of address space * later). * + * - Make sure to call the constructors in the `.init_array` section (see `init.h` for a helper + * function). This is necessary for global variables sanitization (see `__asan_register_globals` + * below). + * * - You should compile the program with: * * -fsanitize=address @@ -56,7 +60,6 @@ * -mllvm -asan-mapping-offset=0x18000000000 * -mllvm -asan-use-after-return=0 * -mllvm -asan-stack=0 - * -mllvm -asan-globals=0 * -DASAN * * If you want to enable stack sanitization (`-mllvm -asan-stack=1`), you need to also handle @@ -149,6 +152,7 @@ #define ASAN_POISON_STACK_AFTER_SCOPE 0xf8 #define ASAN_POISON_ALLOCA_LEFT 0xca #define ASAN_POISON_ALLOCA_RIGHT 0xcb +#define ASAN_POISON_GLOBAL 0xf9 #define ASAN_POISON_USER 0xf7 /* currently used for unallocated SGX memory */ /* Size of `alloca` redzone (hardcoded in LLVM: `kAllocaRzSize`); see `asan_alloca_poison` below. */ @@ -167,8 +171,8 @@ void asan_unpoison_region(uintptr_t addr, size_t size); * prints a warning instead. */ void asan_unpoison_current_stack(uintptr_t addr, size_t size); -/* Initialization callbacks. Generated in object .init sections. Graphene doesn't call these anyway, - * so this needs to be a no-op. */ +/* Initialization callbacks, called in `.init_array`. In Gramine, these do nothing, and we + * initialize ASan separately. */ void __asan_init(void); void __asan_version_mismatch_check_v8(void); @@ -227,6 +231,31 @@ void __asan_alloca_poison(uintptr_t addr, size_t size); /* Unpoison the stack area from `start` to `end`. Do nothing if `start` is zero. */ void __asan_allocas_unpoison(uintptr_t start, uintptr_t end); +/* + * Description of an instrumented global variable. See LLVM (`asan_interface_internal.h`) for + * original definition. + * + * Currently, we read only `addr`, `size`, and `size_with_redzone` fields. + */ +struct __asan_global { + uintptr_t addr; /* in LLVM: `beg` */ + size_t size; + size_t size_with_redzone; + const char* name; + const char* module_name; + size_t has_dynamic_init; + void* location; /* in LLVM: `__asan_global_source_location* location` */ + uintptr_t odr_indicator; +}; + +/* Register global variables. Called in an `.init_array` constructor generated by ASan. Our + * implementation poisons the right redzone (`size .. size_with_redzone`). */ +void __asan_register_globals(struct __asan_global* globals, size_t n); + +/* Unregister global variables. Called in a `.fini_array` destructor generated by ASan. Our + * implementation does nothing (and we don't even handle `.fini_array`). */ +void __asan_unregister_globals(struct __asan_global* globals, size_t n); + /* Callbacks for setting the shadow memory to specific values. As with load/store callbacks, LLVM * normally generates inline stores and calls these functions only for bigger areas. This is * controlled by `-mllvm -asan-max-inline-poisoning-size=N`. */ diff --git a/common/src/asan.c b/common/src/asan.c index 08dd4c0f77..561771750b 100644 --- a/common/src/asan.c +++ b/common/src/asan.c @@ -144,6 +144,9 @@ static void asan_find_problem(uintptr_t addr, size_t size, uintptr_t* out_bad_ad case ASAN_POISON_ALLOCA_RIGHT: bug_type = "dynamic-stack-buffer-overflow"; break; + case ASAN_POISON_GLOBAL: + bug_type = "global-buffer-overflow"; + break; default: bug_type = "unknown-crash"; break; @@ -196,6 +199,7 @@ static void asan_dump(uintptr_t bad_addr) { log_error("asan: %22s %02x", "stack after scope:", ASAN_POISON_STACK_AFTER_SCOPE); log_error("asan: %22s %02x", "alloca left redzone:", ASAN_POISON_ALLOCA_LEFT); log_error("asan: %22s %02x", "alloca right redzone:", ASAN_POISON_ALLOCA_RIGHT); + log_error("asan: %22s %02x", "global redzone:", ASAN_POISON_GLOBAL); log_error("asan: %22s %02x", "user-poisoned:", ASAN_POISON_USER); } @@ -343,6 +347,18 @@ void __asan_allocas_unpoison(uintptr_t start, uintptr_t end) { } } +void __asan_register_globals(struct __asan_global* globals, size_t n) { + for (size_t i = 0; i < n; i++) { + asan_poison_right_redzone(globals[i].addr, globals[i].size, globals[i].size_with_redzone, + ASAN_POISON_GLOBAL); + } +} + +void __asan_unregister_globals(struct __asan_global* globals, size_t n) { + __UNUSED(globals); + __UNUSED(n); +} + #define DEFINE_ASAN_SET_SHADOW(name, value) \ __attribute__((noinline)) \ void __asan_set_shadow_ ## name (uintptr_t addr, size_t size) { \ diff --git a/meson.build b/meson.build index effb68d297..9ef1c23fac 100644 --- a/meson.build +++ b/meson.build @@ -209,7 +209,6 @@ if asan '-fno-sanitize-link-runtime', '-mllvm', '-asan-mapping-offset=' + asan_shadow_start, '-mllvm', '-asan-use-after-return=0', - '-mllvm', '-asan-globals=0', '-DASAN', ] endif