Merge "debuggerd: fork and drop privileges when dumping."

debuggerd/debuggerd.cpp

@@ -24,11 +24,12 @@
 #include <dirent.h>
 #include <time.h>
 
-#include <sys/ptrace.h>
-#include <sys/wait.h>
 #include <elf.h>
-#include <sys/stat.h>
 #include <sys/poll.h>
+#include <sys/prctl.h>
+#include <sys/ptrace.h>
+#include <sys/stat.h>
+#include <sys/wait.h>
 
 #include <selinux/android.h>
 
@@ -363,6 +364,37 @@ static void handle_request(int fd) {
   }
 #endif
 
+  // Fork a child to handle the rest of the request.
+  pid_t fork_pid = fork();
+  if (fork_pid == -1) {
+    ALOGE("debuggerd: failed to fork: %s\n", strerror(errno));
+    return;
+  } else if (fork_pid != 0) {
+    waitpid(fork_pid, nullptr, 0);
+    return;
+  }
+
+  // Open the tombstone file if we need it.
+  std::string tombstone_path;
+  int tombstone_fd = -1;
+  switch (request.action) {
+    case DEBUGGER_ACTION_DUMP_TOMBSTONE:
+    case DEBUGGER_ACTION_CRASH:
+      tombstone_fd = open_tombstone(&tombstone_path);
+      if (tombstone_fd == -1) {
+        ALOGE("debuggerd: failed to open tombstone file: %s\n", strerror(errno));
+        exit(1);
+      }
+      break;
+
+    case DEBUGGER_ACTION_DUMP_BACKTRACE:
+      break;
+
+    default:
+      ALOGE("debuggerd: unexpected request action: %d", request.action);
+      exit(1);
+  }
+
   // At this point, the thread that made the request is blocked in
   // a read() call.  If the thread has crashed, then this gives us
   // time to PTRACE_ATTACH to it before it has a chance to really fault.
@@ -374,19 +406,32 @@ static void handle_request(int fd) {
   // See details in bionic/libc/linker/debugger.c, in function
   // debugger_signal_handler().
   if (ptrace(PTRACE_ATTACH, request.tid, 0, 0)) {
-    ALOGE("ptrace attach failed: %s\n", strerror(errno));
+    ALOGE("debuggerd: ptrace attach failed: %s\n", strerror(errno));
-    return;
+    exit(1);
+  }
+
+  // Generate the backtrace map before dropping privileges.
+  std::unique_ptr<BacktraceMap> backtrace_map(BacktraceMap::Create(request.pid));
+
+  // Now that we've done everything that requires privileges, we can drop them.
+  if (setresgid(AID_DEBUGGERD, AID_DEBUGGERD, AID_DEBUGGERD) != 0) {
+    ALOGE("debuggerd: failed to setresgid");
+    exit(1);
+  }
+
+  if (setresuid(AID_DEBUGGERD, AID_DEBUGGERD, AID_DEBUGGERD) != 0) {
+    ALOGE("debuggerd: failed to setresuid");
+    exit(1);
   }
 
   bool detach_failed = false;
   bool tid_unresponsive = false;
   bool attach_gdb = should_attach_gdb(&request);
   if (TEMP_FAILURE_RETRY(write(fd, "\0", 1)) != 1) {
-    ALOGE("failed responding to client: %s\n", strerror(errno));
+    ALOGE("debuggerd: failed to respond to client: %s\n", strerror(errno));
-    return;
+    exit(1);
   }
 
-  std::unique_ptr<char> tombstone_path;
   int total_sleep_time_usec = 0;
   while (true) {
     int signal = wait_for_sigstop(request.tid, &total_sleep_time_usec, &detach_failed);
@@ -399,9 +444,9 @@ static void handle_request(int fd) {
       case SIGSTOP:
         if (request.action == DEBUGGER_ACTION_DUMP_TOMBSTONE) {
           ALOGV("stopped -- dumping to tombstone\n");
-          tombstone_path.reset(engrave_tombstone(
+          engrave_tombstone(tombstone_fd, backtrace_map.get(), request.pid, request.tid, signal,
-            request.pid, request.tid, signal, request.original_si_code, request.abort_msg_address,
+                            request.original_si_code, request.abort_msg_address, true,
-            true, &detach_failed, &total_sleep_time_usec));
+                            &detach_failed, &total_sleep_time_usec);
         } else if (request.action == DEBUGGER_ACTION_DUMP_BACKTRACE) {
           ALOGV("stopped -- dumping to fd\n");
           dump_backtrace(fd, -1, request.pid, request.tid, &detach_failed, &total_sleep_time_usec);
@@ -409,7 +454,7 @@ static void handle_request(int fd) {
           ALOGV("stopped -- continuing\n");
           status = ptrace(PTRACE_CONT, request.tid, 0, 0);
           if (status) {
-            ALOGE("ptrace continue failed: %s\n", strerror(errno));
+            ALOGE("debuggerd: ptrace continue failed: %s\n", strerror(errno));
           }
           continue;  // loop again
         }
@@ -432,21 +477,21 @@ static void handle_request(int fd) {
         kill(request.pid, SIGSTOP);
         // don't dump sibling threads when attaching to GDB because it
         // makes the process less reliable, apparently...
-        tombstone_path.reset(engrave_tombstone(
+        engrave_tombstone(tombstone_fd, backtrace_map.get(), request.pid, request.tid, signal,
-          request.pid, request.tid, signal, request.original_si_code, request.abort_msg_address,
+                          request.original_si_code, request.abort_msg_address, !attach_gdb,
-          !attach_gdb, &detach_failed, &total_sleep_time_usec));
+                          &detach_failed, &total_sleep_time_usec);
         break;
 
       default:
-        ALOGE("process stopped due to unexpected signal %d\n", signal);
+        ALOGE("debuggerd: process stopped due to unexpected signal %d\n", signal);
         break;
     }
     break;
   }
 
   if (request.action == DEBUGGER_ACTION_DUMP_TOMBSTONE) {
-    if (tombstone_path) {
+    if (!tombstone_path.empty()) {
-      write(fd, tombstone_path.get(), strlen(tombstone_path.get()));
+      write(fd, tombstone_path.c_str(), tombstone_path.length());
     }
   }
 
@@ -457,7 +502,7 @@ static void handle_request(int fd) {
       kill(request.pid, SIGSTOP);
     }
     if (ptrace(PTRACE_DETACH, request.tid, 0, 0)) {
-      ALOGE("ptrace detach from %d failed: %s", request.tid, strerror(errno));
+      ALOGE("debuggerd: ptrace detach from %d failed: %s", request.tid, strerror(errno));
       detach_failed = true;
     } else if (attach_gdb) {
       // if debug.db.uid is set, its value indicates if we should wait
@@ -468,16 +513,9 @@ static void handle_request(int fd) {
     }
   }
 
-  // resume stopped process (so it can crash in peace).
+  // Resume the stopped process so it can crash in peace, and exit.
   kill(request.pid, SIGCONT);
-
+  exit(0);
-  // If we didn't successfully detach, we're still the parent, and the
-  // actual parent won't receive a death notification via wait(2).  At this point
-  // there's not much we can do about that.
-  if (detach_failed) {
-    ALOGE("debuggerd committing suicide to free the zombie!\n");
-    kill(getpid(), SIGKILL);
-  }
 }
 
 static int do_server() {

debuggerd/tombstone.cpp

@@ -632,7 +632,7 @@ static void dump_abort_message(Backtrace* backtrace, log_t* log, uintptr_t addre
 }
 
 // Dumps all information about the specified pid to the tombstone.
-static bool dump_crash(log_t* log, pid_t pid, pid_t tid, int signal, int si_code,
+static bool dump_crash(log_t* log, BacktraceMap* map, pid_t pid, pid_t tid, int signal, int si_code,
                        uintptr_t abort_msg_address, bool dump_sibling_threads,
                        int* total_sleep_time_usec) {
   // don't copy log messages to tombstone unless this is a dev device
@@ -659,8 +659,7 @@ static bool dump_crash(log_t* log, pid_t pid, pid_t tid, int signal, int si_code
     dump_signal_info(log, tid, signal, si_code);
   }
 
-  std::unique_ptr<BacktraceMap> map(BacktraceMap::Create(pid));
+  std::unique_ptr<Backtrace> backtrace(Backtrace::Create(pid, tid, map));
-  std::unique_ptr<Backtrace> backtrace(Backtrace::Create(pid, tid, map.get()));
   dump_abort_message(backtrace.get(), log, abort_msg_address);
   dump_registers(log, tid);
   if (backtrace->Unwind(0)) {
@@ -669,8 +668,8 @@ static bool dump_crash(log_t* log, pid_t pid, pid_t tid, int signal, int si_code
     ALOGE("Unwind failed: pid = %d, tid = %d", pid, tid);
   }
   dump_memory_and_code(log, backtrace.get());
-  if (map.get() != nullptr) {
+  if (map) {
-    dump_all_maps(backtrace.get(), map.get(), log, tid);
+    dump_all_maps(backtrace.get(), map, log, tid);
   }
 
   if (want_logs) {
@@ -679,7 +678,7 @@ static bool dump_crash(log_t* log, pid_t pid, pid_t tid, int signal, int si_code
 
   bool detach_failed = false;
   if (dump_sibling_threads) {
-    detach_failed = dump_sibling_thread_report(log, pid, tid, total_sleep_time_usec, map.get());
+    detach_failed = dump_sibling_thread_report(log, pid, tid, total_sleep_time_usec, map);
   }
 
   if (want_logs) {
@@ -698,53 +697,57 @@ static bool dump_crash(log_t* log, pid_t pid, pid_t tid, int signal, int si_code
   return detach_failed;
 }
 
-// find_and_open_tombstone - find an available tombstone slot, if any, of the
+// open_tombstone - find an available tombstone slot, if any, of the
 // form tombstone_XX where XX is 00 to MAX_TOMBSTONES-1, inclusive. If no
 // file is available, we reuse the least-recently-modified file.
-//
+int open_tombstone(std::string* out_path) {
-// Returns the path of the tombstone file, allocated using malloc().  Caller must free() it.
-static char* find_and_open_tombstone(int* fd) {
   // In a single pass, find an available slot and, in case none
   // exist, find and record the least-recently-modified file.
   char path[128];
+  int fd = -1;
   int oldest = -1;
   struct stat oldest_sb;
   for (int i = 0; i < MAX_TOMBSTONES; i++) {
     snprintf(path, sizeof(path), TOMBSTONE_TEMPLATE, i);
 
     struct stat sb;
-    if (!stat(path, &sb)) {
+    if (stat(path, &sb) == 0) {
       if (oldest < 0 || sb.st_mtime < oldest_sb.st_mtime) {
         oldest = i;
         oldest_sb.st_mtime = sb.st_mtime;
       }
       continue;
     }
-    if (errno != ENOENT)
+    if (errno != ENOENT) continue;
-      continue;
 
-    *fd = open(path, O_CREAT | O_EXCL | O_WRONLY | O_NOFOLLOW | O_CLOEXEC, 0600);
+    fd = open(path, O_CREAT | O_EXCL | O_WRONLY | O_NOFOLLOW | O_CLOEXEC, 0600);
-    if (*fd < 0)
+    if (fd < 0) continue;  // raced ?
-      continue;   // raced ?
 
-    fchown(*fd, AID_SYSTEM, AID_SYSTEM);
+    if (out_path) {
-    return strdup(path);
+      *out_path = path;
+    }
+    fchown(fd, AID_SYSTEM, AID_SYSTEM);
+    return fd;
   }
 
   if (oldest < 0) {
-    ALOGE("Failed to find a valid tombstone, default to using tombstone 0.\n");
+    ALOGE("debuggerd: failed to find a valid tombstone, default to using tombstone 0.\n");
     oldest = 0;
   }
 
   // we didn't find an available file, so we clobber the oldest one
   snprintf(path, sizeof(path), TOMBSTONE_TEMPLATE, oldest);
-  *fd = open(path, O_CREAT | O_TRUNC | O_WRONLY | O_NOFOLLOW | O_CLOEXEC, 0600);
+  fd = open(path, O_CREAT | O_TRUNC | O_WRONLY | O_NOFOLLOW | O_CLOEXEC, 0600);
-  if (*fd < 0) {
+  if (fd < 0) {
-    ALOGE("failed to open tombstone file '%s': %s\n", path, strerror(errno));
+    ALOGE("debuggerd: failed to open tombstone file '%s': %s\n", path, strerror(errno));
-    return NULL;
+    return -1;
   }
-  fchown(*fd, AID_SYSTEM, AID_SYSTEM);
+
-  return strdup(path);
+  if (out_path) {
+    *out_path = path;
+  }
+  fchown(fd, AID_SYSTEM, AID_SYSTEM);
+  return fd;
 }
 
 static int activity_manager_connect() {
@@ -777,49 +780,27 @@ static int activity_manager_connect() {
   return amfd;
 }
 
-char* engrave_tombstone(pid_t pid, pid_t tid, int signal, int original_si_code,
+void engrave_tombstone(int tombstone_fd, BacktraceMap* map, pid_t pid, pid_t tid, int signal,
-                        uintptr_t abort_msg_address, bool dump_sibling_threads,
+                       int original_si_code, uintptr_t abort_msg_address, bool dump_sibling_threads,
-                        bool* detach_failed, int* total_sleep_time_usec) {
+                       bool* detach_failed, int* total_sleep_time_usec) {
-
   log_t log;
   log.current_tid = tid;
   log.crashed_tid = tid;
 
-  if ((mkdir(TOMBSTONE_DIR, 0755) == -1) && (errno != EEXIST)) {
+  if (tombstone_fd < 0) {
-    ALOGE("failed to create %s: %s\n", TOMBSTONE_DIR, strerror(errno));
+    ALOGE("debuggerd: skipping tombstone write, nothing to do.\n");
-  }
-
-  if (chown(TOMBSTONE_DIR, AID_SYSTEM, AID_SYSTEM) == -1) {
-    ALOGE("failed to change ownership of %s: %s\n", TOMBSTONE_DIR, strerror(errno));
-  }
-
-  int fd = -1;
-  char* path = NULL;
-  if (selinux_android_restorecon(TOMBSTONE_DIR, 0) == 0) {
-    path = find_and_open_tombstone(&fd);
-  } else {
-    ALOGE("Failed to restore security context, not writing tombstone.\n");
-  }
-
-  if (fd < 0) {
-    ALOGE("Skipping tombstone write, nothing to do.\n");
     *detach_failed = false;
-    return NULL;
+    return;
   }
 
-  log.tfd = fd;
+  log.tfd = tombstone_fd;
   // Preserve amfd since it can be modified through the calls below without
   // being closed.
   int amfd = activity_manager_connect();
   log.amfd = amfd;
-  *detach_failed = dump_crash(&log, pid, tid, signal, original_si_code, abort_msg_address,
+  *detach_failed = dump_crash(&log, map, pid, tid, signal, original_si_code, abort_msg_address,
                               dump_sibling_threads, total_sleep_time_usec);
 
-  _LOG(&log, logtype::BACKTRACE, "\nTombstone written to: %s\n", path);
+  // This file descriptor can be -1, any error is ignored.
-
-  // Either of these file descriptors can be -1, any error is ignored.
   close(amfd);
-  close(fd);
-
-  return path;
 }

debuggerd/tombstone.h

@@ -17,15 +17,23 @@
 #ifndef _DEBUGGERD_TOMBSTONE_H
 #define _DEBUGGERD_TOMBSTONE_H
 
-#include <stddef.h>
 #include <stdbool.h>
+#include <stddef.h>
 #include <sys/types.h>
+#include <string>
+
+class BacktraceMap;
+
+/* Create and open a tombstone file for writing.
+ * Returns a writable file descriptor, or -1 with errno set appropriately.
+ * If out_path is non-null, *out_path is set to the path of the tombstone file.
+ */
+int open_tombstone(std::string* path);
 
 /* Creates a tombstone file and writes the crash dump to it.
  * Returns the path of the tombstone, which must be freed using free(). */
-char* engrave_tombstone(pid_t pid, pid_t tid, int signal, int original_si_code,
+void engrave_tombstone(int tombstone_fd, BacktraceMap* map, pid_t pid, pid_t tid, int signal,
-                        uintptr_t abort_msg_address,
+                       int original_si_code, uintptr_t abort_msg_address, bool dump_sibling_threads,
-                        bool dump_sibling_threads, bool* detach_failed,
+                       bool* detach_failed, int* total_sleep_time_usec);
-                        int* total_sleep_time_usec);
 
 #endif // _DEBUGGERD_TOMBSTONE_H

include/private/android_filesystem_config.h

@@ -86,6 +86,7 @@
 #define AID_METRICS_COLL  1042  /* metrics_collector process */
 #define AID_METRICSD      1043  /* metricsd process */
 #define AID_WEBSERV       1044  /* webservd process */
+#define AID_DEBUGGERD     1045  /* debuggerd unprivileged user */
 
 #define AID_SHELL         2000  /* adb and debug shell user */
 #define AID_CACHE         2001  /* cache access */
@@ -190,6 +191,7 @@ static const struct android_id_info android_ids[] = {
     { "metrics_coll",  AID_METRICS_COLL },
     { "metricsd",      AID_METRICSD },
     { "webserv",       AID_WEBSERV },
+    { "debuggerd",     AID_DEBUGGERD, },
 
     { "shell",         AID_SHELL, },
     { "cache",         AID_CACHE, },