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

Commit 92abfb41 authored by Josh Gao's avatar Josh Gao
Browse files

debuggerd_handler: use syscall(__NR_get[pt]id) instead of get[pt]id. 

bionic's cached values for getpid/gettid can be invalid if the crashing
process manually invoked clone to create a thread or process, which
will lead the crash_dump refusing to do anything, because it sees the
actual values.

Use the getpid/gettid syscalls directly to ensure correct values on
this end.

Bug: http://b/37769298
Test: debuggerd_test
Change-Id: I0b1e652beb1a66e564a48b88ed7fa971d61c6ff9
(cherry picked from commit 2e7b8e2d)
parent 5f68b422

debuggerd/debuggerd_test.cpp

+35 −3
Original line number Diff line number Diff line
@@ -29,6 +29,7 @@ 


#include <android-base/file.h>

#include <android-base/logging.h>

#include <android-base/macros.h>

#include <android-base/parseint.h>

#include <android-base/properties.h>

#include <android-base/strings.h>
@@ -149,7 +150,7 @@ class CrasherTest : public ::testing::Test { 
  // Returns -1 if we fail to read a response from tombstoned, otherwise the received return code.

  void FinishIntercept(int* result);



  void StartProcess(std::function<void()> function);

  void StartProcess(std::function<void()> function, std::function<pid_t()> forker = fork);

  void StartCrasher(const std::string& crash_type);

  void FinishCrasher();

  void AssertDeath(int signo);
@@ -195,14 +196,14 @@ void CrasherTest::FinishIntercept(int* result) { 
  }

}



void CrasherTest::StartProcess(std::function<void()> function) {

void CrasherTest::StartProcess(std::function<void()> function, std::function<pid_t()> forker) {

  unique_fd read_pipe;

  unique_fd crasher_read_pipe;

  if (!Pipe(&crasher_read_pipe, &crasher_pipe)) {

    FAIL() << "failed to create pipe: " << strerror(errno);

  }



  crasher_pid = fork();

  crasher_pid = forker();

  if (crasher_pid == -1) {

    FAIL() << "fork failed: " << strerror(errno);

  } else if (crasher_pid == 0) {
@@ -527,6 +528,37 @@ TEST_F(CrasherTest, capabilities) { 
  ASSERT_MATCH(result, R"(#00 pc [0-9a-f]+\s+ /system/lib)" ARCH_SUFFIX R"(/libc.so \(tgkill)");

}



TEST_F(CrasherTest, fake_pid) {

  int intercept_result;

  unique_fd output_fd;



  // Prime the getpid/gettid caches.

  UNUSED(getpid());

  UNUSED(gettid());



  std::function<pid_t()> clone_fn = []() {

    return syscall(__NR_clone, SIGCHLD, nullptr, nullptr, nullptr, nullptr);

  };

  StartProcess(

      []() {

        ASSERT_NE(getpid(), syscall(__NR_getpid));

        ASSERT_NE(gettid(), syscall(__NR_gettid));

        raise(SIGSEGV);

      },

      clone_fn);



  StartIntercept(&output_fd);

  FinishCrasher();

  AssertDeath(SIGSEGV);

  FinishIntercept(&intercept_result);



  ASSERT_EQ(1, intercept_result) << "tombstoned reported failure";



  std::string result;

  ConsumeFd(std::move(output_fd), &result);

  ASSERT_MATCH(result, R"(#00 pc [0-9a-f]+\s+ /system/lib)" ARCH_SUFFIX R"(/libc.so \(tgkill)");

}



TEST(crash_dump, zombie) {

  pid_t forkpid = fork();

debuggerd/handler/debuggerd_handler.cpp

+15 −5
Original line number Diff line number Diff line
@@ -62,6 +62,16 @@ 


#define CRASH_DUMP_PATH "/system/bin/" CRASH_DUMP_NAME



// Wrappers that directly invoke the respective syscalls, in case the cached values are invalid.

#pragma GCC poison getpid gettid

static pid_t __getpid() {

  return syscall(__NR_getpid);

}



static pid_t __gettid() {

  return syscall(__NR_gettid);

}



class ErrnoRestorer {

 public:

  ErrnoRestorer() : saved_errno_(errno) {
@@ -120,7 +130,7 @@ static void log_signal_summary(int signum, const siginfo_t* info) { 
  }



  if (signum == DEBUGGER_SIGNAL) {

    __libc_format_log(ANDROID_LOG_INFO, "libc", "Requested dump for tid %d (%s)", gettid(),

    __libc_format_log(ANDROID_LOG_INFO, "libc", "Requested dump for tid %d (%s)", __gettid(),

                      thread_name);

    return;

  }
@@ -173,7 +183,7 @@ static void log_signal_summary(int signum, const siginfo_t* info) { 
  }



  __libc_format_log(ANDROID_LOG_FATAL, "libc", "Fatal signal %d (%s)%s%s in tid %d (%s)", signum,

                    signal_name, code_desc, addr_desc, gettid(), thread_name);

                    signal_name, code_desc, addr_desc, __gettid(), thread_name);

}



/*
@@ -331,7 +341,7 @@ static void resend_signal(siginfo_t* info, bool crash_dump_started) { 
  // rt_tgsigqueueinfo(2) to preserve SA_SIGINFO) will cause it to be delivered

  // when our signal handler returns.

  if (crash_dump_started || info->si_signo != DEBUGGER_SIGNAL) {

    int rc = syscall(SYS_rt_tgsigqueueinfo, getpid(), gettid(), info->si_signo, info);

    int rc = syscall(SYS_rt_tgsigqueueinfo, __getpid(), __gettid(), info->si_signo, info);

    if (rc != 0) {

      fatal_errno("failed to resend signal during crash");

    }
@@ -356,7 +366,7 @@ static void debuggerd_signal_handler(int signal_number, siginfo_t* info, void* c 
    memset(&si, 0, sizeof(si));

    si.si_signo = signal_number;

    si.si_code = SI_USER;

    si.si_pid = getpid();

    si.si_pid = __getpid();

    si.si_uid = getuid();

    info = &si;

  } else if (info->si_code >= 0 || info->si_code == SI_TKILL) {
@@ -398,7 +408,7 @@ static void debuggerd_signal_handler(int signal_number, siginfo_t* info, void* c 
  debugger_thread_info thread_info = {

    .crash_dump_started = false,

    .pseudothread_tid = -1,

    .crashing_tid = gettid(),

    .crashing_tid = __gettid(),

    .signal_number = signal_number,

    .info = info

  };