[llvm-branch-commits] [compiler-rt] [SafeStack] Intercept signal handlers (PR #196970)

[Jakob Koschel via llvm-branch-commits]

================
@@ -304,13 +378,72 @@ INTERCEPTOR(int, pthread_create, pthread_t *thread,
 }
 
 // We are intercepting sigaction in order to keep note of the set sigaction and
-// overwrite it our own function to execute the switching if the unsafe stack
-// pointer before and after the signal is handled.
-// In this version, we are simply making sure the interceptor is functional.
+// overwrite it with 'signal_handler_interceptor()/signal_action_interceptor()'
+// in order to execute custom before and after running the actual signal
+// handler (to switch to the sigalt_unsafe_stack and back). The interception is
+// only done for signal handlers that actually use the sigaltstack.
+// The code here, is largely inspired by the way MSan does intercept signal
+// handlers in compiler-rt/lib/msan/msan_interceptors.cpp.
 // sigaction is required to be async-signal-safe.
 INTERCEPTOR(int, sigaction, int sig, const struct sigaction* act,
             struct sigaction* oldact) {
-  return REAL(sigaction)(sig, act, oldact);
+  if (!act || !sigactions)
+    return REAL(sigaction)(sig, act, oldact);
+  if (!(act->sa_flags & SA_ONSTACK))
+    return REAL(sigaction)(sig, act, oldact);
+
+  int res;
+  sigactions_mu.Lock();
+
+  void* old_cb = (void*)atomic_load(&sigactions[sig], memory_order_relaxed);
+  struct sigaction new_act;
+  struct sigaction* pnew_act = &new_act;
+  memcpy(pnew_act, act, sizeof(struct sigaction));
+  uptr cb;
+  uptr new_cb;
+
+  // We first fetch the original sigaction/handler passed to sigaction.
+  if (pnew_act->sa_flags & SA_SIGINFO) {
+    cb = (uptr)pnew_act->sa_sigaction;
+    new_cb = (uptr)signal_action_interceptor;
+  } else {
+    cb = (uptr)pnew_act->sa_handler;
+    new_cb = (uptr)signal_handler_interceptor;
+  }
+
+  if (cb != (uptr)SIG_IGN && cb != (uptr)SIG_DFL) {
+    // We keep sigactions mapped without write permissions to avoid an arbitrary
+    // write trivially corrupting a signal handler pointer.
----------------
jakos-sec wrote:

With 'native' non-safestack `sigaction` is just a thin wrapper around the rt_sigaction syscall. So those code pointers are actually managed and stored in the kernel and by default not writable from user space. Since for the unsafe component the kernel has no support for it, we are managing it ourselves and should ideally not introduce more trivial writeable code pointers. WDYT?

https://github.com/llvm/llvm-project/pull/196970