msan: account for AVX state when unpoison ucontext_t
authorDmitry Vyukov <dvyukov@google.com>
Thu, 23 Dec 2021 08:06:37 +0000 (09:06 +0100)
committerDmitry Vyukov <dvyukov@google.com>
Wed, 5 Jan 2022 12:20:40 +0000 (13:20 +0100)
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
compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_freebsd.cpp
compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_freebsd.h
compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_netbsd.cpp
compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_netbsd.h
compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp
compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h
compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_solaris.cpp
compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_solaris.h
compiler-rt/lib/sanitizer_common/sanitizer_syscalls_netbsd.inc
compiler-rt/test/msan/Linux/signal_mcontext.cpp [new file with mode: 0644]

index df0cdec..63887e4 100644 (file)
@@ -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 =
index 6453580..0d25fa8 100644 (file)
@@ -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);
index 649e64f..9859c52 100644 (file)
@@ -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;
index 531e07f..648e502 100644 (file)
@@ -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);
index 9407803..dc6eb59 100644 (file)
@@ -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;
index a1c4528..0ffbb81 100644 (file)
@@ -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<ucontext_t *>(ctx)->uc_mcontext.fpregs;
+    if (fpregs->__glibc_reserved1[12] == FP_XSTATE_MAGIC1)
+      return reinterpret_cast<const char *>(fpregs) +
+             fpregs->__glibc_reserved1[13] - static_cast<const char *>(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 =
index d69b344..4472b6e 100644 (file)
@@ -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__)
index a113cb0..dad7bde 100644 (file)
@@ -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);
index cbab577..84a8126 100644 (file)
@@ -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;
index c4a9d99..4ce5de0 100644 (file)
@@ -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 (file)
index 0000000..f1184a4
--- /dev/null
@@ -0,0 +1,38 @@
+// RUN: %clangxx_msan -fsanitize-memory-track-origins=2 -O1 %s -o %t && %run %t 2>&1 | FileCheck %s
+
+#include <pthread.h>
+#include <sanitizer/msan_interface.h>
+#include <signal.h>
+#include <stdio.h>
+#include <ucontext.h>
+#include <unistd.h>
+
+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<ucontext_t *>(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: