[compiler-rt] Reapply "[sanitizer] Add cloak_sanitizer_signal_handlers runtime option" (#163308) (PR #163423)
Thurston Dang via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 14 09:55:42 PDT 2025
https://github.com/thurstond created https://github.com/llvm/llvm-project/pull/163423
This reverts commit https://github.com/llvm/llvm-project/commit/27d8441f8282c740903529d8a6b73401fc6c17fa i.e., relands 812a225811bd43aff1e5a5cf1117a0531e533504.
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.
Additionally, this reland marks signal_handler_is_from_sanitizer as `[[maybe_unused]]`.
Original commit message:
If set, signal/sigaction will pretend that the sanitizers did not preinstall any signal handlers. If a user successfully installs a signal handler, it will not be cloaked.
The flag is currently off by default, which means this patch should not affect the behavior of any sanitizers.
This can be useful in an ecosystem where:
1) there exists a library that will install a signal handler iff it does not detect a preinstalled signal handler (a heuristic to prevent overriding user-installed exception handlers etc.)
2) the aforementioned library is linked in to some, but not all, apps
3) user-installed signal handlers are intended to have the highest priority, followed by the library-installed signal handler, and then the sanitizer's signal handler
The flag is in sanitizer_common, though it is currently only supported in ASan, LSan, MSan, TSan and UBSan.
>From 825e43e485b9517c2dd9d23a4487aa467787903b Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Tue, 14 Oct 2025 04:22:10 +0000
Subject: [PATCH 1/3] 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<unsigned>(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 <signal.h>
#include <stdio.h>
#include <stdlib.h>
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 <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+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<void *>(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 <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+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 <thurston at google.com>
Date: Tue, 14 Oct 2025 04:28:10 +0000
Subject: [PATCH 2/3] 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 <thurston at google.com>
Date: Tue, 14 Oct 2025 04:28:39 +0000
Subject: [PATCH 3/3] 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();
More information about the llvm-commits
mailing list