From 825e43e485b9517c2dd9d23a4487aa467787903b Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Tue, 14 Oct 2025 04:22:10 +0000 Subject: [PATCH 1/4] Reapply "[sanitizer] Add cloak_sanitizer_signal_handlers runtime option" (#163308) This reverts commit 27d8441f8282c740903529d8a6b73401fc6c17fa. This reland uses 'raise(SIGSEGV)' instead of trying to get a segfault by dereferencing *123. The latter caused buildbot failures for cloak_{sigaction,signal}.cpp when assertions are enabled, because e.g., TSan will assert that 123 is not a valid app memory address, preventing the segfault from being triggered. While it is conceivable that a carefully chosen memory address will trigger a segfault, it is cleaner to directly raise the signal. --- .../lib/sanitizer_common/sanitizer_common.h | 3 ++ .../lib/sanitizer_common/sanitizer_flags.inc | 5 ++ .../sanitizer_posix_libcdep.cpp | 19 +++++++ .../sanitizer_signal_interceptors.inc | 42 ++++++++++++++- .../TestCases/Linux/allow_user_segv.cpp | 4 ++ .../TestCases/Linux/cloak_sigaction.cpp | 51 +++++++++++++++++++ .../TestCases/Linux/cloak_signal.cpp | 46 +++++++++++++++++ 7 files changed, 168 insertions(+), 2 deletions(-) create mode 100644 compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_sigaction.cpp create mode 100644 compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_signal.cpp diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common.h b/compiler-rt/lib/sanitizer_common/sanitizer_common.h index 3e82df498572c..ba85a0eb5a35e 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_common.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_common.h @@ -390,6 +390,9 @@ void ReportDeadlySignal(const SignalContext &sig, u32 tid, void SetAlternateSignalStack(); void UnsetAlternateSignalStack(); +bool IsSignalHandlerFromSanitizer(int signum); +bool SetSignalHandlerFromSanitizer(int signum, bool new_state); + // Construct a one-line string: // SUMMARY: SanitizerToolName: error_message // and pass it to __sanitizer_report_error_summary. diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_flags.inc b/compiler-rt/lib/sanitizer_common/sanitizer_flags.inc index 650a4580bbcf0..5f449907f6011 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_flags.inc +++ b/compiler-rt/lib/sanitizer_common/sanitizer_flags.inc @@ -113,6 +113,11 @@ COMMON_FLAG(HandleSignalMode, handle_sigfpe, kHandleSignalYes, COMMON_FLAG(bool, allow_user_segv_handler, true, "Deprecated. True has no effect, use handle_sigbus=1. If false, " "handle_*=1 will be upgraded to handle_*=2.") +COMMON_FLAG(bool, cloak_sanitizer_signal_handlers, false, + "If set, signal/sigaction will pretend that sanitizers did not " + "preinstall any signal handlers. If the user subsequently installs " + "a signal handler, this will disable cloaking for the respective " + "signal.") COMMON_FLAG(bool, use_sigaltstack, true, "If set, uses alternate stack for signal handling.") COMMON_FLAG(bool, detect_deadlocks, true, diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp index b1eb2009cf157..15cd1824abc56 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp @@ -47,6 +47,8 @@ typedef void (*sa_sigaction_t)(int, siginfo_t *, void *); namespace __sanitizer { +static atomic_uint8_t signal_handler_is_from_sanitizer[64]; + u32 GetUid() { return getuid(); } @@ -210,6 +212,20 @@ void UnsetAlternateSignalStack() { UnmapOrDie(oldstack.ss_sp, oldstack.ss_size); } +bool IsSignalHandlerFromSanitizer(int signum) { + return atomic_load(&signal_handler_is_from_sanitizer[signum], + memory_order_relaxed); +} + +bool SetSignalHandlerFromSanitizer(int signum, bool new_state) { + if (signum < 0 || static_cast(signum) >= + ARRAY_SIZE(signal_handler_is_from_sanitizer)) + return false; + + return atomic_exchange(&signal_handler_is_from_sanitizer[signum], new_state, + memory_order_relaxed); +} + static void MaybeInstallSigaction(int signum, SignalHandlerType handler) { if (GetHandleSignalMode(signum) == kHandleSignalNo) return; @@ -223,6 +239,9 @@ static void MaybeInstallSigaction(int signum, if (common_flags()->use_sigaltstack) sigact.sa_flags |= SA_ONSTACK; CHECK_EQ(0, internal_sigaction(signum, &sigact, nullptr)); VReport(1, "Installed the sigaction for signal %d\n", signum); + + if (common_flags()->cloak_sanitizer_signal_handlers) + SetSignalHandlerFromSanitizer(signum, true); } void InstallDeadlySignalHandlers(SignalHandlerType handler) { diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_signal_interceptors.inc b/compiler-rt/lib/sanitizer_common/sanitizer_signal_interceptors.inc index 94e4e2954a3b9..8511e4d55fa9e 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_signal_interceptors.inc +++ b/compiler-rt/lib/sanitizer_common/sanitizer_signal_interceptors.inc @@ -45,6 +45,8 @@ using namespace __sanitizer; INTERCEPTOR(uptr, bsd_signal, int signum, uptr handler) { SIGNAL_INTERCEPTOR_ENTER(); if (GetHandleSignalMode(signum) == kHandleSignalExclusive) return 0; + + // TODO: support cloak_sanitizer_signal_handlers SIGNAL_INTERCEPTOR_SIGNAL_IMPL(bsd_signal, signum, handler); } #define INIT_BSD_SIGNAL COMMON_INTERCEPT_FUNCTION(bsd_signal) @@ -56,19 +58,55 @@ INTERCEPTOR(uptr, bsd_signal, int signum, uptr handler) { INTERCEPTOR(uptr, signal, int signum, uptr handler) { SIGNAL_INTERCEPTOR_ENTER(); if (GetHandleSignalMode(signum) == kHandleSignalExclusive) + // The user can neither view nor change the signal handler, regardless of + // the cloak_sanitizer_signal_handlers setting. This differs from + // sigaction(). return (uptr) nullptr; - SIGNAL_INTERCEPTOR_SIGNAL_IMPL(signal, signum, handler); + + uptr ret = +[](auto signal, int signum, uptr handler) { + SIGNAL_INTERCEPTOR_SIGNAL_IMPL(signal, signum, handler); + }(signal, signum, handler); + + if (ret != sig_err && SetSignalHandlerFromSanitizer(signum, false)) + // If the user sets a signal handler, it becomes uncloaked, even if they + // reuse a sanitizer's signal handler. + ret = sig_dfl; + + return ret; } #define INIT_SIGNAL COMMON_INTERCEPT_FUNCTION(signal) INTERCEPTOR(int, sigaction_symname, int signum, const __sanitizer_sigaction *act, __sanitizer_sigaction *oldact) { SIGNAL_INTERCEPTOR_ENTER(); + if (GetHandleSignalMode(signum) == kHandleSignalExclusive) { if (!oldact) return 0; act = nullptr; + // If cloak_sanitizer_signal_handlers=true, the user can neither view nor + // change the signal handle. + // If false, the user can view but not change the signal handler. This + // differs from signal(). } - SIGNAL_INTERCEPTOR_SIGACTION_IMPL(signum, act, oldact); + + int ret = +[](int signum, const __sanitizer_sigaction* act, + __sanitizer_sigaction* oldact) { + SIGNAL_INTERCEPTOR_SIGACTION_IMPL(signum, act, oldact); + }(signum, act, oldact); + + if (act) { + if (ret == 0 && SetSignalHandlerFromSanitizer(signum, false)) { + // If the user sets a signal handler, it becomes uncloaked, even if they + // reuse a sanitizer's signal handler. + + if (oldact) + oldact->handler = reinterpret_cast<__sanitizer_sighandler_ptr>(sig_dfl); + } + } else if (ret == 0 && oldact && IsSignalHandlerFromSanitizer(signum)) { + oldact->handler = reinterpret_cast<__sanitizer_sighandler_ptr>(sig_dfl); + } + + return ret; } #define INIT_SIGACTION COMMON_INTERCEPT_FUNCTION(sigaction_symname) diff --git a/compiler-rt/test/sanitizer_common/TestCases/Linux/allow_user_segv.cpp b/compiler-rt/test/sanitizer_common/TestCases/Linux/allow_user_segv.cpp index 1c740153a81d7..0c5a922ecfb83 100644 --- a/compiler-rt/test/sanitizer_common/TestCases/Linux/allow_user_segv.cpp +++ b/compiler-rt/test/sanitizer_common/TestCases/Linux/allow_user_segv.cpp @@ -23,6 +23,10 @@ // Flaky errors in debuggerd with "waitpid returned unexpected pid (0)" in logcat. // UNSUPPORTED: android && i386-target-arch +// Note: this test case is unusual because it retrieves the original +// (ASan-installed) signal handler; thus, it is incompatible with the +// cloak_sanitizer_signal_handlers runtime option. + #include #include #include diff --git a/compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_sigaction.cpp b/compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_sigaction.cpp new file mode 100644 index 0000000000000..d713d2d1501b6 --- /dev/null +++ b/compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_sigaction.cpp @@ -0,0 +1,51 @@ +// UNSUPPORTED: android +// UNSUPPORTED: hwasan + +// RUN: %clangxx -O0 %s -o %t + +// Sanitizer signal handler not installed; custom signal handler installed +// RUN: %env_tool_opts=handle_segv=0:cloak_sanitizer_signal_handlers=false not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,CUSTOM +// RUN: %env_tool_opts=handle_segv=0:cloak_sanitizer_signal_handlers=true not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,CUSTOM + +// Sanitizer signal handler installed but overriden by custom signal handler +// RUN: %env_tool_opts=handle_segv=1:cloak_sanitizer_signal_handlers=false not %run %t 2>&1 | FileCheck %s --check-prefixes=NONDEFAULT,CUSTOM +// RUN: %env_tool_opts=handle_segv=1:cloak_sanitizer_signal_handlers=true not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,CUSTOM + +// Sanitizer signal handler installed immutably +// N.B. for handle_segv=2 with cloaking off, there is a pre-existing difference +// in signal vs. sigaction: signal effectively cloaks the handler. +// RUN: %env_tool_opts=handle_segv=2:cloak_sanitizer_signal_handlers=false not %run %t 2>&1 | FileCheck %s --check-prefixes=NONDEFAULT,SANITIZER +// RUN: %env_tool_opts=handle_segv=2:cloak_sanitizer_signal_handlers=true not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,SANITIZER + +#include +#include +#include + +void handler(int signum, siginfo_t *info, void *context) { + printf("Custom signal handler\n"); + exit(1); +} + +int main(int argc, char *argv[]) { + struct sigaction sa = {0}; + struct sigaction old = {0}; + sa.sa_flags = SA_SIGINFO; + sa.sa_sigaction = &handler; + sigaction(SIGSEGV, &sa, &old); + + if (reinterpret_cast(old.sa_sigaction) == SIG_DFL) + printf("Old handler: default\n"); + // DEFAULT: Old handler: default + else + printf("Old handler: non-default\n"); + // NONDEFAULT: Old handler: non-default + + fflush(stdout); + + char *c = (char *)0x123; + printf("%d\n", *c); + // CUSTOM: Custom signal handler + // SANITIZER: Sanitizer:DEADLYSIGNAL + + return 0; +} diff --git a/compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_signal.cpp b/compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_signal.cpp new file mode 100644 index 0000000000000..1c166c953fcfd --- /dev/null +++ b/compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_signal.cpp @@ -0,0 +1,46 @@ +// UNSUPPORTED: android +// UNSUPPORTED: hwasan + +// RUN: %clangxx -O0 %s -o %t + +// Sanitizer signal handler not installed; custom signal handler installed +// RUN: %env_tool_opts=handle_segv=0:cloak_sanitizer_signal_handlers=false not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,CUSTOM +// RUN: %env_tool_opts=handle_segv=0:cloak_sanitizer_signal_handlers=true not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,CUSTOM + +// Sanitizer signal handler installed but overriden by custom signal handler +// RUN: %env_tool_opts=handle_segv=1:cloak_sanitizer_signal_handlers=false not %run %t 2>&1 | FileCheck %s --check-prefixes=NONDEFAULT,CUSTOM +// RUN: %env_tool_opts=handle_segv=1:cloak_sanitizer_signal_handlers=true not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,CUSTOM + +// Sanitizer signal handler installed immutably +// N.B. for handle_segv=2 with cloaking off, there is a pre-existing difference +// in signal vs. sigaction: signal effectively cloaks the handler. +// RUN: %env_tool_opts=handle_segv=2:cloak_sanitizer_signal_handlers=false not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,SANITIZER +// RUN: %env_tool_opts=handle_segv=2:cloak_sanitizer_signal_handlers=true not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,SANITIZER + +#include +#include +#include + +void my_signal_sighandler(int signum) { + printf("Custom signal handler\n"); + exit(1); +} + +int main(int argc, char *argv[]) { + __sighandler_t old = signal(SIGSEGV, &my_signal_sighandler); + if (old == SIG_DFL) + printf("Old handler: default\n"); + // DEFAULT: Old handler: default + else + printf("Old handler: non-default\n"); + // NONDEFAULT: Old handler: non-default + + fflush(stdout); + + char *c = (char *)0x123; + printf("%d\n", *c); + // CUSTOM: Custom signal handler + // SANITIZER: Sanitizer:DEADLYSIGNAL + + return 0; +} From 4d479347ae5c3a2274cbb4171fb7a3c2378baa6b Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Tue, 14 Oct 2025 04:28:10 +0000 Subject: [PATCH 2/4] Use raise(SIGSEGV) instead of dereferencing a pointer --- .../sanitizer_common/TestCases/Linux/cloak_sigaction.cpp | 5 +++-- .../test/sanitizer_common/TestCases/Linux/cloak_signal.cpp | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_sigaction.cpp b/compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_sigaction.cpp index d713d2d1501b6..5ade1386e3574 100644 --- a/compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_sigaction.cpp +++ b/compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_sigaction.cpp @@ -42,8 +42,9 @@ int main(int argc, char *argv[]) { fflush(stdout); - char *c = (char *)0x123; - printf("%d\n", *c); + // Trying to organically segfault by dereferencing a pointer can be tricky + // in builds with assertions. + raise(SIGSEGV); // CUSTOM: Custom signal handler // SANITIZER: Sanitizer:DEADLYSIGNAL diff --git a/compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_signal.cpp b/compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_signal.cpp index 1c166c953fcfd..59ca883d9d3f0 100644 --- a/compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_signal.cpp +++ b/compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_signal.cpp @@ -37,8 +37,9 @@ int main(int argc, char *argv[]) { fflush(stdout); - char *c = (char *)0x123; - printf("%d\n", *c); + // Trying to organically segfault by dereferencing a pointer can be tricky + // in builds with assertions. + raise(SIGSEGV); // CUSTOM: Custom signal handler // SANITIZER: Sanitizer:DEADLYSIGNAL From e01c9b02cba8aa453515a1c46498b852d4fb6c79 Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Tue, 14 Oct 2025 04:28:39 +0000 Subject: [PATCH 3/4] Add [[maybe_unused]] annotation to signal_handler_is_from_sanitizer --- compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp index 15cd1824abc56..8e5e87938c372 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp @@ -47,7 +47,7 @@ typedef void (*sa_sigaction_t)(int, siginfo_t *, void *); namespace __sanitizer { -static atomic_uint8_t signal_handler_is_from_sanitizer[64]; +[[maybe_unused]] static atomic_uint8_t signal_handler_is_from_sanitizer[64]; u32 GetUid() { return getuid(); From 48dae8d48f50a7174c78e52406036fe78af04540 Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Tue, 14 Oct 2025 17:01:30 +0000 Subject: [PATCH 4/4] Add note on SIGBUS --- .../test/sanitizer_common/TestCases/Linux/cloak_sigaction.cpp | 3 ++- .../test/sanitizer_common/TestCases/Linux/cloak_signal.cpp | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_sigaction.cpp b/compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_sigaction.cpp index 5ade1386e3574..422e4abe880c5 100644 --- a/compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_sigaction.cpp +++ b/compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_sigaction.cpp @@ -43,7 +43,8 @@ int main(int argc, char *argv[]) { fflush(stdout); // Trying to organically segfault by dereferencing a pointer can be tricky - // in builds with assertions. + // in builds with assertions. Additionally, some older platforms may SIGBUS + // instead. raise(SIGSEGV); // CUSTOM: Custom signal handler // SANITIZER: Sanitizer:DEADLYSIGNAL diff --git a/compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_signal.cpp b/compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_signal.cpp index 59ca883d9d3f0..48e54756c2d0c 100644 --- a/compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_signal.cpp +++ b/compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_signal.cpp @@ -38,7 +38,8 @@ int main(int argc, char *argv[]) { fflush(stdout); // Trying to organically segfault by dereferencing a pointer can be tricky - // in builds with assertions. + // in builds with assertions. Additionally, some older platforms may SIGBUS + // instead. raise(SIGSEGV); // CUSTOM: Custom signal handler // SANITIZER: Sanitizer:DEADLYSIGNAL