From ec91809daeb35113bfc4840c07367ff24f75903a Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Fri, 10 Mar 2017 14:44:54 -0800 Subject: [PATCH 1/2] debuggerd_handler: restore errno. Bug: http://b/31448909 Test: mma Change-Id: I737d66e8bed5fb31c2558f68608d3df460fa73c9 --- debuggerd/handler/debuggerd_handler.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/debuggerd/handler/debuggerd_handler.cpp b/debuggerd/handler/debuggerd_handler.cpp index c09c2f31f..cf24d5767 100644 --- a/debuggerd/handler/debuggerd_handler.cpp +++ b/debuggerd/handler/debuggerd_handler.cpp @@ -62,6 +62,19 @@ #define CRASH_DUMP_PATH "/system/bin/" CRASH_DUMP_NAME +class ErrnoRestorer { + public: + ErrnoRestorer() : saved_errno_(errno) { + } + + ~ErrnoRestorer() { + errno = saved_errno_; + } + + private: + int saved_errno_; +}; + extern "C" void debuggerd_fallback_handler(siginfo_t*, ucontext_t*, void*); static debuggerd_callbacks_t g_callbacks; @@ -328,6 +341,10 @@ static void resend_signal(siginfo_t* info, bool crash_dump_started) { // 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* context) { + // Make sure we don't change the value of errno, in case a signal comes in between the process + // making a syscall and checking errno. + ErrnoRestorer restorer; + // It's possible somebody cleared the SA_SIGINFO flag, which would mean // our "info" arg holds an undefined value. if (!have_siginfo(signal_number)) { From 428daafc5b9a2186e2d893c8ebd1c4337ce3c7a9 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Fri, 10 Mar 2017 14:49:19 -0800 Subject: [PATCH 2/2] crash_dump: improve logging for when a process dies prematurely. If a process that's getting dumped dies before crash_dump starts (e.g. because seccomp immediately kills it after it execs crash_dump), improve the error message to not just say "target died before we could attach". Bug: http://b/36077710 Test: inserted an exit in the handler, inspected output Change-Id: I7d394c66d60d328b096b15654b3648e1ed711728 --- debuggerd/crash_dump.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/debuggerd/crash_dump.cpp b/debuggerd/crash_dump.cpp index 65854247e..57a6c448a 100644 --- a/debuggerd/crash_dump.cpp +++ b/debuggerd/crash_dump.cpp @@ -148,7 +148,12 @@ static void abort_handler(pid_t target, const bool& tombstoned_connected, } } - dprintf(output_fd.get(), "crash_dump failed to dump process %d: %s\n", target, abort_msg); + dprintf(output_fd.get(), "crash_dump failed to dump process"); + if (target != 1) { + dprintf(output_fd.get(), " %d: %s\n", target, abort_msg); + } else { + dprintf(output_fd.get(), ": %s\n", abort_msg); + } _exit(1); } @@ -195,7 +200,7 @@ int main(int argc, char** argv) { pid_t pseudothread_tid; if (target == 1) { - LOG(FATAL) << "target died before we could attach"; + LOG(FATAL) << "target died before we could attach (received main tid = " << main_tid << ")"; } if (!android::base::ParseInt(argv[1], &main_tid, 1, std::numeric_limits::max())) {