diff --git a/compiler-rt/lib/asan/asan_rtl.cpp b/compiler-rt/lib/asan/asan_rtl.cpp --- a/compiler-rt/lib/asan/asan_rtl.cpp +++ b/compiler-rt/lib/asan/asan_rtl.cpp @@ -551,7 +551,31 @@ static AsanInitializer asan_initializer; #endif // ASAN_DYNAMIC -} // namespace __asan +// Tries to find stack bounds of default stack. +// Returns false IFF stack boundaries could not be determined. +// local_stack: address of a variable on the stack, used to find stack bottom +static bool get_default_stack_bounds(uptr local_stack, AsanThread *curr_thread, + /*out*/ uptr *top, /*out*/ uptr *bottom) { + if (curr_thread) { + uptr PageSize = GetPageSizeCached(); + *top = curr_thread->stack_top(); + *bottom = (local_stack - PageSize) & ~(PageSize - 1); + return true; + } else if (SANITIZER_RTEMS) { + // Give up On RTEMS. + return false; + } else { + CHECK(!SANITIZER_FUCHSIA); + // If we haven't seen this thread, try asking the OS for stack bounds. + uptr tls_addr, tls_size, stack_size; + GetThreadStackAndTls(/*main=*/false, bottom, &stack_size, &tls_addr, + &tls_size); + *top = *bottom + stack_size; + return true; + } +} + +} // namespace __asan // ---------------------- Interface ---------------- {{{1 using namespace __asan; @@ -561,35 +585,55 @@ return; int local_stack; - AsanThread *curr_thread = GetCurrentThread(); - uptr PageSize = GetPageSizeCached(); + uptr top, bottom; - if (curr_thread) { - top = curr_thread->stack_top(); - bottom = ((uptr)&local_stack - PageSize) & ~(PageSize - 1); - } else if (SANITIZER_RTEMS) { - // Give up On RTEMS. - return; + AsanThread *curr_thread = GetCurrentThread(); + + AlternateStackInfo const signal_stack = GetSignalAlternateStack(); + if (0 == signal_stack.error) { + if (signal_stack.on_stack) { + bottom = signal_stack.bottom; + top = signal_stack.top; + } else { + if (!get_default_stack_bounds((uptr)&local_stack, curr_thread, &top, + &bottom)) + return; + } } else { - CHECK(!SANITIZER_FUCHSIA); - // If we haven't seen this thread, try asking the OS for stack bounds. - uptr tls_addr, tls_size, stack_size; - GetThreadStackAndTls(/*main=*/false, &bottom, &stack_size, &tls_addr, - &tls_size); - top = bottom + stack_size; + // This should not happen according to the error list of sigaltstack. + + static bool reported_warning = false; + if (!reported_warning) { + reported_warning = true; + Report( + "WARNING: ASan might fail to perform requested " + "__asan_handle_no_return correctly: " + "error code %d from sigaltstack\n" + "False positive error reports may follow\n", + signal_stack.error); + } + + // We don't know if we're on the alternate stack, + // but we can't get any information on it anyway. + // So we assume we're on the default stack. + if (!get_default_stack_bounds((uptr)&local_stack, curr_thread, &top, + &bottom)) + return; } + static const uptr kMaxExpectedCleanupSize = 64 << 20; // 64M if (top - bottom > kMaxExpectedCleanupSize) { static bool reported_warning = false; if (reported_warning) return; reported_warning = true; - Report("WARNING: ASan is ignoring requested __asan_handle_no_return: " - "stack top: %p; bottom %p; size: %p (%zd)\n" - "False positive error reports may follow\n" - "For details see " - "https://github.com/google/sanitizers/issues/189\n", - top, bottom, top - bottom, top - bottom); + Report( + "WARNING: ASan is ignoring requested __asan_handle_no_return: " + "stack top: %p; bottom %p; size: %p (%zd)\n" + "False positive error reports may follow\n" + "For details see " + "https://github.com/google/sanitizers/issues/189\n", + top, bottom, top - bottom, top - bottom); return; } PoisonShadow(bottom, top - bottom, 0); diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common.h b/compiler-rt/lib/sanitizer_common/sanitizer_common.h --- a/compiler-rt/lib/sanitizer_common/sanitizer_common.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_common.h @@ -317,6 +317,18 @@ // Alternative signal stack (POSIX-only). void SetAlternateSignalStack(); void UnsetAlternateSignalStack(); +// Returns true IFF this thread is running on the signal alternate stack. +// Writes the signal alternate stack top and bottom into *stack_top and +// *stack_bottom. These values are only meaningful if a signal alternate stack +// has been installed. +struct AlternateStackInfo { + // The values are only meaningful if error == 0 + uptr bottom; + uptr top; + int error; // 0 == success, other codes are platform-dependent + bool on_stack; +}; +AlternateStackInfo GetSignalAlternateStack(); // We don't want a summary too long. const int kMaxSummaryLength = 1024; diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp --- a/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp @@ -196,9 +196,24 @@ UnmapOrDie(oldstack.ss_sp, oldstack.ss_size); } -static void MaybeInstallSigaction(int signum, - SignalHandlerType handler) { - if (GetHandleSignalMode(signum) == kHandleSignalNo) return; +AlternateStackInfo GetSignalAlternateStack() { + stack_t signal_stack; + if (0 == sigaltstack(nullptr, &signal_stack)) { + if (signal_stack.ss_flags == SS_ONSTACK) { + uptr const bottom = (uptr)signal_stack.ss_sp; + uptr const top = + (uptr)((char *)signal_stack.ss_sp + signal_stack.ss_size); + return {bottom, top, 0, true}; + } else { + return {0, 0, 0, false}; + } + } else { + return {0, 0, errno, false}; + } +} +static void MaybeInstallSigaction(int signum, SignalHandlerType handler) { + if (GetHandleSignalMode(signum) == kHandleSignalNo) + return; struct sigaction sigact; internal_memset(&sigact, 0, sizeof(sigact)); diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_win.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_win.cpp --- a/compiler-rt/lib/sanitizer_common/sanitizer_win.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_win.cpp @@ -860,6 +860,8 @@ // FIXME: Decide what to do on Windows. } +AlternateStackInfo GetSignalAlternateStack() { return {0, 0, 0, false}; } + void InstallDeadlySignalHandlers(SignalHandlerType handler) { (void)handler; // FIXME: Decide what to do on Windows. diff --git a/compiler-rt/test/asan/TestCases/unpoison-alternate-stack.cpp b/compiler-rt/test/asan/TestCases/unpoison-alternate-stack.cpp new file mode 100644 --- /dev/null +++ b/compiler-rt/test/asan/TestCases/unpoison-alternate-stack.cpp @@ -0,0 +1,161 @@ +// Tests that __asan_handle_no_return properly unpoisons the signal alternate +// stack. + +// Don't optimize, otherwise the variables which create redzones might be +// dropped. +// RUN: %clangxx_asan -fexceptions -O0 %s -o %t -pthread +// RUN: %run %t + +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +#include + +namespace { + +void error(int err) { + std::puts(std::strerror(err)); + std::abort(); +} + +void assert_success(int err) { + if (err) + error(err); +} + +void assert_errno(bool failed) { + if (failed) + error(errno); +} + +char *left_redzone; +char *right_redzone; + +// Create redzones for stack variables in shadow memory and throw, +// which should unpoison the entire stack. +void __attribute__((noinline)) prepare_stack_shadow() { + char blob[100]; // This variable must not be optimized out, because we use it + // to create redzones. + + left_redzone = blob - 1; + right_redzone = blob + sizeof(blob); + + assert(__asan_address_is_poisoned(left_redzone)); + assert(__asan_address_is_poisoned(right_redzone)); + + // Trigger stack unwinding, which avoids normal cleanup of redzone markers. + // Instead, __asan_handle_no_return is called which unpoisons the entire + // stack. + throw "up"; +} + +// Ensure that the redzones created by prepare_stack_shadow have been removed. +void __attribute__((noinline)) assert_no_redzones() { + assert(0 == __asan_region_is_poisoned(left_redzone, + right_redzone - left_redzone)); +} + +// - Create redzones for a stack variable +// - Unwind the stack +// - Ensure that the redzones have been removed +void test_handle_no_return() { + try { + prepare_stack_shadow(); + } catch (...) { + assert_no_redzones(); + } +} + +// Signal handler which must be run on signal alternate stack, +// and tests if stack unwinding properly triggers stack unpoisoning. +void signal_handler(int, siginfo_t *, void *) { + stack_t stack; + assert_errno(sigaltstack(nullptr, &stack)); + assert(stack.ss_flags == SS_ONSTACK); + + // second test on alt stack (actual check) + test_handle_no_return(); +} + +// Main test function. +// Must be run on another thread to be able to control memory placement between +// default stack and alternate signal stack. +// If the alternate signal stack is placed in close proximity before the +// default stack, __asan_handle_no_return might unpoison both, even without +// being aware of the signal alternate stack. +// We want to test reliable that __asan_handle_no_return can properly unpoison +// the signal alternate stack. +void *thread_fun(void *alt_stack) { + // first test on default stack (sanity check) + test_handle_no_return(); + + assert_errno(sigaltstack((stack_t const *)alt_stack, nullptr)); + + struct sigaction const action = { + .sa_sigaction = signal_handler, + .sa_mask = {0}, + .sa_flags = SA_SIGINFO | SA_NODEFER | SA_ONSTACK, + .sa_restorer = nullptr}; + assert_errno(sigaction(SIGSEGV, &action, nullptr)); + + if (0 != raise(SIGSEGV)) + assert(false && "failed to raise SIGSEGV"); + + return nullptr; +} + +} // namespace + +// Check that __asan_handle_no_return properly unpoisons a signal alternate +// stack. +// __asan_handle_no_return tries to determine the stack boundaries and +// unpoisons all memory inside those. +// If the determination of stack boundaries does not work properly on the +// signal alternate stack, then the unpoisoning might be omitted. +// This can leave redzones for variables on the signal alternate stack, which +// can lead to false positive reports when the stack is reused. +int main() { + size_t const page_size = sysconf(_SC_PAGESIZE); + // To align the alternate stack, we round this up to page_size. + size_t const default_stack_size = + (PTHREAD_STACK_MIN - 1 + page_size) & ~(page_size - 1); + // The alternate stack needs a certain size, or the signal handler segfaults. + size_t const alt_stack_size = 10 * page_size; + size_t const mapping_size = default_stack_size + alt_stack_size; + // Using mmap guarantees proper alignment. + void *const mapping = mmap(nullptr, mapping_size, + PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, + 0, 0); + if ((void *)-1 == mapping) + error(errno); + + stack_t const alt_stack = { + .ss_sp = (char *)mapping + default_stack_size, + .ss_flags = 0, + .ss_size = alt_stack_size}; + + pthread_t thread; + pthread_attr_t thread_attr; + assert_success(pthread_attr_init(&thread_attr)); + assert_success(pthread_attr_setstack(&thread_attr, mapping, + default_stack_size)); + assert_success(pthread_create(&thread, &thread_attr, &thread_fun, + (void *)&alt_stack)); + + if (int const err = pthread_join(thread, nullptr)) + if (EINVAL != err) // EINVAL means the other thread exited first + error(err); + + assert_errno(munmap(mapping, mapping_size)); +}