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.

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

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

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>

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: ");
 

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",

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<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 @@ 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) {

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<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 @@ static void debuggerd_signal_handler(int signal_number, siginfo_t* info, void* c
       .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.

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

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 @@ 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<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);
+    }
   }
 }
 

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;
     }

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;
+};