[compiler-rt] 4fa989c - Fix TSAN signal interceptor out-of-bound access
Shu-Chun Weng via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 22 12:38:26 PDT 2021
Author: Shu-Chun Weng
Date: 2021-07-22T12:38:07-07:00
New Revision: 4fa989c7b23c5d518fb381b4a292a00985cb8eec
URL: https://github.com/llvm/llvm-project/commit/4fa989c7b23c5d518fb381b4a292a00985cb8eec
DIFF: https://github.com/llvm/llvm-project/commit/4fa989c7b23c5d518fb381b4a292a00985cb8eec.diff
LOG: Fix TSAN signal interceptor out-of-bound access
signal(2) and sigaction(2) have defined behaviors for invalid signal number
(EINVAL) and some programs rely on it.
The added test case also reveals that MSAN is too strict in this regard.
Test case passed on x86_64 Linux and AArch64 Linux.
Reviewed By: vitalybuka
Differential Revision: https://reviews.llvm.org/D106468
Added:
compiler-rt/test/sanitizer_common/TestCases/Posix/signal.cpp
Modified:
compiler-rt/lib/msan/msan_interceptors.cpp
compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
Removed:
################################################################################
diff --git a/compiler-rt/lib/msan/msan_interceptors.cpp b/compiler-rt/lib/msan/msan_interceptors.cpp
index 529bf22f05b94..760f74e927d09 100644
--- a/compiler-rt/lib/msan/msan_interceptors.cpp
+++ b/compiler-rt/lib/msan/msan_interceptors.cpp
@@ -21,6 +21,7 @@
#include "msan_report.h"
#include "msan_thread.h"
#include "msan_poisoning.h"
+#include "sanitizer_common/sanitizer_errno_codes.h"
#include "sanitizer_common/sanitizer_platform_limits_posix.h"
#include "sanitizer_common/sanitizer_platform_limits_netbsd.h"
#include "sanitizer_common/sanitizer_allocator.h"
@@ -1373,11 +1374,14 @@ static int sigaction_impl(int signo, const __sanitizer_sigaction *act,
static int sigaction_impl(int signo, const __sanitizer_sigaction *act,
__sanitizer_sigaction *oldact) {
ENSURE_MSAN_INITED();
+ if (signo <= 0 || signo >= kMaxSignals) {
+ errno = errno_EINVAL;
+ return -1;
+ }
if (act) read_sigaction(act);
int res;
if (flags()->wrap_signals) {
SpinMutexLock lock(&sigactions_mu);
- CHECK_LT(signo, kMaxSignals);
uptr old_cb = atomic_load(&sigactions[signo], memory_order_relaxed);
__sanitizer_sigaction new_act;
__sanitizer_sigaction *pnew_act = act ? &new_act : nullptr;
@@ -1411,8 +1415,11 @@ static int sigaction_impl(int signo, const __sanitizer_sigaction *act,
static uptr signal_impl(int signo, uptr cb) {
ENSURE_MSAN_INITED();
+ if (signo <= 0 || signo >= kMaxSignals) {
+ errno = errno_EINVAL;
+ return -1;
+ }
if (flags()->wrap_signals) {
- CHECK_LT(signo, kMaxSignals);
SpinMutexLock lock(&sigactions_mu);
if (cb != __sanitizer::sig_ign && cb != __sanitizer::sig_dfl) {
atomic_store(&sigactions[signo], cb, memory_order_relaxed);
diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
index 6808f2e0e2d3d..6636ce6383d71 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
@@ -2422,6 +2422,10 @@ int sigaction_impl(int sig, const __sanitizer_sigaction *act,
// the signal handler through rtl_sigaction, very bad things will happen.
// The handler will run synchronously and corrupt tsan per-thread state.
SCOPED_INTERCEPTOR_RAW(sigaction, sig, act, old);
+ if (sig <= 0 || sig >= kSigCount) {
+ errno = errno_EINVAL;
+ return -1;
+ }
__sanitizer_sigaction *sigactions = interceptor_ctx()->sigactions;
__sanitizer_sigaction old_stored;
if (old) internal_memcpy(&old_stored, &sigactions[sig], sizeof(old_stored));
diff --git a/compiler-rt/test/sanitizer_common/TestCases/Posix/signal.cpp b/compiler-rt/test/sanitizer_common/TestCases/Posix/signal.cpp
new file mode 100644
index 0000000000000..f5351b01ac002
--- /dev/null
+++ b/compiler-rt/test/sanitizer_common/TestCases/Posix/signal.cpp
@@ -0,0 +1,241 @@
+// RUN: %clangxx -O0 -g %s -o %t && %run %t 2>&1 | FileCheck %s
+
+#include <assert.h>
+#include <climits>
+#include <errno.h>
+#include <stdio.h>
+#include <signal.h>
+
+#include <initializer_list>
+
+constexpr int std_signals[] = {
+ SIGHUP,
+ SIGINT,
+ SIGQUIT,
+ SIGILL,
+ SIGTRAP,
+ SIGABRT,
+ SIGIOT,
+ SIGBUS,
+ SIGFPE,
+ SIGUSR1,
+ SIGSEGV,
+ SIGUSR2,
+ SIGPIPE,
+ SIGALRM,
+ SIGTERM,
+ SIGCHLD,
+ SIGCONT,
+ SIGTSTP,
+ SIGTTIN,
+ SIGTTOU,
+ SIGURG,
+ SIGXCPU,
+ SIGXFSZ,
+ SIGVTALRM,
+ SIGPROF,
+ SIGWINCH,
+ SIGIO,
+ SIGSYS,
+};
+
+constexpr int no_change_act_signals[] = {
+ SIGKILL,
+ SIGSTOP,
+};
+
+void signal_handler(int) {}
+void signal_action_handler(int, siginfo_t*, void*) {}
+
+void test_signal_custom() {
+ for (int signum : std_signals) {
+ sighandler_t ret = signal(signum, &signal_handler);
+ assert(ret != SIG_ERR);
+ }
+ for (int signum = SIGRTMIN; signum <= SIGRTMAX; ++signum) {
+ sighandler_t ret = signal(signum, &signal_handler);
+ assert(ret != SIG_ERR);
+ }
+ for (int signum : no_change_act_signals) {
+ sighandler_t ret = signal(signum, &signal_handler);
+ int err = errno;
+ assert(ret == SIG_ERR);
+ assert(err == EINVAL);
+ }
+ for (int signum : { 0, SIGRTMAX + 1, INT_MAX}) {
+ sighandler_t ret = signal(signum, &signal_handler);
+ int err = errno;
+ assert(ret == SIG_ERR);
+ assert(err == EINVAL);
+ }
+}
+
+void test_signal_ignore() {
+ for (int signum : std_signals) {
+ sighandler_t ret = signal(signum, SIG_IGN);
+ if (signum != SIGCHLD) {
+ // POSIX.1-1990 disallowed setting the action for SIGCHLD to SIG_IGN
+ // though POSIX.1-2001 and later allow this possibility.
+ assert(ret != SIG_ERR);
+ }
+ }
+ for (int signum = SIGRTMIN; signum <= SIGRTMAX; ++signum) {
+ sighandler_t ret = signal(signum, SIG_IGN);
+ assert(ret != SIG_ERR);
+ }
+ for (int signum : no_change_act_signals) {
+ sighandler_t ret = signal(signum, SIG_IGN);
+ int err = errno;
+ assert(ret == SIG_ERR);
+ assert(err == EINVAL);
+ }
+ for (int signum : { 0, SIGRTMAX + 1, INT_MAX}) {
+ sighandler_t ret = signal(signum, SIG_IGN);
+ int err = errno;
+ assert(ret == SIG_ERR);
+ assert(err == EINVAL);
+ }
+}
+
+void test_signal_default() {
+ for (int signum : std_signals) {
+ sighandler_t ret = signal(signum, SIG_DFL);
+ assert(ret != SIG_ERR);
+ }
+ for (int signum = SIGRTMIN; signum <= SIGRTMAX; ++signum) {
+ sighandler_t ret = signal(signum, SIG_DFL);
+ assert(ret != SIG_ERR);
+ }
+ for (int signum : { 0, SIGRTMAX + 1, INT_MAX}) {
+ sighandler_t ret = signal(signum, SIG_DFL);
+ int err = errno;
+ assert(ret == SIG_ERR);
+ assert(err == EINVAL);
+ }
+}
+
+void test_sigaction_custom() {
+ struct sigaction act = {}, oldact;
+
+ act.sa_handler = &signal_handler;
+ sigemptyset(&act.sa_mask);
+ act.sa_flags = 0;
+
+ for (int signum : std_signals) {
+ int ret = sigaction(signum, &act, &oldact);
+ assert(ret == 0);
+ }
+ for (int signum = SIGRTMIN; signum <= SIGRTMAX; ++signum) {
+ int ret = sigaction(signum, &act, &oldact);
+ assert(ret == 0);
+ }
+ for (int signum : no_change_act_signals) {
+ int ret = sigaction(signum, &act, &oldact);
+ int err = errno;
+ assert(ret == -1);
+ assert(err == EINVAL);
+ }
+ for (int signum : { 0, SIGRTMAX + 1, INT_MAX}) {
+ int ret = sigaction(signum, &act, &oldact);
+ int err = errno;
+ assert(ret == -1);
+ assert(err == EINVAL);
+ }
+
+ act.sa_handler = nullptr;
+ act.sa_sigaction = &signal_action_handler;
+ act.sa_flags = SA_SIGINFO;
+
+ for (int signum : std_signals) {
+ int ret = sigaction(signum, &act, &oldact);
+ assert(ret == 0);
+ }
+ for (int signum = SIGRTMIN; signum <= SIGRTMAX; ++signum) {
+ int ret = sigaction(signum, &act, &oldact);
+ assert(ret == 0);
+ }
+ for (int signum : no_change_act_signals) {
+ int ret = sigaction(signum, &act, &oldact);
+ int err = errno;
+ assert(ret == -1);
+ assert(err == EINVAL);
+ }
+ for (int signum : { 0, SIGRTMAX + 1, INT_MAX}) {
+ int ret = sigaction(signum, &act, &oldact);
+ int err = errno;
+ assert(ret == -1);
+ assert(err == EINVAL);
+ }
+}
+
+void test_sigaction_ignore() {
+ struct sigaction act = {}, oldact;
+
+ act.sa_handler = SIG_IGN;
+ sigemptyset(&act.sa_mask);
+ act.sa_flags = 0;
+
+ for (int signum : std_signals) {
+ int ret = sigaction(signum, &act, &oldact);
+ if (signum != SIGCHLD) {
+ // POSIX.1-1990 disallowed setting the action for SIGCHLD to SIG_IGN
+ // though POSIX.1-2001 and later allow this possibility.
+ assert(ret == 0);
+ }
+ }
+ for (int signum = SIGRTMIN; signum <= SIGRTMAX; ++signum) {
+ int ret = sigaction(signum, &act, &oldact);
+ assert(ret == 0);
+ }
+ for (int signum : no_change_act_signals) {
+ int ret = sigaction(signum, &act, &oldact);
+ int err = errno;
+ assert(ret == -1);
+ assert(err == EINVAL);
+ }
+ for (int signum : { 0, SIGRTMAX + 1, INT_MAX}) {
+ int ret = sigaction(signum, &act, &oldact);
+ int err = errno;
+ assert(ret == -1);
+ assert(err == EINVAL);
+ }
+}
+
+void test_sigaction_default() {
+ struct sigaction act = {}, oldact;
+
+ act.sa_handler = SIG_DFL;
+ sigemptyset(&act.sa_mask);
+ act.sa_flags = 0;
+
+ for (int signum : std_signals) {
+ int ret = sigaction(signum, &act, &oldact);
+ assert(ret == 0);
+ }
+ for (int signum = SIGRTMIN; signum <= SIGRTMAX; ++signum) {
+ int ret = sigaction(signum, &act, &oldact);
+ assert(ret == 0);
+ }
+ for (int signum : { 0, SIGRTMAX + 1, INT_MAX}) {
+ int ret = sigaction(signum, &act, &oldact);
+ int err = errno;
+ assert(ret == -1);
+ assert(err == EINVAL);
+ }
+}
+
+int main(void) {
+ printf("sigaction\n");
+
+ test_signal_custom();
+ test_signal_ignore();
+ test_signal_default();
+
+ test_sigaction_custom();
+ test_sigaction_ignore();
+ test_sigaction_default();
+
+ // CHECK: sigaction
+
+ return 0;
+}
More information about the llvm-commits
mailing list