diff --git a/compiler-rt/lib/dfsan/dfsan.h b/compiler-rt/lib/dfsan/dfsan.h --- a/compiler-rt/lib/dfsan/dfsan.h +++ b/compiler-rt/lib/dfsan/dfsan.h @@ -67,6 +67,30 @@ return flags_data; } +// Backup DFSan runtime TLS state. +// Implementation must be async-signal-safe and use small data size, because +// instances of this class may live on the signal handler stack. +// +// This is used by only signal headlers for now. DFSan uses TLS to pass metadata +// of arguments and return values. When an instrumented function accesse the +// TLS, if a signal callback happens, and the callback calls other instrumented +// functions with updating the same TLS, the TLS is in an inconsistent state +// after the callback ends. This may cause either under-tainting or +// over-tainting. +// +// The current implementation simply resets TLS at restore. This prevents from +// over-tainting. Although under-tainting may still happen, a taint flow can be +// found eventually if we run a DFSan-instrumented program multiple times. The +// alternative option is saving the entire TLS. However the TLS storage takes +// 2k bytes, and signal calls could be nested. So it does not seem worth. +class ScopedThreadLocalStateBackup { + public: + ScopedThreadLocalStateBackup() { Backup(); } + ~ScopedThreadLocalStateBackup() { Restore(); } + void Backup(); + void Restore(); +}; + } // namespace __dfsan #endif // DFSAN_H diff --git a/compiler-rt/lib/dfsan/dfsan.cpp b/compiler-rt/lib/dfsan/dfsan.cpp --- a/compiler-rt/lib/dfsan/dfsan.cpp +++ b/compiler-rt/lib/dfsan/dfsan.cpp @@ -456,6 +456,13 @@ if (common_flags()->help) parser.PrintFlagDescriptions(); } +void ScopedThreadLocalStateBackup::Backup() {} + +void ScopedThreadLocalStateBackup::Restore() { + internal_memset(__dfsan_arg_tls, 0, sizeof(__dfsan_arg_tls)); + internal_memset(__dfsan_retval_tls, 0, sizeof(__dfsan_retval_tls)); +} + static void InitializePlatformEarly() { AvoidCVE_2016_2143(); #ifdef DFSAN_RUNTIME_VMA diff --git a/compiler-rt/lib/dfsan/dfsan_custom.cpp b/compiler-rt/lib/dfsan/dfsan_custom.cpp --- a/compiler-rt/lib/dfsan/dfsan_custom.cpp +++ b/compiler-rt/lib/dfsan/dfsan_custom.cpp @@ -812,12 +812,80 @@ return ret; } +// sigactions_mu guarantees atomicity of sigaction() calls. +// Access to sigactions[] is gone with relaxed atomics to avoid data race with +// the signal handler. +const int kMaxSignals = 1024; +static atomic_uintptr_t sigactions[kMaxSignals]; +static StaticSpinMutex sigactions_mu; + +static void SignalHandler(int signo) { + ScopedThreadLocalStateBackup stlsb; + + // Clear shadows for all inputs provided by system. This is why DFSan + // instrumentation generates a trampoline function to each function pointer, + // and uses the trampoline to clear shadows. However sigaction does not use + // a function pointer directly, so we have to do this manually. + dfsan_set_label(0, &signo, sizeof(signo)); + + typedef void (*signal_cb)(int x); + signal_cb cb = + (signal_cb)atomic_load(&sigactions[signo], memory_order_relaxed); + cb(signo); +} + +static void SignalAction(int signo, siginfo_t *si, void *uc) { + ScopedThreadLocalStateBackup stlsb; + + // Clear shadows for all inputs provided by system. Similar to SignalHandler. + dfsan_set_label(0, &signo, sizeof(signo)); + dfsan_set_label(0, &si, sizeof(si)); + dfsan_set_label(0, &uc, sizeof(uc)); + dfsan_set_label(0, si, sizeof(__sanitizer_sigaction)); + dfsan_set_label(0, uc, __sanitizer::ucontext_t_sz); + + typedef void (*sigaction_cb)(int, siginfo_t *, void *); + sigaction_cb cb = + (sigaction_cb)atomic_load(&sigactions[signo], memory_order_relaxed); + cb(signo, si, uc); +} + SANITIZER_INTERFACE_ATTRIBUTE int __dfsw_sigaction(int signum, const struct sigaction *act, struct sigaction *oldact, dfsan_label signum_label, dfsan_label act_label, dfsan_label oldact_label, dfsan_label *ret_label) { - int ret = sigaction(signum, act, oldact); + SpinMutexLock lock(&sigactions_mu); + CHECK_LT(signum, kMaxSignals); + uptr old_cb = atomic_load(&sigactions[signum], memory_order_relaxed); + struct sigaction new_act; + struct sigaction *pnew_act = act ? &new_act : nullptr; + if (act) { + internal_memcpy(pnew_act, act, sizeof(struct sigaction)); + if (pnew_act->sa_flags & __sanitizer::sa_siginfo) { + uptr cb = (uptr)(pnew_act->sa_sigaction); + if (cb != __sanitizer::sig_ign && cb != __sanitizer::sig_dfl) { + atomic_store(&sigactions[signum], cb, memory_order_relaxed); + pnew_act->sa_sigaction = (decltype(pnew_act->sa_sigaction))SignalAction; + } + } else { + uptr cb = (uptr)(pnew_act->sa_handler); + if (cb != __sanitizer::sig_ign && cb != __sanitizer::sig_dfl) { + atomic_store(&sigactions[signum], cb, memory_order_relaxed); + pnew_act->sa_handler = (decltype(pnew_act->sa_handler))SignalHandler; + } + } + } + + int ret = sigaction(signum, pnew_act, oldact); + + if (ret == 0 && oldact) { + uptr cb = (uptr)oldact->sa_sigaction; + if (cb == (uptr)SignalAction || cb == (uptr)SignalHandler) { + oldact->sa_sigaction = (decltype(oldact->sa_sigaction))old_cb; + } + } + if (oldact) { dfsan_set_label(0, oldact, sizeof(struct sigaction)); } diff --git a/compiler-rt/test/dfsan/sigaction.c b/compiler-rt/test/dfsan/sigaction.c new file mode 100644 --- /dev/null +++ b/compiler-rt/test/dfsan/sigaction.c @@ -0,0 +1,49 @@ +// RUN: %clang_dfsan -DUSE_SIGNAL_ACTION -mllvm -dfsan-fast-16-labels=true %s -o %t && \ +// RUN: %run %t + +// RUN: %clang_dfsan -DUSE_SIGNAL_ACTION %s -o %t && %run %t + +// RUN: %clang_dfsan -mllvm -dfsan-fast-16-labels=true %s -o %t && %run %t + +// RUN: %clang_dfsan %s -o %t && %run %t + +#include + +#include +#include +#include +#include +#include + +volatile int x; +volatile int z = 1; + +void SignalHandler(int signo) { + x = z; +} + +void SignalAction(int signo, siginfo_t *si, void *uc) { + x = z; +} + +int main(int argc, char *argv[]) { + dfsan_set_label(8, (void *)&z, sizeof(z)); + + struct sigaction psa; +#ifdef USE_SIGNAL_ACTION + psa.sa_flags = SA_SIGINFO; + psa.sa_sigaction = SignalAction; +#else + psa.sa_flags = 0; + psa.sa_handler = SignalHandler; +#endif + int r = sigaction(SIGHUP, &psa, NULL); + assert(dfsan_get_label(r) == 0); + + kill(getpid(), SIGHUP); + signal(SIGHUP, SIG_DFL); + + assert(x == 1); + + return 0; +} diff --git a/compiler-rt/test/dfsan/sigaction_stress_test.c b/compiler-rt/test/dfsan/sigaction_stress_test.c new file mode 100644 --- /dev/null +++ b/compiler-rt/test/dfsan/sigaction_stress_test.c @@ -0,0 +1,65 @@ +// RUN: %clangxx_dfsan -mllvm -dfsan-fast-16-labels=true -O0 %s -o %t && %run %t +// RUN: %clangxx_dfsan -O0 %s -o %t && %run %t +// +// Test that the state of shadows from a sigaction handler are consistent. + +#include +#include +#include +#include +#include +#include + +const int kSigCnt = 200; +int x; + +__attribute__((noinline)) +int f(int a) { + return a; +} + +__attribute__((noinline)) +void g() { + int r = f(x); + const dfsan_label r_label = dfsan_get_label(r); + assert(r_label == 8 || r_label == 0); + return; +} + +int sigcnt; + +void SignalHandler(int signo) { + assert(signo == SIGPROF); + int a = 0; + dfsan_set_label(4, &a, sizeof(a)); + (void)f(a); + ++sigcnt; +} + +int main() { + struct sigaction psa; + psa.sa_flags = 0; + psa.sa_handler = SignalHandler; + int r = sigaction(SIGPROF, &psa, NULL); + + itimerval itv; + itv.it_interval.tv_sec = 0; + itv.it_interval.tv_usec = 100; + itv.it_value.tv_sec = 0; + itv.it_value.tv_usec = 100; + setitimer(ITIMER_PROF, &itv, NULL); + + dfsan_set_label(8, &x, sizeof(x)); + do { + g(); + } while (sigcnt < kSigCnt); + + itv.it_interval.tv_sec = 0; + itv.it_interval.tv_usec = 0; + itv.it_value.tv_sec = 0; + itv.it_value.tv_usec = 0; + setitimer(ITIMER_PROF, &itv, NULL); + + signal(SIGPROF, SIG_DFL); + return 0; +}