diff --git a/changelogs/unreleased/gh-9222-sigaltstack.md b/changelogs/unreleased/gh-9222-sigaltstack.md new file mode 100644 index 0000000000000000000000000000000000000000..f17d280bd99daa96ea035b4b7ea28d9051e43946 --- /dev/null +++ b/changelogs/unreleased/gh-9222-sigaltstack.md @@ -0,0 +1,4 @@ +## bugfix/core + +* Fixed a crash that happened while printing the stack trace on a stack + overflow bug (gh-9222). diff --git a/src/lib/core/crash.c b/src/lib/core/crash.c index 6e324997c9c3ef8bec0daf6eec934146d9f93fa3..55825d3db522a624d399dc8c13db4740844db853 100644 --- a/src/lib/core/crash.c +++ b/src/lib/core/crash.c @@ -225,9 +225,12 @@ crash_signal_init(void) * * SA_NODEFER allows receiving the same signal * during handler. + * + * SA_ONSTACK is required to handle stack overflow, which + * makes the current stack unusable. */ struct sigaction sa = { - .sa_flags = SA_RESETHAND | SA_NODEFER | SA_SIGINFO, + .sa_flags = SA_RESETHAND | SA_NODEFER | SA_SIGINFO | SA_ONSTACK, .sa_sigaction = crash_signal_cb, }; sigemptyset(&sa.sa_mask); diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c index f6ff1aaa7bb8eadc98417200d43a8adb492227e5..1437036062d118a80479c3b89b990b38b751a233 100644 --- a/src/lib/core/fiber.c +++ b/src/lib/core/fiber.c @@ -223,6 +223,49 @@ fiber_mprotect(void *addr, size_t len, int prot) return -1; } +/** + * Sets an alternative signal stack. + * + * We use an alternative signal stack so that we can print the stack trace on + * stack overflow, which makes the current stack unusable. This function should + * be called in each thread because the SIGSEGV signal is thread-directed. + * At thread exit, one should call signal_stack_free() to free the stack set by + * this function. + * + * We skip this for ASAN because it installs its own signal stack. + */ +static void +signal_stack_init(void) +{ +#ifndef ENABLE_ASAN + stack_t stack; + stack.ss_flags = 0; + /* SIGSTKSZ doesn't seem to be enough for libunwind. */ + stack.ss_size = 4 * SIGSTKSZ; + stack.ss_sp = xmalloc(stack.ss_size); + if (sigaltstack(&stack, NULL) != 0) { + say_syserror("sigaltstack"); + free(stack.ss_sp); + } +#endif +} + +/** + * Frees the signal stack set with signal_stack_init(). + */ +static void +signal_stack_free(void) +{ +#ifndef ENABLE_ASAN + stack_t stack; + if (sigaltstack(NULL, &stack) != 0) { + say_syserror("sigaltstack"); + } else { + free(stack.ss_sp); + } +#endif +} + static __thread bool fiber_top_enabled = false; #ifdef ENABLE_BACKTRACE @@ -1730,6 +1773,7 @@ cord_create(struct cord *cord, const char *name) #ifdef HAVE_MADV_DONTNEED cord->sched.stack_watermark = NULL; #endif + signal_stack_init(); } void @@ -1759,6 +1803,7 @@ cord_exit(struct cord *cord) assert(cord == cord()); (void)cord; trigger_free_in_thread(); + signal_stack_free(); } void diff --git a/test/unit/guard.cc b/test/unit/guard.cc index 236d3180b8daf0ef4611229e4e70f9673152a97d..14a4afc6a1afe70ad3416cbef4d40cf38223adda 100644 --- a/test/unit/guard.cc +++ b/test/unit/guard.cc @@ -42,21 +42,6 @@ stack_break_f(char *frame_zero) static int main_f(va_list ap) { - stack_t stack; - stack.ss_flags = 0; - /* - * It is said that SIGSTKSZ is not enough for one of llvm sanitizers - * (probably asan, because this test fails with segmentation fault if - * we use SIGSTKSZ as alternative signal stack size when we use it). - * https://github.com/llvm/llvm-project/blame/699231ab3c7dd8f028d868b103481fa901f3c721/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp#L169 - */ - stack.ss_size = 4 * SIGSTKSZ; - stack.ss_sp = xmalloc(stack.ss_size); - if (sigaltstack(&stack, NULL) < 0) { - free(stack.ss_sp); - perror("sigaltstack"); - exit(EXIT_FAILURE); - } struct sigaction sa; sa.sa_handler = sigsegf_handler; sigemptyset(&sa.sa_mask); @@ -67,7 +52,6 @@ main_f(va_list ap) int res = stack_break_f((char *)__builtin_frame_address(0)); ev_break(loop(), EVBREAK_ALL); - free(stack.ss_sp); return res; }