From 1298273e8206a8fc28369c1ac8dc71a0c9b3851e Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Thu, 23 Dec 2021 09:06:37 +0100 Subject: [PATCH] msan: account for AVX state when unpoison ucontext_t ucontext_t can be larger than its static size if it contains AVX state and YMM/ZMM registers. Currently a signal handler that tries to access that state can produce false positives with random origins on stack. Account for the additional ucontext_t state. Reviewed By: vitalybuka Differential Revision: https://reviews.llvm.org/D116208 --- compiler-rt/lib/msan/msan_interceptors.cpp | 2 +- .../sanitizer_platform_limits_freebsd.cpp | 2 +- .../sanitizer_platform_limits_freebsd.h | 2 +- .../sanitizer_platform_limits_netbsd.cpp | 2 +- .../sanitizer_platform_limits_netbsd.h | 2 +- .../sanitizer_platform_limits_posix.cpp | 21 ++++++++++-- .../sanitizer_platform_limits_posix.h | 8 ++--- .../sanitizer_platform_limits_solaris.cpp | 2 +- .../sanitizer_platform_limits_solaris.h | 2 +- .../sanitizer_common/sanitizer_syscalls_netbsd.inc | 4 +-- compiler-rt/test/msan/Linux/signal_mcontext.cpp | 38 ++++++++++++++++++++++ 11 files changed, 69 insertions(+), 16 deletions(-) create mode 100644 compiler-rt/test/msan/Linux/signal_mcontext.cpp diff --git a/compiler-rt/lib/msan/msan_interceptors.cpp b/compiler-rt/lib/msan/msan_interceptors.cpp index df0cdec..63887e4 100644 --- a/compiler-rt/lib/msan/msan_interceptors.cpp +++ b/compiler-rt/lib/msan/msan_interceptors.cpp @@ -990,7 +990,7 @@ static void SignalAction(int signo, void *si, void *uc) { ScopedThreadLocalStateBackup stlsb; UnpoisonParam(3); __msan_unpoison(si, sizeof(__sanitizer_sigaction)); - __msan_unpoison(uc, __sanitizer::ucontext_t_sz); + __msan_unpoison(uc, ucontext_t_sz(uc)); typedef void (*sigaction_cb)(int, void *, void *); sigaction_cb cb = diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_freebsd.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_freebsd.cpp index 6453580..0d25fa8 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_freebsd.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_freebsd.cpp @@ -130,7 +130,7 @@ unsigned struct_sigevent_sz = sizeof(struct sigevent); unsigned struct_sched_param_sz = sizeof(struct sched_param); unsigned struct_statfs_sz = sizeof(struct statfs); unsigned struct_sockaddr_sz = sizeof(struct sockaddr); -unsigned ucontext_t_sz = sizeof(ucontext_t); +unsigned ucontext_t_sz(void *ctx) { return sizeof(ucontext_t); } unsigned struct_rlimit_sz = sizeof(struct rlimit); unsigned struct_timespec_sz = sizeof(struct timespec); unsigned struct_utimbuf_sz = sizeof(struct utimbuf); diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_freebsd.h b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_freebsd.h index 649e64f..9859c52 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_freebsd.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_freebsd.h @@ -57,7 +57,7 @@ extern unsigned struct_sched_param_sz; extern unsigned struct_statfs64_sz; extern unsigned struct_statfs_sz; extern unsigned struct_sockaddr_sz; -extern unsigned ucontext_t_sz; +unsigned ucontext_t_sz(void *ctx); extern unsigned struct_rlimit_sz; extern unsigned struct_utimbuf_sz; extern unsigned struct_timespec_sz; diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_netbsd.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_netbsd.cpp index 531e07f..648e502 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_netbsd.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_netbsd.cpp @@ -554,7 +554,7 @@ unsigned struct_tms_sz = sizeof(struct tms); unsigned struct_sigevent_sz = sizeof(struct sigevent); unsigned struct_sched_param_sz = sizeof(struct sched_param); unsigned struct_sockaddr_sz = sizeof(struct sockaddr); -unsigned ucontext_t_sz = sizeof(ucontext_t); +unsigned ucontext_t_sz(void *ctx) { return sizeof(ucontext_t); } unsigned struct_rlimit_sz = sizeof(struct rlimit); unsigned struct_timespec_sz = sizeof(struct timespec); unsigned struct_sembuf_sz = sizeof(struct sembuf); diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_netbsd.h b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_netbsd.h index 9407803..dc6eb59 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_netbsd.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_netbsd.h @@ -45,7 +45,7 @@ extern unsigned struct_stack_t_sz; extern unsigned struct_sched_param_sz; extern unsigned struct_statfs_sz; extern unsigned struct_sockaddr_sz; -extern unsigned ucontext_t_sz; +unsigned ucontext_t_sz(void *ctx); extern unsigned struct_rlimit_sz; extern unsigned struct_utimbuf_sz; diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp index a1c4528..0ffbb81 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp @@ -173,6 +173,11 @@ typedef struct user_fpregs elf_fpregset_t; #include "sanitizer_internal_defs.h" #include "sanitizer_platform_limits_posix.h" +// To prevent macro redefinition warning between our sanitizer_thread_safety.h +// and system's scsi.h. +# undef RELEASE +# include "sanitizer_common.h" + namespace __sanitizer { unsigned struct_utsname_sz = sizeof(struct utsname); unsigned struct_stat_sz = sizeof(struct stat); @@ -214,10 +219,20 @@ namespace __sanitizer { #if !SANITIZER_ANDROID unsigned struct_statfs_sz = sizeof(struct statfs); unsigned struct_sockaddr_sz = sizeof(struct sockaddr); - unsigned ucontext_t_sz = sizeof(ucontext_t); -#endif // !SANITIZER_ANDROID -#if SANITIZER_LINUX + unsigned ucontext_t_sz(void *ctx) { +# if SANITIZER_LINUX && SANITIZER_X64 + // See kernel arch/x86/kernel/fpu/signal.c for details. + const auto *fpregs = static_cast(ctx)->uc_mcontext.fpregs; + if (fpregs->__glibc_reserved1[12] == FP_XSTATE_MAGIC1) + return reinterpret_cast(fpregs) + + fpregs->__glibc_reserved1[13] - static_cast(ctx); +# endif + return sizeof(ucontext_t); + } +# endif // !SANITIZER_ANDROID + +# if SANITIZER_LINUX unsigned struct_epoll_event_sz = sizeof(struct epoll_event); unsigned struct_sysinfo_sz = sizeof(struct sysinfo); unsigned __user_cap_header_struct_sz = diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h index d69b344..4472b6e 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h @@ -57,12 +57,12 @@ extern unsigned struct_regmatch_sz; extern unsigned struct_fstab_sz; extern unsigned struct_statfs_sz; extern unsigned struct_sockaddr_sz; -extern unsigned ucontext_t_sz; -#endif // !SANITIZER_ANDROID +unsigned ucontext_t_sz(void *uctx); +# endif // !SANITIZER_ANDROID -#if SANITIZER_LINUX +# if SANITIZER_LINUX -#if defined(__x86_64__) +# if defined(__x86_64__) const unsigned struct_kernel_stat_sz = 144; const unsigned struct_kernel_stat64_sz = 0; #elif defined(__i386__) diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_solaris.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_solaris.cpp index a113cb0..dad7bde 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_solaris.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_solaris.cpp @@ -89,7 +89,7 @@ namespace __sanitizer { unsigned struct_sched_param_sz = sizeof(struct sched_param); unsigned struct_statfs_sz = sizeof(struct statfs); unsigned struct_sockaddr_sz = sizeof(struct sockaddr); - unsigned ucontext_t_sz = sizeof(ucontext_t); + unsigned ucontext_t_sz(void *ctx) { return sizeof(ucontext_t); } unsigned struct_timespec_sz = sizeof(struct timespec); #if SANITIZER_SOLARIS32 unsigned struct_statvfs64_sz = sizeof(struct statvfs64); diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_solaris.h b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_solaris.h index cbab577..84a8126 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_solaris.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_solaris.h @@ -43,7 +43,7 @@ extern unsigned struct_sched_param_sz; extern unsigned struct_statfs64_sz; extern unsigned struct_statfs_sz; extern unsigned struct_sockaddr_sz; -extern unsigned ucontext_t_sz; +unsigned ucontext_t_sz(void *ctx); extern unsigned struct_timespec_sz; extern unsigned struct_rlimit_sz; diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_syscalls_netbsd.inc b/compiler-rt/lib/sanitizer_common/sanitizer_syscalls_netbsd.inc index c4a9d99f..4ce5de0 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_syscalls_netbsd.inc +++ b/compiler-rt/lib/sanitizer_common/sanitizer_syscalls_netbsd.inc @@ -2255,13 +2255,13 @@ PRE_SYSCALL(getcontext)(void *ucp_) { /* Nothing to do */ } POST_SYSCALL(getcontext)(long long res, void *ucp_) { /* Nothing to do */ } PRE_SYSCALL(setcontext)(void *ucp_) { if (ucp_) { - PRE_READ(ucp_, ucontext_t_sz); + PRE_READ(ucp_, ucontext_t_sz(ucp_)); } } POST_SYSCALL(setcontext)(long long res, void *ucp_) {} PRE_SYSCALL(_lwp_create)(void *ucp_, long long flags_, void *new_lwp_) { if (ucp_) { - PRE_READ(ucp_, ucontext_t_sz); + PRE_READ(ucp_, ucontext_t_sz(ucp_)); } } POST_SYSCALL(_lwp_create) diff --git a/compiler-rt/test/msan/Linux/signal_mcontext.cpp b/compiler-rt/test/msan/Linux/signal_mcontext.cpp new file mode 100644 index 0000000..f1184a4 --- /dev/null +++ b/compiler-rt/test/msan/Linux/signal_mcontext.cpp @@ -0,0 +1,38 @@ +// RUN: %clangxx_msan -fsanitize-memory-track-origins=2 -O1 %s -o %t && %run %t 2>&1 | FileCheck %s + +#include +#include +#include +#include +#include +#include + +void handler(int sig, siginfo_t *info, void *uctx) { + __msan_check_mem_is_initialized(uctx, sizeof(ucontext_t)); +#if defined(__x86_64__) + auto *mctx = &static_cast(uctx)->uc_mcontext; + if (auto *fpregs = mctx->fpregs) { + if (fpregs->__glibc_reserved1[12] == FP_XSTATE_MAGIC1) { + auto *xstate = reinterpret_cast<_xstate *>(mctx->fpregs); + __msan_check_mem_is_initialized(xstate, sizeof(*xstate)); + } + } +#endif +} + +__attribute__((noinline)) void poison_stack() { + char buf[64 << 10]; + printf("buf: %p-%p\n", buf, buf + sizeof(buf)); +} + +int main(int argc, char **argv) { + struct sigaction act = {}; + act.sa_sigaction = handler; + act.sa_flags = SA_SIGINFO; + sigaction(SIGPROF, &act, 0); + poison_stack(); + pthread_kill(pthread_self(), SIGPROF); + return 0; +} + +// CHECK-NOT: WARNING: MemorySanitizer: -- 2.7.4