[compiler-rt] implement sigaltstack interception

[sugak]

Summary

An implementation for sigaltstack to make its side effect be visible to MSAN.

ninja check-msan

[sugak]

I manually tested this on CentOS and would need guidance about the use of SI_POSIX, as well as how to make a unittest for this (if needed).

[eugenis]

In D73816#1852789, @sugak wrote:

I manually tested this on CentOS and would need guidance about the use of SI_POSIX, as well as how to make a unittest for this (if needed).

SI_POSIX seems alright. Please add a simple test under compiler-rt/test/msan - just call the function and check that accessing memory does not trigger msan.

compiler-rt/lib/sanitizer_common/sanitizer_common_syscalls.inc

2901

POST_READ is unnecessary here - it is mainly for the case when the read range is not known until the syscall returns.

[sugak]

Thank you @eugenis! I've updated the change with you feedback and added a test.

[eugenis]

LGTM
Thank you!

[sugak]

@eugenis, thank you for the review! Would you please commit this for me?

[mgorny]

This seems to have broken NetBSD:

FAILED: projects/compiler-rt/lib/tsan/CMakeFiles/clang_rt.tsan-x86_64.dir/rtl/tsan_interceptors_posix.cpp.o 
/home/motus/netbsd8/netbsd8/wrappers/clang++  -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Iprojects/compiler-rt/lib/tsan -I/home/motus/netbsd8/netbsd8/llvm-project/llvm/projects/compiler-rt/lib/tsan -Iinclude -I/home/motus/netbsd8/netbsd8/llvm-project/llvm/include -I/home/motus/netbsd8/netbsd8/llvm-project/llvm/projects/compiler-rt/lib/tsan/.. -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -Wall -std=c++14 -Wno-unused-parameter -O3 -DNDEBUG    -m64 -fno-lto -fPIC -fno-builtin -fno-exceptions -fomit-frame-pointer -funwind-tables -fno-stack-protector -fno-sanitize=safe-stack -fvisibility=hidden -fno-lto -O3 -gline-tables-only -Wno-gnu -Wno-variadic-macros -Wno-c99-extensions -Wno-non-virtual-dtor -fPIE -fno-rtti -msse3 -Wframe-larger-than=530 -Wglobal-constructors -std=c++14 -MD -MT projects/compiler-rt/lib/tsan/CMakeFiles/clang_rt.tsan-x86_64.dir/rtl/tsan_interceptors_posix.cpp.o -MF projects/compiler-rt/lib/tsan/CMakeFiles/clang_rt.tsan-x86_64.dir/rtl/tsan_interceptors_posix.cpp.o.d -o projects/compiler-rt/lib/tsan/CMakeFiles/clang_rt.tsan-x86_64.dir/rtl/tsan_interceptors_posix.cpp.o -c /home/motus/netbsd8/netbsd8/llvm-project/llvm/projects/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
In file included from /home/motus/netbsd8/netbsd8/llvm-project/llvm/projects/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:2337:
/home/motus/netbsd8/netbsd8/llvm-project/llvm/projects/compiler-rt/lib/tsan/../sanitizer_common/sanitizer_common_interceptors.inc:9739:44: error: use of undeclared identifier 'struct_stack_t_sz'; did you mean 'struct_stat_sz'?
    COMMON_INTERCEPTOR_READ_RANGE(ctx, ss, struct_stack_t_sz);
                                           ^~~~~~~~~~~~~~~~~
                                           struct_stat_sz
/home/motus/netbsd8/netbsd8/llvm-project/llvm/projects/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:2223:71: note: expanded from macro 'COMMON_INTERCEPTOR_READ_RANGE'
                    ((TsanInterceptorContext *) ctx)->pc, (uptr) ptr, size, \
                                                                      ^
/home/motus/netbsd8/netbsd8/llvm-project/llvm/projects/compiler-rt/lib/tsan/../sanitizer_common/sanitizer_platform_limits_netbsd.h:35:17: note: 'struct_stat_sz' declared here
extern unsigned struct_stat_sz;
                ^

http://lab.llvm.org:8014/builders/netbsd-amd64/builds/968/steps/ninja%20build%20local/logs/stdio

[krytarowski]

For fixing NetBSD, remember to go for #define sigaltstack __sigaltstack14 as the symbol is renamed.

[sugak]

Sorry for breaking NetBSD builds! An attempt to fix it in D73976.

[eugenis]

Another problem with this interceptor - stack_t on linux has internal padding that is not required to be initialized.

See this bug: https://bugs.chromium.org/p/chromium/issues/detail?id=1050279

[sugak]

@eugenis: would you want me to send a patch for it or are you looking at this?

[eugenis]

I'm not looking into it. Could you?

[sugak]

Yes, I'll fix this!

[eugenis]

I've removed the check in https://github.com/llvm/llvm-project/commit/987f153929e813db1ea7e3354fd7a0b6b4adf4c0

compiler-rt/include/sanitizer/linux_syscall_hooks.h

#define __sanitizer_syscall_post_sigaction(res, signum, act, oldact)           \
   __sanitizer_syscall_post_impl_sigaction(res, (long)signum, (long)act,        \
                                           (long)oldact)
 #define __sanitizer_syscall_pre_rt_sigaction(signum, act, oldact, sz)          \
   __sanitizer_syscall_pre_impl_rt_sigaction((long)signum, (long)act,           \
                                             (long)oldact, (long)sz)
 #define __sanitizer_syscall_post_rt_sigaction(res, signum, act, oldact, sz)    \
   __sanitizer_syscall_post_impl_rt_sigaction(res, (long)signum, (long)act,     \
                                              (long)oldact, (long)sz)
+#define __sanitizer_syscall_pre_sigaltstack(ss, oss)                           \
+  __sanitizer_syscall_pre_impl_sigaltstack((long)ss, (long)oss)
+#define __sanitizer_syscall_post_sigaltstack(res, ss, oss)                     \
+  __sanitizer_syscall_post_impl_sigaltstack(res, (long)ss, (long)oss)
 
 // And now a few syscalls we don't handle yet.
 #define __sanitizer_syscall_pre_afs_syscall(...)
 #define __sanitizer_syscall_pre_arch_prctl(...)
 #define __sanitizer_syscall_pre_break(...)
 #define __sanitizer_syscall_pre_chown32(...)
 #define __sanitizer_syscall_pre_clone(...)
 #define __sanitizer_syscall_pre_create_module(...)

 #define __sanitizer_syscall_pre_setgid32(...)
 #define __sanitizer_syscall_pre_setgroups32(...)
 #define __sanitizer_syscall_pre_setregid32(...)
 #define __sanitizer_syscall_pre_setresgid32(...)
 #define __sanitizer_syscall_pre_setresuid32(...)
 #define __sanitizer_syscall_pre_setreuid32(...)
 #define __sanitizer_syscall_pre_set_thread_area(...)
 #define __sanitizer_syscall_pre_setuid32(...)
-#define __sanitizer_syscall_pre_sigaltstack(...)
 #define __sanitizer_syscall_pre_sigreturn(...)
 #define __sanitizer_syscall_pre_sigsuspend(...)
 #define __sanitizer_syscall_pre_stty(...)
 #define __sanitizer_syscall_pre_sync_file_range(...)
 #define __sanitizer_syscall_pre__sysctl(...)
 #define __sanitizer_syscall_pre_truncate64(...)
 #define __sanitizer_syscall_pre_tuxcall(...)
 #define __sanitizer_syscall_pre_ugetrlimit(...)

 #define __sanitizer_syscall_post_setgid32(res, ...)
 #define __sanitizer_syscall_post_setgroups32(res, ...)
 #define __sanitizer_syscall_post_setregid32(res, ...)
 #define __sanitizer_syscall_post_setresgid32(res, ...)
 #define __sanitizer_syscall_post_setresuid32(res, ...)
 #define __sanitizer_syscall_post_setreuid32(res, ...)
 #define __sanitizer_syscall_post_set_thread_area(res, ...)
 #define __sanitizer_syscall_post_setuid32(res, ...)
-#define __sanitizer_syscall_post_sigaltstack(res, ...)
 #define __sanitizer_syscall_post_sigreturn(res, ...)
 #define __sanitizer_syscall_post_sigsuspend(res, ...)
 #define __sanitizer_syscall_post_stty(res, ...)
 #define __sanitizer_syscall_post_sync_file_range(res, ...)
 #define __sanitizer_syscall_post__sysctl(res, ...)
 #define __sanitizer_syscall_post_truncate64(res, ...)
 #define __sanitizer_syscall_post_tuxcall(res, ...)
 #define __sanitizer_syscall_post_ugetrlimit(res, ...)

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

INTERCEPTOR(void, qsort_r, void *base, SIZE_T nmemb, SIZE_T size,
   qsort_r_size = old_size;
   COMMON_INTERCEPTOR_WRITE_RANGE(ctx, base, nmemb * size);
 }
 #define INIT_QSORT_R COMMON_INTERCEPT_FUNCTION(qsort_r)
 #else
 #define INIT_QSORT_R
 #endif
 
+#if SANITIZER_INTERCEPT_SIGALTSTACK
+INTERCEPTOR(int, sigaltstack, void *ss, void *oss) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, sigaltstack, ss, oss);
+  if (ss != nullptr) {
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, ss, struct_stack_t_sz);
+  }
+  int r = REAL(sigaltstack)(ss, oss);
+  if (r == 0 && oss != nullptr) {
+    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, oss, struct_stack_t_sz);
+  }
+  return r;
+}
+#define INIT_SIGALTSTACK COMMON_INTERCEPT_FUNCTION(sigaltstack)
+#else
+#define INIT_SIGALTSTACK
+#endif
+
 #include "sanitizer_common_interceptors_netbsd_compat.inc"
 
 static void InitializeCommonInterceptors() {
 #if SI_POSIX
   static u64 metadata_mem[sizeof(MetadataHashMap) / sizeof(u64) + 1];
   interceptor_metadata_map = new ((void *)&metadata_mem) MetadataHashMap();
 #endif
 
#endif
   INIT_GETUSERSHELL;
   INIT_SL_INIT;
   INIT_GETRANDOM;
   INIT_CRYPT;
   INIT_CRYPT_R;
   INIT_GETENTROPY;
   INIT_QSORT;
   INIT_QSORT_R;
+  INIT_SIGALTSTACK;
 
   INIT___PRINTF_CHK;
 }

compiler-rt/lib/sanitizer_common/sanitizer_common_syscalls.inc

PRE_SYSCALL(getrandom)(void *buf, uptr count, long flags) {
   }
 }
 
 POST_SYSCALL(getrandom)(long res, void *buf, uptr count, long flags) {
   if (res > 0 && buf) {
     POST_WRITE(buf, res);
   }
 }
+
+PRE_SYSCALL(sigaltstack)(const void *ss, void *oss) {
+  if (ss != nullptr) {
+    PRE_READ(ss, struct_stack_t_sz);
+  }
+  if (oss != nullptr) {
+    PRE_WRITE(oss, struct_stack_t_sz);
+  }
+}
+
+POST_SYSCALL(sigaltstack)(long res, void *ss, void *oss) {
+  if (res == 0) {
+    if (oss != nullptr) {
+      POST_WRITE(oss, struct_stack_t_sz);
+    }
+  }
+}
 }  // extern "C"
 
 #undef PRE_SYSCALL
 #undef PRE_READ
 #undef PRE_WRITE
 #undef POST_SYSCALL
 #undef POST_READ
 #undef POST_WRITE
 
 #endif  // SANITIZER_LINUX

compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h

#define SANITIZER_INTERCEPT_GETRANDOM \
   ((SI_LINUX && __GLIBC_PREREQ(2, 25)) || SI_FREEBSD)
 #define SANITIZER_INTERCEPT___CXA_ATEXIT SI_NETBSD
 #define SANITIZER_INTERCEPT_ATEXIT SI_NETBSD
 #define SANITIZER_INTERCEPT_PTHREAD_ATFORK SI_NETBSD
 #define SANITIZER_INTERCEPT_GETENTROPY SI_FREEBSD
 #define SANITIZER_INTERCEPT_QSORT \
   (SI_POSIX && !SI_IOSSIM && !SI_WATCHOS && !SI_TVOS)
 #define SANITIZER_INTERCEPT_QSORT_R (SI_LINUX && !SI_ANDROID)
+#define SANITIZER_INTERCEPT_SIGALTSTACK SI_POSIX
 
 #endif  // #ifndef SANITIZER_PLATFORM_INTERCEPTORS_H

compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h

 extern unsigned timeval_sz;
 extern unsigned uid_t_sz;
 extern unsigned gid_t_sz;
 extern unsigned mbstate_t_sz;
 extern unsigned struct_timezone_sz;
 extern unsigned struct_tms_sz;
 extern unsigned struct_itimerspec_sz;
 extern unsigned struct_sigevent_sz;
+extern unsigned struct_stack_t_sz;
 extern unsigned struct_sched_param_sz;
 extern unsigned struct_statfs64_sz;
 extern unsigned struct_regex_sz;
 extern unsigned struct_regmatch_sz;
 
 #if !SANITIZER_ANDROID
 extern unsigned struct_fstab_sz;
 extern unsigned struct_statfs_sz;

compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp

#if !SANITIZER_IOS
   unsigned struct_stat64_sz = sizeof(struct stat64);
 #endif // !SANITIZER_IOS
   unsigned struct_rusage_sz = sizeof(struct rusage);
   unsigned struct_tm_sz = sizeof(struct tm);
   unsigned struct_passwd_sz = sizeof(struct passwd);
   unsigned struct_group_sz = sizeof(struct group);
   unsigned siginfo_t_sz = sizeof(siginfo_t);
   unsigned struct_sigaction_sz = sizeof(struct sigaction);
+  unsigned struct_stack_t_sz = sizeof(stack_t);
   unsigned struct_itimerval_sz = sizeof(struct itimerval);
   unsigned pthread_t_sz = sizeof(pthread_t);
   unsigned pthread_mutex_t_sz = sizeof(pthread_mutex_t);
   unsigned pthread_cond_t_sz = sizeof(pthread_cond_t);
   unsigned pid_t_sz = sizeof(pid_t);
   unsigned timeval_sz = sizeof(timeval);
   unsigned uid_t_sz = sizeof(uid_t);
   unsigned gid_t_sz = sizeof(gid_t);

compiler-rt/test/msan/sigaltstack.cpp

+// RUN: %clangxx_msan -O0 -g %s -o %t && not %run %t
+//
+#include <signal.h>
+#include <assert.h>
+
+#include <sanitizer/msan_interface.h>
+
+int main(void) {
+  stack_t old_ss;
+
+  assert(sigaltstack(nullptr, &old_ss) == 0);
+  __msan_check_mem_is_initialized(&old_ss, sizeof(stack_t));
+
+  stack_t ss;
+  sigaltstack(&ss, nullptr);
+// CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value
+// CHECK:    in main {{.*}}sigaltstack.cpp:15
+
+  return 0;
+}