Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 42fd74bd authored by Josh Gao's avatar Josh Gao
Browse files

crash_dump: don't abort if we fail to attach a sibling. 

A TOCTOU race can occur between listing threads and attaching them.
Don't abort and leave the process in a stopped state when this happens.

Bug: http://b/34472671
Test: while true; do debuggerd -b `pidof audioserver`; done
Change-Id: Ib1632c3423fddf506b5c7874223c82fada78a85e
parent 1e7cdf3c

debuggerd/crash_dump.cpp

+15 −15
Original line number Diff line number Diff line
@@ -57,8 +57,9 @@ static bool pid_contains_tid(pid_t pid, pid_t tid) { 
}



// Attach to a thread, and verify that it's still a member of the given process

static bool ptrace_attach_thread(pid_t pid, pid_t tid) {

static bool ptrace_attach_thread(pid_t pid, pid_t tid, std::string* error) {

  if (ptrace(PTRACE_ATTACH, tid, 0, 0) != 0) {

    *error = StringPrintf("failed to attach to thread %d: %s", tid, strerror(errno));

    return false;

  }
@@ -67,7 +68,7 @@ static bool ptrace_attach_thread(pid_t pid, pid_t tid) { 
    if (ptrace(PTRACE_DETACH, tid, 0, 0) != 0) {

      PLOG(FATAL) << "failed to detach from thread " << tid;

    }

    errno = ECHILD;

    *error = StringPrintf("thread %d is not in process %d", tid, pid);

    return false;

  }

  return true;
@@ -244,9 +245,9 @@ int main(int argc, char** argv) { 


  check_process(target_proc_fd, target);



  int attach_error = 0;

  if (!ptrace_attach_thread(target, main_tid)) {

    PLOG(FATAL) << "failed to attach to thread " << main_tid << " in process " << target;

  std::string attach_error;

  if (!ptrace_attach_thread(target, main_tid, &attach_error)) {

    LOG(FATAL) << attach_error;

  }



  check_process(target_proc_fd, target);
@@ -268,10 +269,6 @@ int main(int argc, char** argv) { 
    TEMP_FAILURE_RETRY(dup2(devnull.get(), STDOUT_FILENO));

  }



  if (attach_error != 0) {

    PLOG(FATAL) << "failed to attach to thread " << main_tid << " in process " << target;

  }



  LOG(INFO) << "performing dump of process " << target << " (target tid = " << main_tid << ")";



  // At this point, the thread that made the request has been PTRACE_ATTACHed
@@ -307,6 +304,7 @@ int main(int argc, char** argv) { 
  bool fatal_signal = signo != DEBUGGER_SIGNAL;

  int resume_signal = fatal_signal ? signo : 0;

  std::set<pid_t> siblings;

  std::set<pid_t> attached_siblings;

  if (resume_signal == 0) {

    if (!android::procinfo::GetProcessTids(target, &siblings)) {

      PLOG(FATAL) << "failed to get process siblings";
@@ -314,8 +312,10 @@ int main(int argc, char** argv) { 
    siblings.erase(main_tid);



    for (pid_t sibling_tid : siblings) {

      if (!ptrace_attach_thread(target, sibling_tid)) {

        PLOG(FATAL) << "failed to attach to thread " << main_tid << " in process " << target;

      if (!ptrace_attach_thread(target, sibling_tid, &attach_error)) {

        LOG(WARNING) << attach_error;

      } else {

        attached_siblings.insert(sibling_tid);

      }

    }

  }
@@ -328,14 +328,14 @@ int main(int argc, char** argv) { 
  std::string amfd_data;



  if (backtrace) {

    dump_backtrace(output_fd.get(), backtrace_map.get(), target, main_tid, siblings, 0);

    dump_backtrace(output_fd.get(), backtrace_map.get(), target, main_tid, attached_siblings, 0);

  } else {

    // Collect the list of open files.

    OpenFilesList open_files;

    populate_open_files_list(target, &open_files);



    engrave_tombstone(output_fd.get(), backtrace_map.get(), open_files, target, main_tid, siblings,

                      abort_address, fatal_signal ? &amfd_data : nullptr);

    engrave_tombstone(output_fd.get(), backtrace_map.get(), open_files, target, main_tid,

                      attached_siblings, abort_address, fatal_signal ? &amfd_data : nullptr);

  }



  bool wait_for_gdb = android::base::GetBoolProperty("debug.debuggerd.wait_for_gdb", false);
@@ -357,7 +357,7 @@ int main(int argc, char** argv) { 
    }

  }



  for (pid_t tid : siblings) {

  for (pid_t tid : attached_siblings) {

    // Don't send the signal to sibling threads.

    if (ptrace(PTRACE_DETACH, tid, 0, wait_for_gdb ? SIGSTOP : 0) != 0) {

      PLOG(ERROR) << "ptrace detach from " << tid << " failed";