From 743f1c2fc756b3ccbd662946ab3cc569a54b6926 Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Mon, 13 Oct 2025 19:38:46 -0700 Subject: [PATCH] Revert "[sanitizer] Add cloak_sanitizer_signal_handlers runtime option (#162746)" This reverts commit 812a225811bd43aff1e5a5cf1117a0531e533504. --- .../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, 2 insertions(+), 168 deletions(-) delete mode 100644 compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_sigaction.cpp delete 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 ba85a0eb5a35e..3e82df498572c 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_common.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_common.h @@ -390,9 +390,6 @@ 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 5f449907f6011..650a4580bbcf0 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_flags.inc +++ b/compiler-rt/lib/sanitizer_common/sanitizer_flags.inc @@ -113,11 +113,6 @@ 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 15cd1824abc56..b1eb2009cf157 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp @@ -47,8 +47,6 @@ 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(); } @@ -212,20 +210,6 @@ 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; @@ -239,9 +223,6 @@ 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 8511e4d55fa9e..94e4e2954a3b9 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_signal_interceptors.inc +++ b/compiler-rt/lib/sanitizer_common/sanitizer_signal_interceptors.inc @@ -45,8 +45,6 @@ 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) @@ -58,55 +56,19 @@ 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; - - 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; + SIGNAL_INTERCEPTOR_SIGNAL_IMPL(signal, signum, handler); } #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(). } - - 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; + SIGNAL_INTERCEPTOR_SIGACTION_IMPL(signum, act, oldact); } #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 0c5a922ecfb83..1c740153a81d7 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,10 +23,6 @@ // 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 deleted file mode 100644 index d713d2d1501b6..0000000000000 --- a/compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_sigaction.cpp +++ /dev/null @@ -1,51 +0,0 @@ -// 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 deleted file mode 100644 index 1c166c953fcfd..0000000000000 --- a/compiler-rt/test/sanitizer_common/TestCases/Linux/cloak_signal.cpp +++ /dev/null @@ -1,46 +0,0 @@ -// 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; -}