diff --git a/adb/adb.cpp b/adb/adb.cpp index 38c6f62c9..a8fe73651 100644 --- a/adb/adb.cpp +++ b/adb/adb.cpp @@ -891,6 +891,10 @@ int launch_server(const std::string& socket_spec) { // child side of the fork pipe_read.reset(); + // android::base::Pipe unconditionally opens the pipe with O_CLOEXEC. + // Undo this manually. + fcntl(pipe_write.get(), F_SETFD, 0); + char reply_fd[30]; snprintf(reply_fd, sizeof(reply_fd), "%d", pipe_write.get()); // child process diff --git a/adb/adb_unique_fd.cpp b/adb/adb_unique_fd.cpp index 2079be152..dec73bc2e 100644 --- a/adb/adb_unique_fd.cpp +++ b/adb/adb_unique_fd.cpp @@ -21,45 +21,8 @@ #include "sysdeps.h" -#if !defined(_WIN32) -bool Pipe(unique_fd* read, unique_fd* write, int flags) { - int pipefd[2]; -#if !defined(__APPLE__) - if (pipe2(pipefd, flags) != 0) { - return false; - } -#else - // Darwin doesn't have pipe2. Implement it ourselves. - if (flags != 0 && (flags & ~(O_CLOEXEC | O_NONBLOCK)) != 0) { - errno = EINVAL; - return false; - } - - if (pipe(pipefd) != 0) { - return false; - } - - if (flags & O_CLOEXEC) { - if (fcntl(pipefd[0], F_SETFD, FD_CLOEXEC) != 0 || - fcntl(pipefd[1], F_SETFD, FD_CLOEXEC) != 0) { - adb_close(pipefd[0]); - adb_close(pipefd[1]); - return false; - } - } - - if (flags & O_NONBLOCK) { - if (fcntl(pipefd[0], F_SETFL, O_NONBLOCK) != 0 || - fcntl(pipefd[1], F_SETFL, O_NONBLOCK) != 0) { - adb_close(pipefd[0]); - adb_close(pipefd[1]); - return false; - } - } -#endif - - read->reset(pipefd[0]); - write->reset(pipefd[1]); - return true; +#if defined(_WIN32) +void AdbCloser::Close(int fd) { + adb_close(fd); } #endif diff --git a/adb/adb_unique_fd.h b/adb/adb_unique_fd.h index bad501abb..d47213d75 100644 --- a/adb/adb_unique_fd.h +++ b/adb/adb_unique_fd.h @@ -21,15 +21,15 @@ #include +#if defined(_WIN32) // Helper to automatically close an FD when it goes out of scope. struct AdbCloser { static void Close(int fd); }; using unique_fd = android::base::unique_fd_impl; - -#if !defined(_WIN32) -bool Pipe(unique_fd* read, unique_fd* write, int flags = 0); +#else +using unique_fd = android::base::unique_fd; #endif template diff --git a/adb/adb_utils.cpp b/adb/adb_utils.cpp index 0c3327fc3..ffac315b3 100644 --- a/adb/adb_utils.cpp +++ b/adb/adb_utils.cpp @@ -274,10 +274,6 @@ std::string adb_get_android_dir_path() { return android_dir; } -void AdbCloser::Close(int fd) { - adb_close(fd); -} - int syntax_error(const char* fmt, ...) { fprintf(stderr, "adb: usage: "); diff --git a/debuggerd/Android.bp b/debuggerd/Android.bp index 1b20157b3..03b328727 100644 --- a/debuggerd/Android.bp +++ b/debuggerd/Android.bp @@ -43,7 +43,7 @@ cc_library_shared { export_include_dirs: ["tombstoned/include"], } -// Utility library to tombstoned and get an output fd. +// Utility library to talk to tombstoned and get an output fd. cc_library_static { name: "libtombstoned_client_static", defaults: ["debuggerd_defaults"], @@ -166,6 +166,9 @@ cc_library_static { local_include_dirs: ["libdebuggerd/include"], export_include_dirs: ["libdebuggerd/include"], + // Needed for private/bionic_fdsan.h + include_dirs: ["bionic/libc"], + static_libs: [ "libbacktrace", "libunwindstack", diff --git a/debuggerd/crash_dump.cpp b/debuggerd/crash_dump.cpp index 40cf6c340..93f757236 100644 --- a/debuggerd/crash_dump.cpp +++ b/debuggerd/crash_dump.cpp @@ -249,24 +249,48 @@ static void ParseArgs(int argc, char** argv, pid_t* pseudothread_tid, DebuggerdD } static void ReadCrashInfo(unique_fd& fd, siginfo_t* siginfo, - std::unique_ptr* regs, uintptr_t* abort_address) { + std::unique_ptr* regs, uintptr_t* abort_msg_address, + uintptr_t* fdsan_table_address) { std::aligned_storage::type buf; + CrashInfo* crash_info = reinterpret_cast(&buf); ssize_t rc = TEMP_FAILURE_RETRY(read(fd.get(), &buf, sizeof(buf))); if (rc == -1) { PLOG(FATAL) << "failed to read target ucontext"; - } else if (rc != sizeof(CrashInfo)) { - LOG(FATAL) << "read " << rc << " bytes when reading target crash information, expected " - << sizeof(CrashInfo); + } else { + ssize_t expected_size = 0; + switch (crash_info->header.version) { + case 1: + expected_size = sizeof(CrashInfoHeader) + sizeof(CrashInfoDataV1); + break; + + case 2: + expected_size = sizeof(CrashInfoHeader) + sizeof(CrashInfoDataV2); + break; + + default: + LOG(FATAL) << "unexpected CrashInfo version: " << crash_info->header.version; + break; + }; + + if (rc != expected_size) { + LOG(FATAL) << "read " << rc << " bytes when reading target crash information, expected " + << expected_size; + } } - CrashInfo* crash_info = reinterpret_cast(&buf); - if (crash_info->version != 1) { - LOG(FATAL) << "version mismatch, expected 1, received " << crash_info->version; - } + *fdsan_table_address = 0; + switch (crash_info->header.version) { + case 2: + *fdsan_table_address = crash_info->data.v2.fdsan_table_address; + case 1: + *abort_msg_address = crash_info->data.v1.abort_msg_address; + *siginfo = crash_info->data.v1.siginfo; + regs->reset(Regs::CreateFromUcontext(Regs::CurrentArch(), &crash_info->data.v1.ucontext)); + break; - *siginfo = crash_info->siginfo; - regs->reset(Regs::CreateFromUcontext(Regs::CurrentArch(), &crash_info->ucontext)); - *abort_address = crash_info->abort_msg_address; + default: + __builtin_unreachable(); + } } // Wait for a process to clone and return the child's pid. @@ -369,7 +393,8 @@ int main(int argc, char** argv) { ATRACE_NAME("after reparent"); pid_t pseudothread_tid; DebuggerdDumpType dump_type; - uintptr_t abort_address = 0; + uintptr_t abort_msg_address = 0; + uintptr_t fdsan_table_address = 0; Initialize(argv); ParseArgs(argc, argv, &pseudothread_tid, &dump_type); @@ -387,7 +412,7 @@ int main(int argc, char** argv) { OpenFilesList open_files; { ATRACE_NAME("open files"); - populate_open_files_list(g_target_thread, &open_files); + populate_open_files_list(&open_files, g_target_thread); } // In order to reduce the duration that we pause the process for, we ptrace @@ -429,7 +454,8 @@ int main(int argc, char** argv) { if (thread == g_target_thread) { // Read the thread's registers along with the rest of the crash info out of the pipe. - ReadCrashInfo(input_pipe, &siginfo, &info.registers, &abort_address); + ReadCrashInfo(input_pipe, &siginfo, &info.registers, &abort_msg_address, + &fdsan_table_address); info.siginfo = &siginfo; info.signo = info.siginfo->si_signo; } else { @@ -504,8 +530,8 @@ int main(int argc, char** argv) { g_output_fd = std::move(devnull); } - LOG(INFO) << "performing dump of process " << target_process << " (target tid = " << g_target_thread - << ")"; + LOG(INFO) << "performing dump of process " << target_process + << " (target tid = " << g_target_thread << ")"; int signo = siginfo.si_signo; bool fatal_signal = signo != DEBUGGER_SIGNAL; @@ -541,9 +567,16 @@ int main(int argc, char** argv) { ATRACE_NAME("dump_backtrace"); dump_backtrace(std::move(g_output_fd), map.get(), thread_info, g_target_thread); } else { - ATRACE_NAME("engrave_tombstone"); - engrave_tombstone(std::move(g_output_fd), map.get(), process_memory.get(), thread_info, - g_target_thread, abort_address, &open_files, &amfd_data); + { + ATRACE_NAME("fdsan table dump"); + populate_fdsan_table(&open_files, process_memory, fdsan_table_address); + } + + { + ATRACE_NAME("engrave_tombstone"); + engrave_tombstone(std::move(g_output_fd), map.get(), process_memory.get(), thread_info, + g_target_thread, abort_msg_address, &open_files, &amfd_data); + } } if (fatal_signal) { diff --git a/debuggerd/handler/debuggerd_handler.cpp b/debuggerd/handler/debuggerd_handler.cpp index 615fb46ad..91e6f7111 100644 --- a/debuggerd/handler/debuggerd_handler.cpp +++ b/debuggerd/handler/debuggerd_handler.cpp @@ -108,6 +108,7 @@ class ErrnoRestorer { int saved_errno_; }; +extern "C" void* android_fdsan_get_fd_table(); extern "C" void debuggerd_fallback_handler(siginfo_t*, ucontext_t*, void*); static debuggerd_callbacks_t g_callbacks; @@ -286,6 +287,7 @@ struct debugger_thread_info { siginfo_t* siginfo; void* ucontext; uintptr_t abort_msg; + uintptr_t fdsan_table; }; // Logging and contacting debuggerd requires free file descriptors, which we might not have. @@ -330,23 +332,23 @@ static int debuggerd_dispatch_pseudothread(void* arg) { } // ucontext_t is absurdly large on AArch64, so piece it together manually with writev. - uint32_t version = 1; - constexpr size_t expected = - sizeof(version) + sizeof(siginfo_t) + sizeof(ucontext_t) + sizeof(uintptr_t); + uint32_t version = 2; + constexpr size_t expected = sizeof(CrashInfoHeader) + sizeof(CrashInfoDataV2); errno = 0; if (fcntl(output_write.get(), F_SETPIPE_SZ, expected) < static_cast(expected)) { - fatal_errno("failed to set pipe bufer size"); + fatal_errno("failed to set pipe buffer size"); } - struct iovec iovs[4] = { + struct iovec iovs[5] = { {.iov_base = &version, .iov_len = sizeof(version)}, {.iov_base = thread_info->siginfo, .iov_len = sizeof(siginfo_t)}, {.iov_base = thread_info->ucontext, .iov_len = sizeof(ucontext_t)}, {.iov_base = &thread_info->abort_msg, .iov_len = sizeof(uintptr_t)}, + {.iov_base = &thread_info->fdsan_table, .iov_len = sizeof(uintptr_t)}, }; - ssize_t rc = TEMP_FAILURE_RETRY(writev(output_write.get(), iovs, 4)); + ssize_t rc = TEMP_FAILURE_RETRY(writev(output_write.get(), iovs, 5)); if (rc == -1) { fatal_errno("failed to write crash info"); } else if (rc != expected) { @@ -504,6 +506,7 @@ static void debuggerd_signal_handler(int signal_number, siginfo_t* info, void* c .siginfo = info, .ucontext = context, .abort_msg = reinterpret_cast(abort_message), + .fdsan_table = reinterpret_cast(android_fdsan_get_fd_table()), }; // Set PR_SET_DUMPABLE to 1, so that crash_dump can ptrace us. diff --git a/debuggerd/libdebuggerd/include/libdebuggerd/open_files_list.h b/debuggerd/libdebuggerd/include/libdebuggerd/open_files_list.h index 4727ca4d7..d47f2ddf6 100644 --- a/debuggerd/libdebuggerd/include/libdebuggerd/open_files_list.h +++ b/debuggerd/libdebuggerd/include/libdebuggerd/open_files_list.h @@ -14,23 +14,31 @@ * limitations under the License. */ -#ifndef _DEBUGGERD_OPEN_FILES_LIST_H -#define _DEBUGGERD_OPEN_FILES_LIST_H +#pragma once +#include #include +#include +#include #include #include -#include #include "utility.h" -typedef std::vector> OpenFilesList; +struct FDInfo { + std::optional path; + std::optional fdsan_owner; +}; -/* Populates the given list with open files for the given process. */ -void populate_open_files_list(pid_t pid, OpenFilesList* list); +using OpenFilesList = std::map; -/* Dumps the open files list to the log. */ +// Populates the given list with open files for the given process. +void populate_open_files_list(OpenFilesList* list, pid_t pid); + +// Populates the given list with the target process's fdsan table. +void populate_fdsan_table(OpenFilesList* list, std::shared_ptr memory, + uint64_t fdsan_table_address); + +// Dumps the open files list to the log. void dump_open_files_list(log_t* log, const OpenFilesList& files, const char* prefix); - -#endif // _DEBUGGERD_OPEN_FILES_LIST_H diff --git a/debuggerd/libdebuggerd/open_files_list.cpp b/debuggerd/libdebuggerd/open_files_list.cpp index b12703e14..1fdf2369a 100644 --- a/debuggerd/libdebuggerd/open_files_list.cpp +++ b/debuggerd/libdebuggerd/open_files_list.cpp @@ -32,10 +32,12 @@ #include #include +#include #include "libdebuggerd/utility.h" +#include "private/bionic_fdsan.h" -void populate_open_files_list(pid_t pid, OpenFilesList* list) { +void populate_open_files_list(OpenFilesList* list, pid_t pid) { std::string fd_dir_name = "/proc/" + std::to_string(pid) + "/fd"; std::unique_ptr dir(opendir(fd_dir_name.c_str()), closedir); if (dir == nullptr) { @@ -53,17 +55,84 @@ void populate_open_files_list(pid_t pid, OpenFilesList* list) { std::string path = fd_dir_name + "/" + std::string(de->d_name); std::string target; if (android::base::Readlink(path, &target)) { - list->emplace_back(fd, target); + (*list)[fd].path = target; } else { + (*list)[fd].path = "???"; ALOGE("failed to readlink %s: %s", path.c_str(), strerror(errno)); - list->emplace_back(fd, "???"); } } } +void populate_fdsan_table(OpenFilesList* list, std::shared_ptr memory, + uint64_t fdsan_table_address) { + constexpr size_t inline_fds = sizeof(FdTable::entries) / sizeof(*FdTable::entries); + static_assert(inline_fds == 128); + size_t entry_offset = offsetof(FdTable, entries); + for (size_t i = 0; i < inline_fds; ++i) { + uint64_t address = fdsan_table_address + entry_offset + sizeof(FdEntry) * i; + FdEntry entry; + if (!memory->Read(address, &entry, sizeof(entry))) { + ALOGE("failed to read fdsan table entry %zu: %s", i, strerror(errno)); + return; + } + ALOGE("fd %zu = %#" PRIx64, i, entry.close_tag.load()); + if (entry.close_tag) { + (*list)[i].fdsan_owner = entry.close_tag.load(); + } + } + + size_t overflow_offset = offsetof(FdTable, overflow); + uintptr_t overflow = 0; + if (!memory->Read(fdsan_table_address + overflow_offset, &overflow, sizeof(overflow))) { + ALOGE("failed to read fdsan table overflow pointer: %s", strerror(errno)); + return; + } + + if (!overflow) { + return; + } + + size_t overflow_length; + if (!memory->Read(overflow, &overflow_length, sizeof(overflow_length))) { + ALOGE("failed to read fdsan overflow table length: %s", strerror(errno)); + return; + } + + if (overflow_length > 131072) { + ALOGE("unreasonable large fdsan overflow table size %zu, bailing out", overflow_length); + return; + } + + for (size_t i = 0; i < overflow_length; ++i) { + int fd = i + inline_fds; + uint64_t address = overflow + offsetof(FdTableOverflow, entries) + i * sizeof(FdEntry); + FdEntry entry; + if (!memory->Read(address, &entry, sizeof(entry))) { + ALOGE("failed to read fdsan overflow entry for fd %d: %s", fd, strerror(errno)); + return; + } + if (entry.close_tag) { + (*list)[fd].fdsan_owner = entry.close_tag; + } + } + return; +} + void dump_open_files_list(log_t* log, const OpenFilesList& files, const char* prefix) { - for (auto& file : files) { - _LOG(log, logtype::OPEN_FILES, "%sfd %i: %s\n", prefix, file.first, file.second.c_str()); + for (auto& [fd, entry] : files) { + const std::optional& path = entry.path; + const std::optional& fdsan_owner = entry.fdsan_owner; + if (path && fdsan_owner) { + _LOG(log, logtype::OPEN_FILES, "%sfd %i: %s (owned by %#" PRIx64 ")\n", prefix, fd, + path->c_str(), *fdsan_owner); + } else if (path && !fdsan_owner) { + _LOG(log, logtype::OPEN_FILES, "%sfd %i: %s (unowned)\n", prefix, fd, path->c_str()); + } else if (!path && fdsan_owner) { + _LOG(log, logtype::OPEN_FILES, "%sfd %i: (owned by %#" PRIx64 ")\n", prefix, fd, + *fdsan_owner); + } else { + ALOGE("OpenFilesList contains an entry (fd %d) with no path or owner", fd); + } } } diff --git a/debuggerd/libdebuggerd/test/open_files_list_test.cpp b/debuggerd/libdebuggerd/test/open_files_list_test.cpp index acac72c22..d7036fd39 100644 --- a/debuggerd/libdebuggerd/test/open_files_list_test.cpp +++ b/debuggerd/libdebuggerd/test/open_files_list_test.cpp @@ -34,13 +34,13 @@ TEST(OpenFilesListTest, BasicTest) { // Get the list of open files for this process. OpenFilesList list; - populate_open_files_list(getpid(), &list); + populate_open_files_list(&list, getpid()); // Verify our open file is in the list. bool found = false; - for (auto& file : list) { + for (auto& file : list) { if (file.first == tf.fd) { - EXPECT_EQ(file.second, std::string(tf.path)); + EXPECT_EQ(file.second.path.value_or(""), std::string(tf.path)); found = true; break; } diff --git a/debuggerd/protocol.h b/debuggerd/protocol.h index 6903b0e55..bfd0fbbe2 100644 --- a/debuggerd/protocol.h +++ b/debuggerd/protocol.h @@ -81,9 +81,24 @@ struct InterceptResponse { }; // Sent from handler to crash_dump via pipe. -struct __attribute__((__packed__)) CrashInfo { - uint32_t version; // must be 1. +struct __attribute__((__packed__)) CrashInfoHeader { + uint32_t version; +}; + +struct __attribute__((__packed__)) CrashInfoDataV1 { siginfo_t siginfo; ucontext_t ucontext; uintptr_t abort_msg_address; }; + +struct __attribute__((__packed__)) CrashInfoDataV2 : public CrashInfoDataV1 { + uintptr_t fdsan_table_address; +}; + +struct __attribute__((__packed__)) CrashInfo { + CrashInfoHeader header; + union { + CrashInfoDataV1 v1; + CrashInfoDataV2 v2; + } data; +};