Merge "debuggerd: rethrow the full signal we receive, always."

2 files changed, 65 insertions, 21 deletions

diff --git a/linker/debugger.cpp b/linker/debugger.cpp
index b7e84acb5..d4c7928f5 100644
--- a/linker/debugger.cpp
+++ b/linker/debugger.cpp
@@ -39,6 +39,7 @@
 #include <sys/mman.h>
 #include <sys/prctl.h>
 #include <sys/socket.h>
+#include <sys/syscall.h>
 #include <sys/un.h>
 #include <unistd.h>
 
@@ -61,7 +62,9 @@ enum debugger_action_t {
   DEBUGGER_ACTION_DUMP_BACKTRACE,
 };
 
-/* message sent over the socket */
+// Message sent over the socket.
+// NOTE: Any changes to this structure must also be reflected in
+//       system/core/include/cutils/debugger.h.
 struct __attribute__((packed)) debugger_msg_t {
   int32_t action;
   pid_t tid;
@@ -267,26 +270,32 @@ static void debuggerd_signal_handler(int signal_number, siginfo_t* info, void*)
 
   send_debuggerd_packet(info);
 
-  // Remove our net so we fault for real when we return.
+  // We need to return from the signal handler so that debuggerd can dump the
+  // thread that crashed, but returning here does not guarantee that the signal
+  // will be thrown again, even for SIGSEGV and friends, since the signal could
+  // have been sent manually. Resend the signal with rt_tgsigqueueinfo(2) to
+  // preserve the SA_SIGINFO contents.
   signal(signal_number, SIG_DFL);
 
-  // These signals are not re-thrown when we resume.  This means that
-  // crashing due to (say) SIGABRT doesn't work the way you'd expect it
-  // to.  We work around this by throwing them manually.  We don't want
-  // to do this for *all* signals because it'll screw up the si_addr for
-  // faults like SIGSEGV. It does screw up the si_code, which is why we
-  // passed that to debuggerd above.
-  switch (signal_number) {
-    case SIGABRT:
-    case SIGFPE:
-#if defined(SIGSTKFLT)
-    case SIGSTKFLT:
-#endif
-    case SIGTRAP:
-      tgkill(getpid(), gettid(), signal_number);
-      break;
-    default:    // SIGILL, SIGBUS, SIGSEGV
-      break;
+  struct siginfo si;
+  if (!info) {
+    memset(&si, 0, sizeof(si));
+    si.si_code = SI_USER;
+    si.si_pid = getpid();
+    si.si_uid = getuid();
+    info = &si;
+  } else if (info->si_code >= 0 || info->si_code == SI_TKILL) {
+    // rt_tgsigqueueinfo(2)'s documentation appears to be incorrect on kernels
+    // that contain commit 66dd34a (3.9+). The manpage claims to only allow
+    // negative si_code values that are not SI_TKILL, but 66dd34a changed the
+    // check to allow all si_code values in calls coming from inside the house.
+  }
+
+  int rc = syscall(SYS_rt_tgsigqueueinfo, getpid(), gettid(), signal_number, info);
+  if (rc != 0) {
+    __libc_format_log(ANDROID_LOG_FATAL, "libc", "failed to resend signal during crash: %s",
+                      strerror(errno));
+    _exit(0);
   }
 }
 
diff --git a/tests/signal_test.cpp b/tests/signal_test.cpp
index f8fdc3f99..8c1e83469 100644
--- a/tests/signal_test.cpp
+++ b/tests/signal_test.cpp
@@ -14,12 +14,14 @@
  * limitations under the License.
  */
 
+#include <errno.h>
 #include <signal.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <unistd.h>
 
 #include <gtest/gtest.h>
 
-#include <errno.h>
-
 #include "ScopedSignalHandler.h"
 
 static size_t SIGNAL_MIN() {
@@ -375,3 +377,36 @@ TEST(signal, sigtimedwait_timeout) {
 
   ASSERT_EQ(0, sigprocmask(SIG_SETMASK, &original_set, NULL));
 }
+
+#if defined(__BIONIC__)
+TEST(signal, rt_tgsigqueueinfo) {
+  // Test whether rt_tgsigqueueinfo allows sending arbitrary si_code values to self.
+  // If this fails, your kernel needs commit 66dd34a to be backported.
+  static constexpr char error_msg[] =
+    "\nPlease ensure that the following kernel patch has been applied:\n"
+    "* https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=66dd34ad31e5963d72a700ec3f2449291d322921\n";
+  static siginfo received;
+
+  struct sigaction handler = {};
+  handler.sa_sigaction = [](int, siginfo_t* siginfo, void*) { received = *siginfo; };
+  handler.sa_flags = SA_SIGINFO;
+
+  ASSERT_EQ(0, sigaction(SIGUSR1, &handler, nullptr));
+
+  siginfo sent = {};
+
+  sent.si_code = SI_TKILL;
+  ASSERT_EQ(0, syscall(SYS_rt_tgsigqueueinfo, getpid(), gettid(), SIGUSR1, &sent))
+    << "rt_tgsigqueueinfo failed: " << strerror(errno) << error_msg;
+  ASSERT_EQ(sent.si_code, received.si_code) << "rt_tgsigqueueinfo modified si_code, expected "
+                                            << sent.si_code << ", received " << received.si_code
+                                            << error_msg;
+
+  sent.si_code = SI_USER;
+  ASSERT_EQ(0, syscall(SYS_rt_tgsigqueueinfo, getpid(), gettid(), SIGUSR1, &sent))
+    << "rt_tgsigqueueinfo failed: " << strerror(errno) << error_msg;
+  ASSERT_EQ(sent.si_code, received.si_code) << "rt_tgsigqueueinfo modified si_code, expected "
+                                            << sent.si_code << ", received " << received.si_code
+                                            << error_msg;
+}
+#endif