From: Shu-Chun Weng Date: Wed, 21 Jul 2021 17:59:09 +0000 (-0700) Subject: Fix TSAN signal interceptor out-of-bound access X-Git-Tag: llvmorg-14-init~584 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=4fa989c7b23c5d518fb381b4a292a00985cb8eec;p=platform%2Fupstream%2Fllvm.git 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 --- diff --git a/compiler-rt/lib/msan/msan_interceptors.cpp b/compiler-rt/lib/msan/msan_interceptors.cpp index 529bf22..760f74e 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 6808f2e..6636ce6 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 0000000..f5351b0 --- /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 +#include +#include +#include +#include + +#include + +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; +}