From ba40ff657f841c685d220ebdecbe8ba2c2ab1a8a Mon Sep 17 00:00:00 2001
From: Josh Gao <jmgao@google.com>
Date: Tue, 22 Jan 2019 22:53:49 -0800
Subject: [PATCH] Don't filter reserved signals in sigaction.

If a signal handler is blocking all of their signals, we should
probably respect that and not silently unblock bionic's reserved
signals for them. Otherwise, user code can deadlock, run out of stack,
etc. through no fault of their own, if one of the reserved signals
comes in while they've pivoted onto their signal stack.

Bug: http://b/122939726
Test: treehugger
Change-Id: I6425a3e7413edc16157b35dffe632e1ab1d76618
---
 libc/bionic/sigaction.cpp |  7 +++++--
 tests/signal_test.cpp     | 23 +++++++++++++++++++----
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/libc/bionic/sigaction.cpp b/libc/bionic/sigaction.cpp
index 42dcccdf6..96e6f3c31 100644
--- a/libc/bionic/sigaction.cpp
+++ b/libc/bionic/sigaction.cpp
@@ -43,7 +43,8 @@ int sigaction(int signal, const struct sigaction* bionic_new_action, struct siga
   if (bionic_new_action != nullptr) {
     kernel_new_action.sa_flags = bionic_new_action->sa_flags;
     kernel_new_action.sa_handler = bionic_new_action->sa_handler;
-    kernel_new_action.sa_mask = filter_reserved_signals(bionic_new_action->sa_mask, SIG_SETMASK);
+    // Don't filter signals here; if the caller asked for everything to be blocked, we should obey.
+    kernel_new_action.sa_mask = bionic_new_action->sa_mask;
 #if defined(SA_RESTORER)
     kernel_new_action.sa_restorer = bionic_new_action->sa_restorer;
 #if defined(__aarch64__)
@@ -95,6 +96,7 @@ int sigaction(int signal, const struct sigaction* bionic_new, struct sigaction*
 #if defined(SA_RESTORER)
     kernel_new.sa_restorer = bionic_new->sa_restorer;
 #endif
+    // Don't filter signals here; if the caller asked for everything to be blocked, we should obey.
     memcpy(&kernel_new.sa_mask, &bionic_new->sa_mask, sizeof(bionic_new->sa_mask));
   }
 
@@ -122,7 +124,8 @@ int sigaction64(int signal, const struct sigaction64* bionic_new, struct sigacti
       kernel_new.sa_restorer = (kernel_new.sa_flags & SA_SIGINFO) ? &__restore_rt : &__restore;
     }
 #endif
-    kernel_new.sa_mask = filter_reserved_signals(kernel_new.sa_mask, SIG_SETMASK);
+    // Don't filter signals here; if the caller asked for everything to be blocked, we should obey.
+    kernel_new.sa_mask = kernel_new.sa_mask;
   }
 
   return __rt_sigaction(signal,
diff --git a/tests/signal_test.cpp b/tests/signal_test.cpp
index dd27aef33..77b004ff3 100644
--- a/tests/signal_test.cpp
+++ b/tests/signal_test.cpp
@@ -392,11 +392,19 @@ TEST(signal, sigaction_filter) {
   static uint64_t sigset;
   struct sigaction sa = {};
   sa.sa_handler = [](int) { sigset = GetSignalMask(); };
+  sa.sa_flags = SA_ONSTACK | SA_NODEFER;
   sigfillset(&sa.sa_mask);
   sigaction(SIGUSR1, &sa, nullptr);
   raise(SIGUSR1);
-  ASSERT_NE(0ULL, sigset);
-  TestSignalMaskFiltered(sigset);
+
+  // On LP32, struct sigaction::sa_mask is only 32-bits wide.
+  unsigned long expected_sigset = ~0UL;
+
+  // SIGKILL and SIGSTOP are always blocked.
+  expected_sigset &= ~(1UL << (SIGKILL - 1));
+  expected_sigset &= ~(1UL << (SIGSTOP - 1));
+
+  ASSERT_EQ(static_cast<uint64_t>(expected_sigset), sigset);
 }
 
 TEST(signal, sigaction64_filter) {
@@ -404,11 +412,18 @@ TEST(signal, sigaction64_filter) {
   static uint64_t sigset;
   struct sigaction64 sa = {};
   sa.sa_handler = [](int) { sigset = GetSignalMask(); };
+  sa.sa_flags = SA_ONSTACK | SA_NODEFER;
   sigfillset64(&sa.sa_mask);
   sigaction64(SIGUSR1, &sa, nullptr);
   raise(SIGUSR1);
-  ASSERT_NE(0ULL, sigset);
-  TestSignalMaskFiltered(sigset);
+
+  uint64_t expected_sigset = ~0ULL;
+
+  // SIGKILL and SIGSTOP are always blocked.
+  expected_sigset &= ~(1ULL << (SIGKILL - 1));
+  expected_sigset &= ~(1ULL << (SIGSTOP - 1));
+
+  ASSERT_EQ(expected_sigset, sigset);
 }
 
 TEST(signal, sigprocmask_setmask_filter) {