Merge "debuggerd_handler: properly crash when PR_GET_DUMPABLE is 0." am: 90e05f68e2
am: 4e0648e7ca
Change-Id: If6821ade9e57e439e46a10efd008c8f55a6b160c
diff --git a/debuggerd/debuggerd_test.cpp b/debuggerd/debuggerd_test.cpp
index 3b24193..e7503e9 100644
--- a/debuggerd/debuggerd_test.cpp
+++ b/debuggerd/debuggerd_test.cpp
@@ -17,6 +17,7 @@
#include <err.h>
#include <fcntl.h>
#include <unistd.h>
+#include <sys/prctl.h>
#include <sys/types.h>
#include <chrono>
@@ -90,6 +91,7 @@
// 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 StartCrasher(const std::string& crash_type);
void FinishCrasher();
void AssertDeath(int signo);
@@ -166,9 +168,8 @@
}
}
-void CrasherTest::StartCrasher(const std::string& crash_type) {
- std::string type = "wait-" + crash_type;
-
+void CrasherTest::StartProcess(std::function<void()> function) {
+ unique_fd read_pipe;
unique_fd crasher_read_pipe;
if (!Pipe(&crasher_read_pipe, &crasher_pipe)) {
FAIL() << "failed to create pipe: " << strerror(errno);
@@ -182,9 +183,17 @@
dup2(crasher_read_pipe.get(), STDIN_FILENO);
dup2(devnull.get(), STDOUT_FILENO);
dup2(devnull.get(), STDERR_FILENO);
+ function();
+ _exit(0);
+ }
+}
+
+void CrasherTest::StartCrasher(const std::string& crash_type) {
+ std::string type = "wait-" + crash_type;
+ StartProcess([type]() {
execl(CRASHER_PATH, CRASHER_PATH, type.c_str(), nullptr);
err(1, "exec failed");
- }
+ });
}
void CrasherTest::FinishCrasher() {
@@ -379,3 +388,20 @@
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_F(CrasherTest, PR_SET_DUMPABLE_0_crash) {
+ StartProcess([]() {
+ prctl(PR_SET_DUMPABLE, 0);
+ volatile char* null = static_cast<char*>(nullptr);
+ *null = '\0';
+ });
+ AssertDeath(SIGSEGV);
+}
+
+TEST_F(CrasherTest, PR_SET_DUMPABLE_0_raise) {
+ StartProcess([]() {
+ prctl(PR_SET_DUMPABLE, 0);
+ raise(SIGUSR1);
+ });
+ AssertDeath(SIGUSR1);
+}
diff --git a/debuggerd/handler/debuggerd_handler.cpp b/debuggerd/handler/debuggerd_handler.cpp
index c031046..8ea06c0 100644
--- a/debuggerd/handler/debuggerd_handler.cpp
+++ b/debuggerd/handler/debuggerd_handler.cpp
@@ -63,6 +63,9 @@
static debuggerd_callbacks_t g_callbacks;
+// Mutex to ensure only one crashing thread dumps itself.
+static pthread_mutex_t crash_mutex = PTHREAD_MUTEX_INITIALIZER;
+
// Don't use __libc_fatal because it exits via abort, which might put us back into a signal handler.
#define fatal(...) \
do { \
@@ -153,7 +156,7 @@
}
struct debugger_thread_info {
- bool crash_dump_started = false;
+ bool crash_dump_started;
pid_t crashing_tid;
pid_t pseudothread_tid;
int signal_number;
@@ -233,12 +236,37 @@
return 0;
}
+static void resend_signal(siginfo_t* info) {
+ // Signals can either be fatal or nonfatal.
+ // For fatal signals, crash_dump will send us the signal we crashed with
+ // before resuming us, so that processes using waitpid on us will see that we
+ // exited with the correct exit status (e.g. so that sh will report
+ // "Segmentation fault" instead of "Killed"). For this to work, we need
+ // to deregister our signal handler for that signal before continuing.
+ if (info->si_signo != DEBUGGER_SIGNAL) {
+ signal(info->si_signo, SIG_DFL);
+ }
+
+ // We need to return from our signal handler so that crash_dump can see the
+ // signal via ptrace and dump the thread that crashed. However, returning
+ // does not guarantee that the signal will be thrown again, even for SIGSEGV
+ // and friends, since the signal could have been sent manually. We blocked
+ // all signals when registering the handler, so resending the signal (using
+ // rt_tgsigqueueinfo(2) to preserve SA_SIGINFO) will cause it to be delivered
+ // when our signal handler returns.
+ int rc = syscall(SYS_rt_tgsigqueueinfo, getpid(), gettid(), info->si_signo, info);
+ if (rc != 0) {
+ fatal("failed to resend signal during crash: %s", strerror(errno));
+ }
+
+ if (info->si_signo == DEBUGGER_SIGNAL) {
+ pthread_mutex_unlock(&crash_mutex);
+ }
+}
+
// Handler that does crash dumping by forking and doing the processing in the child.
// Do this by ptracing the relevant thread, and then execing debuggerd to do the actual dump.
static void debuggerd_signal_handler(int signal_number, siginfo_t* info, void*) {
- // Mutex to prevent multiple crashing threads from trying to talk
- // to debuggerd at the same time.
- static pthread_mutex_t crash_mutex = PTHREAD_MUTEX_INITIALIZER;
int ret = pthread_mutex_lock(&crash_mutex);
if (ret != 0) {
__libc_format_log(ANDROID_LOG_INFO, "libc", "pthread_mutex_lock failed: %s", strerror(ret));
@@ -251,14 +279,28 @@
info = nullptr;
}
+ struct siginfo si = {};
+ if (!info) {
+ memset(&si, 0, sizeof(si));
+ si.si_signo = signal_number;
+ 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.
+ }
+
log_signal_summary(signal_number, info);
if (prctl(PR_GET_DUMPABLE, 0, 0, 0, 0) == 0) {
- // process has disabled core dumps and PTRACE_ATTACH, and does not want to be dumped.
- // Honor that intention by not connecting to debuggerd and asking it to dump our internal state.
+ // The process has disabled core dumps and PTRACE_ATTACH, and does not want to be dumped.
__libc_format_log(ANDROID_LOG_INFO, "libc",
"Suppressing debuggerd output because prctl(PR_GET_DUMPABLE)==0");
- pthread_mutex_unlock(&crash_mutex);
+ resend_signal(info);
return;
}
@@ -266,8 +308,13 @@
if (g_callbacks.get_abort_message) {
abort_message = g_callbacks.get_abort_message();
}
+ // Populate si_value with the abort message address, if found.
+ if (abort_message) {
+ info->si_value.sival_ptr = abort_message;
+ }
debugger_thread_info thread_info = {
+ .crash_dump_started = false,
.pseudothread_tid = -1,
.crashing_tid = gettid(),
.signal_number = signal_number,
@@ -299,42 +346,7 @@
signal(signal_number, SIG_DFL);
}
- // 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.
- 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.
- }
-
- // Populate si_value with the abort message address, if found.
- if (abort_message) {
- info->si_value.sival_ptr = abort_message;
- }
-
- // Only resend the signal if we know that either crash_dump has ptraced us or
- // the signal was fatal.
- if (thread_info.crash_dump_started || signal_number != DEBUGGER_SIGNAL) {
- int rc = syscall(SYS_rt_tgsigqueueinfo, getpid(), gettid(), signal_number, info);
- if (rc != 0) {
- fatal("failed to resend signal during crash: %s", strerror(errno));
- }
- }
-
- if (signal_number == DEBUGGER_SIGNAL) {
- pthread_mutex_unlock(&crash_mutex);
- }
+ resend_signal(info);
}
void debuggerd_init(debuggerd_callbacks_t* callbacks) {