Merge changes from topic "fdsan_debuggerd"
* changes:
adb: actually enable fdsan.
adb: move AdbCloser to its rightful place.
libdebuggerd: extract and print the fdsan table.
crash_dump: pass the address of the fdsan table.
diff --git a/adb/adb.cpp b/adb/adb.cpp
index 38c6f62..a8fe736 100644
--- a/adb/adb.cpp
+++ b/adb/adb.cpp
@@ -891,6 +891,10 @@
// 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 2079be1..dec73bc 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 bad501a..d47213d 100644
--- a/adb/adb_unique_fd.h
+++ b/adb/adb_unique_fd.h
@@ -21,15 +21,15 @@
#include <android-base/unique_fd.h>
+#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<AdbCloser>;
-
-#if !defined(_WIN32)
-bool Pipe(unique_fd* read, unique_fd* write, int flags = 0);
+#else
+using unique_fd = android::base::unique_fd;
#endif
template <typename T>
diff --git a/adb/adb_utils.cpp b/adb/adb_utils.cpp
index 0c3327f..ffac315 100644
--- a/adb/adb_utils.cpp
+++ b/adb/adb_utils.cpp
@@ -274,10 +274,6 @@
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 1b20157..03b3287 100644
--- a/debuggerd/Android.bp
+++ b/debuggerd/Android.bp
@@ -43,7 +43,7 @@
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 @@
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 40cf6c3..93f7572 100644
--- a/debuggerd/crash_dump.cpp
+++ b/debuggerd/crash_dump.cpp
@@ -249,24 +249,48 @@
}
static void ReadCrashInfo(unique_fd& fd, siginfo_t* siginfo,
- std::unique_ptr<unwindstack::Regs>* regs, uintptr_t* abort_address) {
+ std::unique_ptr<unwindstack::Regs>* regs, uintptr_t* abort_msg_address,
+ uintptr_t* fdsan_table_address) {
std::aligned_storage<sizeof(CrashInfo) + 1, alignof(CrashInfo)>::type buf;
+ CrashInfo* crash_info = reinterpret_cast<CrashInfo*>(&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<CrashInfo*>(&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 @@
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 @@
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 @@
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 @@
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 @@
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 615fb46..91e6f71 100644
--- a/debuggerd/handler/debuggerd_handler.cpp
+++ b/debuggerd/handler/debuggerd_handler.cpp
@@ -108,6 +108,7 @@
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 @@
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 @@
}
// 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<int>(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 @@
.siginfo = info,
.ucontext = context,
.abort_msg = reinterpret_cast<uintptr_t>(abort_message),
+ .fdsan_table = reinterpret_cast<uintptr_t>(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 4727ca4..d47f2dd 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 <stdint.h>
#include <sys/types.h>
+#include <map>
+#include <optional>
#include <string>
#include <utility>
-#include <vector>
#include "utility.h"
-typedef std::vector<std::pair<int, std::string>> OpenFilesList;
+struct FDInfo {
+ std::optional<std::string> path;
+ std::optional<uint64_t> 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<int, FDInfo>;
-/* 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<unwindstack::Memory> 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 b12703e..1fdf236 100644
--- a/debuggerd/libdebuggerd/open_files_list.cpp
+++ b/debuggerd/libdebuggerd/open_files_list.cpp
@@ -32,10 +32,12 @@
#include <android-base/file.h>
#include <log/log.h>
+#include <unwindstack/Memory.h>
#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, int (*)(DIR*)> dir(opendir(fd_dir_name.c_str()), closedir);
if (dir == nullptr) {
@@ -53,17 +55,84 @@
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<unwindstack::Memory> 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<std::string>& path = entry.path;
+ const std::optional<uint64_t>& 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: <MISSING> (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 acac72c..d7036fd 100644
--- a/debuggerd/libdebuggerd/test/open_files_list_test.cpp
+++ b/debuggerd/libdebuggerd/test/open_files_list_test.cpp
@@ -34,13 +34,13 @@
// 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 6903b0e..bfd0fbb 100644
--- a/debuggerd/protocol.h
+++ b/debuggerd/protocol.h
@@ -81,9 +81,24 @@
};
// 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;
+};