Fix multithreaded backtraces for seccomp processes.

Add threads to the existing seccomp backtrace test to prevent
regressing this.

Bug: http://b/114139908
Bug: http://b/115349586
Test: debuggerd_test32
Test: debuggerd_test64
Change-Id: I07fbe1619b60f0008deb045a249f9045404478c2

debuggerd/debuggerd_test.cpp

@@ -587,9 +587,28 @@ static const char* const kDebuggerdSeccompPolicy =
     "/system/etc/seccomp_policy/crash_dump." ABI_STRING ".policy";
 
 static pid_t seccomp_fork_impl(void (*prejail)()) {
-  unique_fd policy_fd(open(kDebuggerdSeccompPolicy, O_RDONLY | O_CLOEXEC));
-  if (policy_fd == -1) {
-    LOG(FATAL) << "failed to open policy " << kDebuggerdSeccompPolicy;
+  std::string policy;
+  if (!android::base::ReadFileToString(kDebuggerdSeccompPolicy, &policy)) {
+    PLOG(FATAL) << "failed to read policy file";
+  }
+
+  // Allow a bunch of syscalls used by the tests.
+  policy += "\nclone: 1";
+  policy += "\nsigaltstack: 1";
+  policy += "\nnanosleep: 1";
+
+  FILE* tmp_file = tmpfile();
+  if (!tmp_file) {
+    PLOG(FATAL) << "tmpfile failed";
+  }
+
+  unique_fd tmp_fd(dup(fileno(tmp_file)));
+  if (!android::base::WriteStringToFd(policy, tmp_fd.get())) {
+    PLOG(FATAL) << "failed to write policy to tmpfile";
+  }
+
+  if (lseek(tmp_fd.get(), 0, SEEK_SET) != 0) {
+    PLOG(FATAL) << "failed to seek tmp_fd";
   }
 
   ScopedMinijail jail{minijail_new()};
@@ -600,7 +619,7 @@ static pid_t seccomp_fork_impl(void (*prejail)()) {
   minijail_no_new_privs(jail.get());
   minijail_log_seccomp_filter_failures(jail.get());
   minijail_use_seccomp_filter(jail.get());
-  minijail_parse_seccomp_filters_from_fd(jail.get(), policy_fd.release());
+  minijail_parse_seccomp_filters_from_fd(jail.get(), tmp_fd.release());
 
   pid_t result = fork();
   if (result == -1) {
@@ -735,6 +754,16 @@ TEST_F(CrasherTest, seccomp_tombstone) {
   ASSERT_BACKTRACE_FRAME(result, "raise_debugger_signal");
 }
 
+extern "C" void foo() {
+  LOG(INFO) << "foo";
+  std::this_thread::sleep_for(1s);
+}
+
+extern "C" void bar() {
+  LOG(INFO) << "bar";
+  std::this_thread::sleep_for(1s);
+}
+
 TEST_F(CrasherTest, seccomp_backtrace) {
   int intercept_result;
   unique_fd output_fd;
@@ -742,6 +771,11 @@ TEST_F(CrasherTest, seccomp_backtrace) {
   static const auto dump_type = kDebuggerdNativeBacktrace;
   StartProcess(
       []() {
+        std::thread a(foo);
+        std::thread b(bar);
+
+        std::this_thread::sleep_for(100ms);
+
         raise_debugger_signal(dump_type);
         _exit(0);
       },
@@ -756,6 +790,8 @@ TEST_F(CrasherTest, seccomp_backtrace) {
   std::string result;
   ConsumeFd(std::move(output_fd), &result);
   ASSERT_BACKTRACE_FRAME(result, "raise_debugger_signal");
+  ASSERT_BACKTRACE_FRAME(result, "foo");
+  ASSERT_BACKTRACE_FRAME(result, "bar");
 }
 
 TEST_F(CrasherTest, seccomp_crash_logcat) {

debuggerd/handler/debuggerd_fallback.cpp

@@ -47,6 +47,7 @@
 #include <unwindstack/Regs.h>
 
 #include "debuggerd/handler.h"
+#include "handler/fallback.h"
 #include "tombstoned/tombstoned.h"
 #include "util.h"
 
@@ -187,7 +188,7 @@ static std::pair<pid_t, int> unpack_thread_fd(uint64_t value) {
 static void trace_handler(siginfo_t* info, ucontext_t* ucontext) {
   static std::atomic<uint64_t> trace_output(pack_thread_fd(-1, -1));
 
-  if (info->si_value.sival_int == ~0) {
+  if (info->si_value.sival_ptr == kDebuggerdFallbackSivalPtrRequestDump) {
     // Asked to dump by the original signal recipient.
     uint64_t val = trace_output.load();
     auto [tid, fd] = unpack_thread_fd(val);
@@ -259,7 +260,7 @@ static void trace_handler(siginfo_t* info, ucontext_t* ucontext) {
 
         siginfo_t siginfo = {};
         siginfo.si_code = SI_QUEUE;
-        siginfo.si_value.sival_int = ~0;
+        siginfo.si_value.sival_ptr = kDebuggerdFallbackSivalPtrRequestDump;
         siginfo.si_pid = getpid();
         siginfo.si_uid = getuid();
 
@@ -331,7 +332,7 @@ static void crash_handler(siginfo_t* info, ucontext_t* ucontext, void* abort_mes
 
 extern "C" void debuggerd_fallback_handler(siginfo_t* info, ucontext_t* ucontext,
                                            void* abort_message) {
-  if (info->si_signo == DEBUGGER_SIGNAL && info->si_value.sival_int != 0) {
+  if (info->si_signo == DEBUGGER_SIGNAL && info->si_value.sival_ptr != nullptr) {
     return trace_handler(info, ucontext);
   } else {
     return crash_handler(info, ucontext, abort_message);

debuggerd/handler/debuggerd_handler.cpp

@@ -58,6 +58,8 @@
 #include "dump_type.h"
 #include "protocol.h"
 
+#include "handler/fallback.h"
+
 using android::base::Pipe;
 
 // We muck with our fds in a 'thread' that doesn't share the same fd table.
@@ -473,13 +475,15 @@ static void debuggerd_signal_handler(int signal_number, siginfo_t* info, void* c
   }
 
   void* abort_message = nullptr;
+  uintptr_t si_val = reinterpret_cast<uintptr_t>(info->si_ptr);
   if (signal_number == DEBUGGER_SIGNAL) {
     if (info->si_code == SI_QUEUE && info->si_pid == __getpid()) {
       // Allow for the abort message to be explicitly specified via the sigqueue value.
       // Keep the bottom bit intact for representing whether we want a backtrace or a tombstone.
-      uintptr_t value = reinterpret_cast<uintptr_t>(info->si_ptr);
-      abort_message = reinterpret_cast<void*>(value & ~1);
-      info->si_ptr = reinterpret_cast<void*>(value & 1);
+      if (si_val != kDebuggerdFallbackSivalUintptrRequestDump) {
+        abort_message = reinterpret_cast<void*>(si_val & ~1);
+        info->si_ptr = reinterpret_cast<void*>(si_val & 1);
+      }
     }
   } else {
     if (g_callbacks.get_abort_message) {
@@ -492,7 +496,8 @@ static void debuggerd_signal_handler(int signal_number, siginfo_t* info, void* c
   // of a specific thread. It is possible that the prctl call might return 1,
   // then return 0 in subsequent calls, so check the sival_int to determine if
   // the fallback handler should be called first.
-  if (info->si_value.sival_int == ~0 || prctl(PR_GET_NO_NEW_PRIVS, 0, 0, 0, 0) == 1) {
+  if (si_val == kDebuggerdFallbackSivalUintptrRequestDump ||
+      prctl(PR_GET_NO_NEW_PRIVS, 0, 0, 0, 0) == 1) {
     // This check might be racy if another thread sets NO_NEW_PRIVS, but this should be unlikely,
     // you can only set NO_NEW_PRIVS to 1, and the effect should be at worst a single missing
     // ANR trace.

debuggerd/handler/fallback.h

@@ -0,0 +1,22 @@
+/*
+ * Copyright 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <stdint.h>
+
+static void* const kDebuggerdFallbackSivalPtrRequestDump = reinterpret_cast<void*>(~0UL);
+static const uintptr_t kDebuggerdFallbackSivalUintptrRequestDump = ~0UL;