Skip to content

Commit 16fa592

Browse files
thurstondgithub-actions[bot]
authored andcommitted
Automerge: Reapply "[sanitizer] Add cloak_sanitizer_signal_handlers runtime option" (#163308) (#163423)
This reverts commit llvm/llvm-project@27d8441 i.e., relands 812a225. This reland uses `raise(SIGSEGV)` instead of trying to segfault via 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.
2 parents 80c575c + 7f5ed91 commit 16fa592

File tree

7 files changed

+172
-2
lines changed

7 files changed

+172
-2
lines changed

compiler-rt/lib/sanitizer_common/sanitizer_common.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,9 @@ void ReportDeadlySignal(const SignalContext &sig, u32 tid,
390390
void SetAlternateSignalStack();
391391
void UnsetAlternateSignalStack();
392392

393+
bool IsSignalHandlerFromSanitizer(int signum);
394+
bool SetSignalHandlerFromSanitizer(int signum, bool new_state);
395+
393396
// Construct a one-line string:
394397
// SUMMARY: SanitizerToolName: error_message
395398
// and pass it to __sanitizer_report_error_summary.

compiler-rt/lib/sanitizer_common/sanitizer_flags.inc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,11 @@ COMMON_FLAG(HandleSignalMode, handle_sigfpe, kHandleSignalYes,
113113
COMMON_FLAG(bool, allow_user_segv_handler, true,
114114
"Deprecated. True has no effect, use handle_sigbus=1. If false, "
115115
"handle_*=1 will be upgraded to handle_*=2.")
116+
COMMON_FLAG(bool, cloak_sanitizer_signal_handlers, false,
117+
"If set, signal/sigaction will pretend that sanitizers did not "
118+
"preinstall any signal handlers. If the user subsequently installs "
119+
"a signal handler, this will disable cloaking for the respective "
120+
"signal.")
116121
COMMON_FLAG(bool, use_sigaltstack, true,
117122
"If set, uses alternate stack for signal handling.")
118123
COMMON_FLAG(bool, detect_deadlocks, true,

compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ typedef void (*sa_sigaction_t)(int, siginfo_t *, void *);
4747

4848
namespace __sanitizer {
4949

50+
[[maybe_unused]] static atomic_uint8_t signal_handler_is_from_sanitizer[64];
51+
5052
u32 GetUid() {
5153
return getuid();
5254
}
@@ -210,6 +212,20 @@ void UnsetAlternateSignalStack() {
210212
UnmapOrDie(oldstack.ss_sp, oldstack.ss_size);
211213
}
212214

215+
bool IsSignalHandlerFromSanitizer(int signum) {
216+
return atomic_load(&signal_handler_is_from_sanitizer[signum],
217+
memory_order_relaxed);
218+
}
219+
220+
bool SetSignalHandlerFromSanitizer(int signum, bool new_state) {
221+
if (signum < 0 || static_cast<unsigned>(signum) >=
222+
ARRAY_SIZE(signal_handler_is_from_sanitizer))
223+
return false;
224+
225+
return atomic_exchange(&signal_handler_is_from_sanitizer[signum], new_state,
226+
memory_order_relaxed);
227+
}
228+
213229
static void MaybeInstallSigaction(int signum,
214230
SignalHandlerType handler) {
215231
if (GetHandleSignalMode(signum) == kHandleSignalNo) return;
@@ -223,6 +239,9 @@ static void MaybeInstallSigaction(int signum,
223239
if (common_flags()->use_sigaltstack) sigact.sa_flags |= SA_ONSTACK;
224240
CHECK_EQ(0, internal_sigaction(signum, &sigact, nullptr));
225241
VReport(1, "Installed the sigaction for signal %d\n", signum);
242+
243+
if (common_flags()->cloak_sanitizer_signal_handlers)
244+
SetSignalHandlerFromSanitizer(signum, true);
226245
}
227246

228247
void InstallDeadlySignalHandlers(SignalHandlerType handler) {

compiler-rt/lib/sanitizer_common/sanitizer_signal_interceptors.inc

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ using namespace __sanitizer;
4545
INTERCEPTOR(uptr, bsd_signal, int signum, uptr handler) {
4646
SIGNAL_INTERCEPTOR_ENTER();
4747
if (GetHandleSignalMode(signum) == kHandleSignalExclusive) return 0;
48+
49+
// TODO: support cloak_sanitizer_signal_handlers
4850
SIGNAL_INTERCEPTOR_SIGNAL_IMPL(bsd_signal, signum, handler);
4951
}
5052
#define INIT_BSD_SIGNAL COMMON_INTERCEPT_FUNCTION(bsd_signal)
@@ -56,19 +58,55 @@ INTERCEPTOR(uptr, bsd_signal, int signum, uptr handler) {
5658
INTERCEPTOR(uptr, signal, int signum, uptr handler) {
5759
SIGNAL_INTERCEPTOR_ENTER();
5860
if (GetHandleSignalMode(signum) == kHandleSignalExclusive)
61+
// The user can neither view nor change the signal handler, regardless of
62+
// the cloak_sanitizer_signal_handlers setting. This differs from
63+
// sigaction().
5964
return (uptr) nullptr;
60-
SIGNAL_INTERCEPTOR_SIGNAL_IMPL(signal, signum, handler);
65+
66+
uptr ret = +[](auto signal, int signum, uptr handler) {
67+
SIGNAL_INTERCEPTOR_SIGNAL_IMPL(signal, signum, handler);
68+
}(signal, signum, handler);
69+
70+
if (ret != sig_err && SetSignalHandlerFromSanitizer(signum, false))
71+
// If the user sets a signal handler, it becomes uncloaked, even if they
72+
// reuse a sanitizer's signal handler.
73+
ret = sig_dfl;
74+
75+
return ret;
6176
}
6277
#define INIT_SIGNAL COMMON_INTERCEPT_FUNCTION(signal)
6378

6479
INTERCEPTOR(int, sigaction_symname, int signum,
6580
const __sanitizer_sigaction *act, __sanitizer_sigaction *oldact) {
6681
SIGNAL_INTERCEPTOR_ENTER();
82+
6783
if (GetHandleSignalMode(signum) == kHandleSignalExclusive) {
6884
if (!oldact) return 0;
6985
act = nullptr;
86+
// If cloak_sanitizer_signal_handlers=true, the user can neither view nor
87+
// change the signal handle.
88+
// If false, the user can view but not change the signal handler. This
89+
// differs from signal().
7090
}
71-
SIGNAL_INTERCEPTOR_SIGACTION_IMPL(signum, act, oldact);
91+
92+
int ret = +[](int signum, const __sanitizer_sigaction* act,
93+
__sanitizer_sigaction* oldact) {
94+
SIGNAL_INTERCEPTOR_SIGACTION_IMPL(signum, act, oldact);
95+
}(signum, act, oldact);
96+
97+
if (act) {
98+
if (ret == 0 && SetSignalHandlerFromSanitizer(signum, false)) {
99+
// If the user sets a signal handler, it becomes uncloaked, even if they
100+
// reuse a sanitizer's signal handler.
101+
102+
if (oldact)
103+
oldact->handler = reinterpret_cast<__sanitizer_sighandler_ptr>(sig_dfl);
104+
}
105+
} else if (ret == 0 && oldact && IsSignalHandlerFromSanitizer(signum)) {
106+
oldact->handler = reinterpret_cast<__sanitizer_sighandler_ptr>(sig_dfl);
107+
}
108+
109+
return ret;
72110
}
73111
#define INIT_SIGACTION COMMON_INTERCEPT_FUNCTION(sigaction_symname)
74112

compiler-rt/test/sanitizer_common/TestCases/Linux/allow_user_segv.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@
2323
// Flaky errors in debuggerd with "waitpid returned unexpected pid (0)" in logcat.
2424
// UNSUPPORTED: android && i386-target-arch
2525

26+
// Note: this test case is unusual because it retrieves the original
27+
// (ASan-installed) signal handler; thus, it is incompatible with the
28+
// cloak_sanitizer_signal_handlers runtime option.
29+
2630
#include <signal.h>
2731
#include <stdio.h>
2832
#include <stdlib.h>
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// UNSUPPORTED: android
2+
// UNSUPPORTED: hwasan
3+
4+
// RUN: %clangxx -O0 %s -o %t
5+
6+
// Sanitizer signal handler not installed; custom signal handler installed
7+
// RUN: %env_tool_opts=handle_segv=0:cloak_sanitizer_signal_handlers=false not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,CUSTOM
8+
// RUN: %env_tool_opts=handle_segv=0:cloak_sanitizer_signal_handlers=true not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,CUSTOM
9+
10+
// Sanitizer signal handler installed but overriden by custom signal handler
11+
// RUN: %env_tool_opts=handle_segv=1:cloak_sanitizer_signal_handlers=false not %run %t 2>&1 | FileCheck %s --check-prefixes=NONDEFAULT,CUSTOM
12+
// RUN: %env_tool_opts=handle_segv=1:cloak_sanitizer_signal_handlers=true not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,CUSTOM
13+
14+
// Sanitizer signal handler installed immutably
15+
// N.B. for handle_segv=2 with cloaking off, there is a pre-existing difference
16+
// in signal vs. sigaction: signal effectively cloaks the handler.
17+
// RUN: %env_tool_opts=handle_segv=2:cloak_sanitizer_signal_handlers=false not %run %t 2>&1 | FileCheck %s --check-prefixes=NONDEFAULT,SANITIZER
18+
// RUN: %env_tool_opts=handle_segv=2:cloak_sanitizer_signal_handlers=true not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,SANITIZER
19+
20+
#include <signal.h>
21+
#include <stdio.h>
22+
#include <stdlib.h>
23+
24+
void handler(int signum, siginfo_t *info, void *context) {
25+
printf("Custom signal handler\n");
26+
exit(1);
27+
}
28+
29+
int main(int argc, char *argv[]) {
30+
struct sigaction sa = {0};
31+
struct sigaction old = {0};
32+
sa.sa_flags = SA_SIGINFO;
33+
sa.sa_sigaction = &handler;
34+
sigaction(SIGSEGV, &sa, &old);
35+
36+
if (reinterpret_cast<void *>(old.sa_sigaction) == SIG_DFL)
37+
printf("Old handler: default\n");
38+
// DEFAULT: Old handler: default
39+
else
40+
printf("Old handler: non-default\n");
41+
// NONDEFAULT: Old handler: non-default
42+
43+
fflush(stdout);
44+
45+
// Trying to organically segfault by dereferencing a pointer can be tricky
46+
// in builds with assertions. Additionally, some older platforms may SIGBUS
47+
// instead.
48+
raise(SIGSEGV);
49+
// CUSTOM: Custom signal handler
50+
// SANITIZER: Sanitizer:DEADLYSIGNAL
51+
52+
return 0;
53+
}
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// UNSUPPORTED: android
2+
// UNSUPPORTED: hwasan
3+
4+
// RUN: %clangxx -O0 %s -o %t
5+
6+
// Sanitizer signal handler not installed; custom signal handler installed
7+
// RUN: %env_tool_opts=handle_segv=0:cloak_sanitizer_signal_handlers=false not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,CUSTOM
8+
// RUN: %env_tool_opts=handle_segv=0:cloak_sanitizer_signal_handlers=true not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,CUSTOM
9+
10+
// Sanitizer signal handler installed but overriden by custom signal handler
11+
// RUN: %env_tool_opts=handle_segv=1:cloak_sanitizer_signal_handlers=false not %run %t 2>&1 | FileCheck %s --check-prefixes=NONDEFAULT,CUSTOM
12+
// RUN: %env_tool_opts=handle_segv=1:cloak_sanitizer_signal_handlers=true not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,CUSTOM
13+
14+
// Sanitizer signal handler installed immutably
15+
// N.B. for handle_segv=2 with cloaking off, there is a pre-existing difference
16+
// in signal vs. sigaction: signal effectively cloaks the handler.
17+
// RUN: %env_tool_opts=handle_segv=2:cloak_sanitizer_signal_handlers=false not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,SANITIZER
18+
// RUN: %env_tool_opts=handle_segv=2:cloak_sanitizer_signal_handlers=true not %run %t 2>&1 | FileCheck %s --check-prefixes=DEFAULT,SANITIZER
19+
20+
#include <signal.h>
21+
#include <stdio.h>
22+
#include <stdlib.h>
23+
24+
void my_signal_sighandler(int signum) {
25+
printf("Custom signal handler\n");
26+
exit(1);
27+
}
28+
29+
int main(int argc, char *argv[]) {
30+
__sighandler_t old = signal(SIGSEGV, &my_signal_sighandler);
31+
if (old == SIG_DFL)
32+
printf("Old handler: default\n");
33+
// DEFAULT: Old handler: default
34+
else
35+
printf("Old handler: non-default\n");
36+
// NONDEFAULT: Old handler: non-default
37+
38+
fflush(stdout);
39+
40+
// Trying to organically segfault by dereferencing a pointer can be tricky
41+
// in builds with assertions. Additionally, some older platforms may SIGBUS
42+
// instead.
43+
raise(SIGSEGV);
44+
// CUSTOM: Custom signal handler
45+
// SANITIZER: Sanitizer:DEADLYSIGNAL
46+
47+
return 0;
48+
}

0 commit comments

Comments
 (0)