From 9774df613409c91f01ced1483bc01f42f6b4bf63 Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Thu, 15 Jan 2015 14:47:36 -0800 Subject: Move 32 bit/64 bit check into debuggerd. On 64 bit systems, calls to dump_backtrace_to_file or dump_tombstone try and directly contact the correct debuggerd (32 bit vs 64 bit) by reading the elf information for the executable. Unfortunately, system_server makes a call to dump_backtrace_to_file and it doesn't have permissions to read the executable data, so it defaults to always contacting the 64 bit debuggerd. This CL changes the code so that all dump requests go to the 64 bit debuggerd, which reads the elf information and redirects requests for 32 bit processes to the 32 bit debuggerd. Testing: - Forced the watchdog code in system_server to dump stacks and verified that all native stacks are dumped correctly. - Verified that dumpstate and bugreport still properly dump the native processes on a 64 bit and 32 bit system. - Intentionally forced the 64 bit to 32 bit redirect to write only a byte at a time and verified there are no errors, and no dropped data. - Used debuggerd and debuggerd64 to dump 32 bit and 64 bit processes seemlessly. - Used debuggerd on a 32 bit system to dump native stacks. Bug: https://code.google.com/p/android/issues/detail?id=97024 Change-Id: Ie01945153bdc1c4ded696c7334b61d58575314d1 --- debuggerd/Android.mk | 4 ++ debuggerd/debuggerd.cpp | 109 +++++++++++++++++++++++++++++++++++++++++++++- include/cutils/debugger.h | 40 ++++++----------- libcutils/debugger.c | 58 +++--------------------- 4 files changed, 129 insertions(+), 82 deletions(-) diff --git a/debuggerd/Android.mk b/debuggerd/Android.mk index 0cf2818..fbaac39 100644 --- a/debuggerd/Android.mk +++ b/debuggerd/Android.mk @@ -22,6 +22,10 @@ LOCAL_CPPFLAGS := \ -Wunused \ -Werror \ +ifeq ($(TARGET_IS_64_BIT),true) +LOCAL_CPPFLAGS += -DTARGET_IS_64_BIT +endif + LOCAL_SHARED_LIBRARIES := \ libbacktrace \ libcutils \ diff --git a/debuggerd/debuggerd.cpp b/debuggerd/debuggerd.cpp index 318b224..039b8ec 100644 --- a/debuggerd/debuggerd.cpp +++ b/debuggerd/debuggerd.cpp @@ -47,6 +47,14 @@ #include "tombstone.h" #include "utility.h" +// If the 32 bit executable is compiled on a 64 bit system, +// use the 32 bit socket name. +#if defined(TARGET_IS_64_BIT) && !defined(__LP64__) +#define SOCKET_NAME DEBUGGER32_SOCKET_NAME +#else +#define SOCKET_NAME DEBUGGER_SOCKET_NAME +#endif + struct debugger_request_t { debugger_action_t action; pid_t pid, tid; @@ -207,7 +215,7 @@ static int read_request(int fd, debugger_request_t* out_request) { return -1; } - out_request->action = msg.action; + out_request->action = static_cast(msg.action); out_request->tid = msg.tid; out_request->pid = cr.pid; out_request->uid = cr.uid; @@ -255,6 +263,85 @@ static bool should_attach_gdb(debugger_request_t* request) { return false; } +#if defined(__LP64__) +static bool is32bit(pid_t tid) { + char* exeline; + if (asprintf(&exeline, "/proc/%d/exe", tid) == -1) { + return false; + } + int fd = TEMP_FAILURE_RETRY(open(exeline, O_RDONLY | O_CLOEXEC)); + int saved_errno = errno; + free(exeline); + if (fd == -1) { + ALOGW("Failed to open /proc/%d/exe %s", tid, strerror(saved_errno)); + return false; + } + + char ehdr[EI_NIDENT]; + ssize_t bytes = TEMP_FAILURE_RETRY(read(fd, &ehdr, sizeof(ehdr))); + TEMP_FAILURE_RETRY(close(fd)); + if (bytes != (ssize_t) sizeof(ehdr) || memcmp(ELFMAG, ehdr, SELFMAG) != 0) { + return false; + } + if (ehdr[EI_CLASS] == ELFCLASS32) { + return true; + } + return false; +} + +static void redirect_to_32(int fd, debugger_request_t* request) { + debugger_msg_t msg; + memset(&msg, 0, sizeof(msg)); + msg.tid = request->tid; + msg.action = request->action; + + int sock_fd = socket_local_client(DEBUGGER32_SOCKET_NAME, ANDROID_SOCKET_NAMESPACE_ABSTRACT, + SOCK_STREAM | SOCK_CLOEXEC); + if (sock_fd < 0) { + ALOGE("Failed to connect to debuggerd32: %s", strerror(errno)); + return; + } + + if (TEMP_FAILURE_RETRY(write(sock_fd, &msg, sizeof(msg))) != (ssize_t) sizeof(msg)) { + ALOGE("Failed to write request to debuggerd32 socket: %s", strerror(errno)); + TEMP_FAILURE_RETRY(close(sock_fd)); + return; + } + + char ack; + if (TEMP_FAILURE_RETRY(read(sock_fd, &ack, 1)) == -1) { + ALOGE("Failed to read ack from debuggerd32 socket: %s", strerror(errno)); + TEMP_FAILURE_RETRY(close(sock_fd)); + return; + } + + char buffer[1024]; + ssize_t bytes_read; + while ((bytes_read = TEMP_FAILURE_RETRY(read(sock_fd, buffer, sizeof(buffer)))) > 0) { + ssize_t bytes_to_send = bytes_read; + ssize_t bytes_written; + do { + bytes_written = TEMP_FAILURE_RETRY(write(fd, buffer + bytes_read - bytes_to_send, + bytes_to_send)); + if (bytes_written == -1) { + if (errno == EAGAIN) { + // Retry the write. + continue; + } + ALOGE("Error while writing data to fd: %s", strerror(errno)); + break; + } + bytes_to_send -= bytes_written; + } while (bytes_written != 0 && bytes_to_send > 0); + if (bytes_to_send != 0) { + ALOGE("Failed to write all data to fd: read %zd, sent %zd", bytes_read, bytes_to_send); + break; + } + } + TEMP_FAILURE_RETRY(close(sock_fd)); +} +#endif + static void handle_request(int fd) { ALOGV("handle_request(%d)\n", fd); @@ -265,6 +352,24 @@ static void handle_request(int fd) { ALOGV("BOOM: pid=%d uid=%d gid=%d tid=%d\n", request.pid, request.uid, request.gid, request.tid); +#if defined(__LP64__) + // On 64 bit systems, requests to dump 32 bit and 64 bit tids come + // to the 64 bit debuggerd. If the process is a 32 bit executable, + // redirect the request to the 32 bit debuggerd. + if (is32bit(request.tid)) { + // Only dump backtrace and dump tombstone requests can be redirected. + if (request.action == DEBUGGER_ACTION_DUMP_BACKTRACE + || request.action == DEBUGGER_ACTION_DUMP_TOMBSTONE) { + redirect_to_32(fd, &request); + } else { + ALOGE("debuggerd: Not allowed to redirect action %d to 32 bit debuggerd\n", + request.action); + } + TEMP_FAILURE_RETRY(close(fd)); + return; + } +#endif + // 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. @@ -428,7 +533,7 @@ static int do_server() { act.sa_flags = SA_NOCLDWAIT; sigaction(SIGCHLD, &act, 0); - int s = socket_local_server(DEBUGGER_SOCKET_NAME, ANDROID_SOCKET_NAMESPACE_ABSTRACT, SOCK_STREAM); + int s = socket_local_server(SOCKET_NAME, ANDROID_SOCKET_NAMESPACE_ABSTRACT, SOCK_STREAM); if (s < 0) return 1; fcntl(s, F_SETFD, FD_CLOEXEC); diff --git a/include/cutils/debugger.h b/include/cutils/debugger.h index bae687d..285e1af 100644 --- a/include/cutils/debugger.h +++ b/include/cutils/debugger.h @@ -17,20 +17,14 @@ #ifndef __CUTILS_DEBUGGER_H #define __CUTILS_DEBUGGER_H +#include #include -#ifdef __cplusplus -extern "C" { -#endif +__BEGIN_DECLS -#define DEBUGGER32_SOCKET_NAME "android:debuggerd" -#define DEBUGGER64_SOCKET_NAME "android:debuggerd64" - -#if defined(__LP64__) -#define DEBUGGER_SOCKET_NAME DEBUGGER64_SOCKET_NAME -#else -#define DEBUGGER_SOCKET_NAME DEBUGGER32_SOCKET_NAME -#endif +#define DEBUGGER_SOCKET_NAME "android:debuggerd" +#define DEBUGGER32_SOCKET_NAME "android:debuggerd32" +#define DEBUGGER64_SOCKET_NAME DEBUGGER_SOCKET_NAME typedef enum { // dump a crash @@ -41,23 +35,17 @@ typedef enum { DEBUGGER_ACTION_DUMP_BACKTRACE, } debugger_action_t; -typedef struct { - debugger_action_t action; +// Make sure that all values have a fixed size so that this structure +// is the same for 32 bit and 64 bit processes. +// NOTE: Any changes to this structure must also be reflected in +// bionic/linker/debugger.cpp. +typedef struct __attribute__((packed)) { + int32_t action; pid_t tid; - uintptr_t abort_msg_address; + uint64_t abort_msg_address; int32_t original_si_code; } debugger_msg_t; -#if defined(__LP64__) -// For a 64 bit process to contact the 32 bit debuggerd. -typedef struct { - debugger_action_t action; - pid_t tid; - uint32_t abort_msg_address; - int32_t original_si_code; -} debugger32_msg_t; -#endif - /* Dumps a process backtrace, registers, and stack to a tombstone file (requires root). * Stores the tombstone path in the provided buffer. * Returns 0 on success, -1 on error. @@ -84,8 +72,6 @@ int dump_backtrace_to_file(pid_t tid, int fd); */ int dump_backtrace_to_file_timeout(pid_t tid, int fd, int timeout_secs); -#ifdef __cplusplus -} -#endif +__END_DECLS #endif /* __CUTILS_DEBUGGER_H */ diff --git a/libcutils/debugger.c b/libcutils/debugger.c index b8a2efc..2cd8ec3 100644 --- a/libcutils/debugger.c +++ b/libcutils/debugger.c @@ -29,33 +29,6 @@ #define LOG_TAG "DEBUG" #include -#if defined(__LP64__) -#include - -static bool is32bit(pid_t tid) { - char* exeline; - if (asprintf(&exeline, "/proc/%d/exe", tid) == -1) { - return false; - } - int fd = open(exeline, O_RDONLY | O_CLOEXEC); - free(exeline); - if (fd == -1) { - return false; - } - - char ehdr[EI_NIDENT]; - ssize_t bytes = read(fd, &ehdr, sizeof(ehdr)); - close(fd); - if (bytes != (ssize_t) sizeof(ehdr) || memcmp(ELFMAG, ehdr, SELFMAG) != 0) { - return false; - } - if (ehdr[EI_CLASS] == ELFCLASS32) { - return true; - } - return false; -} -#endif - static int send_request(int sock_fd, void* msg_ptr, size_t msg_len) { int result = 0; if (TEMP_FAILURE_RETRY(write(sock_fd, msg_ptr, msg_len)) != (ssize_t) msg_len) { @@ -72,32 +45,11 @@ static int send_request(int sock_fd, void* msg_ptr, size_t msg_len) { static int make_dump_request(debugger_action_t action, pid_t tid, int timeout_secs) { const char* socket_name; debugger_msg_t msg; - size_t msg_len; - void* msg_ptr; - -#if defined(__LP64__) - debugger32_msg_t msg32; - if (is32bit(tid)) { - msg_len = sizeof(debugger32_msg_t); - memset(&msg32, 0, msg_len); - msg32.tid = tid; - msg32.action = action; - msg_ptr = &msg32; - - socket_name = DEBUGGER32_SOCKET_NAME; - } else -#endif - { - msg_len = sizeof(debugger_msg_t); - memset(&msg, 0, msg_len); - msg.tid = tid; - msg.action = action; - msg_ptr = &msg; - - socket_name = DEBUGGER_SOCKET_NAME; - } + memset(&msg, 0, sizeof(msg)); + msg.tid = tid; + msg.action = action; - int sock_fd = socket_local_client(socket_name, ANDROID_SOCKET_NAMESPACE_ABSTRACT, + int sock_fd = socket_local_client(DEBUGGER_SOCKET_NAME, ANDROID_SOCKET_NAMESPACE_ABSTRACT, SOCK_STREAM | SOCK_CLOEXEC); if (sock_fd < 0) { return -1; @@ -116,7 +68,7 @@ static int make_dump_request(debugger_action_t action, pid_t tid, int timeout_se } } - if (send_request(sock_fd, msg_ptr, msg_len) < 0) { + if (send_request(sock_fd, &msg, sizeof(msg)) < 0) { TEMP_FAILURE_RETRY(close(sock_fd)); return -1; } -- cgit v1.1