linux: Read thread IDs via a PtraceConnection

Bug: crashpad:250
Change-Id: I2ff9c2d810f7af25f7438e974e0adfb5abebec16
Reviewed-on: https://chromium-review.googlesource.com/1200962
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>

snapshot/linux/process_reader_linux.cc

@@ -17,7 +17,6 @@
 #include <elf.h>
 #include <errno.h>
 #include <sched.h>
-#include <stdio.h>
 #include <string.h>
 #include <sys/resource.h>
 #include <unistd.h>
@@ -25,13 +24,10 @@
 #include <algorithm>
 
 #include "base/logging.h"
-#include "base/strings/string_number_conversions.h"
 #include "build/build_config.h"
 #include "snapshot/linux/debug_rendezvous.h"
-#include "util/file/directory_reader.h"
 #include "util/linux/auxiliary_vector.h"
 #include "util/linux/proc_stat_reader.h"
-#include "util/misc/as_underlying_type.h"
 
 namespace crashpad {
 
@@ -298,23 +294,11 @@ void ProcessReaderLinux::InitializeThreads() {
     LOG(WARNING) << "Couldn't initialize main thread.";
   }
 
-  char path[32];
-  snprintf(path, arraysize(path), "/proc/%d/task", pid);
   bool main_thread_found = false;
-  DirectoryReader reader;
+  std::vector<pid_t> thread_ids;
-  if (!reader.Open(base::FilePath(path))) {
+  bool result = connection_->Threads(&thread_ids);
-    return;
+  DCHECK(result);
-  }
+  for (pid_t tid : thread_ids) {
-  base::FilePath tid_str;
-  DirectoryReader::Result result;
-  while ((result = reader.NextFile(&tid_str)) ==
-         DirectoryReader::Result::kSuccess) {
-    pid_t tid;
-    if (!base::StringToInt(tid_str.value(), &tid)) {
-      LOG(ERROR) << "format error";
-      continue;
-    }
-
     if (tid == pid) {
       DCHECK(!main_thread_found);
       main_thread_found = true;
@@ -328,8 +312,6 @@ void ProcessReaderLinux::InitializeThreads() {
       threads_.push_back(thread);
     }
   }
-  DCHECK_EQ(AsUnderlyingType(result),
-            AsUnderlyingType(DirectoryReader::Result::kNoMoreFiles));
   DCHECK(main_thread_found);
 }
 

test/linux/fake_ptrace_connection.cc

@@ -92,5 +92,11 @@ ProcessMemory* FakePtraceConnection::Memory() {
   return memory_.get();
 }
 
+bool FakePtraceConnection::Threads(std::vector<pid_t>* threads) {
+  // TODO(jperaza): Implement this if/when it's needed.
+  NOTREACHED();
+  return false;
+}
+
 }  // namespace test
 }  // namespace crashpad

test/linux/fake_ptrace_connection.h

@@ -62,6 +62,9 @@ class FakePtraceConnection : public PtraceConnection {
   //!     ADD_FAILURE() and returning `nullptr` on failure.
   ProcessMemory* Memory() override;
 
+  //! \todo Not yet implemented.
+  bool Threads(std::vector<pid_t>* threads) override;
+
  private:
   std::set<pid_t> attachments_;
   std::unique_ptr<ProcessMemoryLinux> memory_;

util/linux/direct_ptrace_connection.cc

@@ -14,9 +14,15 @@
 
 #include "util/linux/direct_ptrace_connection.h"
 
+#include <stdio.h>
+
 #include <utility>
 
+#include "base/logging.h"
+#include "base/strings/string_number_conversions.h"
+#include "util/file/directory_reader.h"
 #include "util/file/file_io.h"
+#include "util/misc/as_underlying_type.h"
 
 namespace crashpad {
 
@@ -81,4 +87,35 @@ ProcessMemory* DirectPtraceConnection::Memory() {
   return &memory_;
 }
 
+bool DirectPtraceConnection::Threads(std::vector<pid_t>* threads) {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  DCHECK(threads->empty());
+
+  char path[32];
+  snprintf(path, arraysize(path), "/proc/%d/task", pid_);
+  DirectoryReader reader;
+  if (!reader.Open(base::FilePath(path))) {
+    return false;
+  }
+
+  std::vector<pid_t> local_threads;
+  base::FilePath tid_str;
+  DirectoryReader::Result result;
+  while ((result = reader.NextFile(&tid_str)) ==
+         DirectoryReader::Result::kSuccess) {
+    pid_t tid;
+    if (!base::StringToInt(tid_str.value(), &tid)) {
+      LOG(ERROR) << "format error";
+      continue;
+    }
+
+    local_threads.push_back(tid);
+  }
+  DCHECK_EQ(AsUnderlyingType(result),
+            AsUnderlyingType(DirectoryReader::Result::kNoMoreFiles));
+
+  threads->swap(local_threads);
+  return true;
+}
+
 }  // namespace crashpad

util/linux/direct_ptrace_connection.h

@@ -56,6 +56,7 @@ class DirectPtraceConnection : public PtraceConnection {
   bool ReadFileContents(const base::FilePath& path,
                         std::string* contents) override;
   ProcessMemory* Memory() override;
+  bool Threads(std::vector<pid_t>* threads) override;
 
  private:
   std::vector<std::unique_ptr<ScopedPtraceAttach>> attachments_;

util/linux/ptrace_broker.cc

@@ -17,6 +17,7 @@
 #include <fcntl.h>
 #include <limits.h>
 #include <string.h>
+#include <syscall.h>
 #include <unistd.h>
 
 #include <algorithm>
@@ -192,7 +193,9 @@ int PtraceBroker::RunImpl() {
 
       case Request::kTypeReadFile: {
         ScopedFileHandle handle;
-        int result = ReceiveAndOpenFilePath(request.path.path_length, &handle);
+        int result = ReceiveAndOpenFilePath(request.path.path_length,
+                                            /* is_directory= */ false,
+                                            &handle);
         if (result != 0) {
           return result;
         }
@@ -217,6 +220,26 @@ int PtraceBroker::RunImpl() {
         continue;
       }
 
+      case Request::kTypeListDirectory: {
+        ScopedFileHandle handle;
+        int result = ReceiveAndOpenFilePath(request.path.path_length,
+                                            /* is_directory= */ true,
+                                            &handle);
+        if (result != 0) {
+          return result;
+        }
+
+        if (!handle.is_valid()) {
+          continue;
+        }
+
+        result = SendDirectory(handle.get());
+        if (result != 0) {
+          return result;
+        }
+        continue;
+      }
+
       case Request::kTypeExit:
         return 0;
     }
@@ -329,7 +352,32 @@ int PtraceBroker::SendMemory(pid_t pid, VMAddress address, VMSize size) {
   return 0;
 }
 
+int PtraceBroker::SendDirectory(FileHandle handle) {
+  char buffer[4096];
+  int rv;
+  do {
+    rv = syscall(SYS_getdents64, handle, buffer, sizeof(buffer));
+
+    if (rv < 0) {
+      return SendReadError(static_cast<ReadError>(errno));
+    }
+
+    if (!WriteFile(sock_, &rv, sizeof(rv))) {
+      return errno;
+    }
+
+    if (rv > 0) {
+      if (!WriteFile(sock_, buffer, static_cast<size_t>(rv))) {
+        return errno;
+      }
+    }
+  } while (rv > 0);
+
+  return 0;
+}
+
 int PtraceBroker::ReceiveAndOpenFilePath(VMSize path_length,
+                                         bool is_directory,
                                          ScopedFileHandle* handle) {
   char path[std::max(4096, PATH_MAX)];
 
@@ -346,8 +394,11 @@ int PtraceBroker::ReceiveAndOpenFilePath(VMSize path_length,
     return SendOpenResult(kOpenResultAccessDenied);
   }
 
-  ScopedFileHandle local_handle(
+  int flags = O_RDONLY | O_CLOEXEC | O_NOCTTY;
-      HANDLE_EINTR(open(path, O_RDONLY | O_CLOEXEC | O_NOCTTY)));
+  if (is_directory) {
+    flags |= O_DIRECTORY;
+  }
+  ScopedFileHandle local_handle(HANDLE_EINTR(open(path, flags)));
   if (!local_handle.is_valid()) {
     return SendOpenResult(static_cast<OpenResult>(errno));
   }

util/linux/ptrace_broker.h

@@ -75,6 +75,16 @@ class PtraceBroker {
       //!     errors, followed by an Errno. On success, the bytes read follow.
       kTypeReadFile,
 
+      //! \brief Reads the contents of a directory. The data is returned in a
+      //!     series of messages. The first message is an OpenResult, indicating
+      //!     the validity of the received file path. If the OpenResult is
+      //!     kOpenResultSuccess, the subsequent messages return the contents of
+      //!     the directory as a dirent stream, as read by `getdents64()`. Each
+      //!     subsequent message begins with an int32_t indicating the number of
+      //!     bytes read, 0 for end-of-file, or -1 for errors, followed by an
+      //!     Errno. On success, the bytes read follow.
+      kTypeListDirectory,
+
       //! \brief Causes the broker to return from Run(), detaching all attached
       //!     threads. Does not respond.
       kTypeExit
@@ -190,9 +200,12 @@ class PtraceBroker {
   int SendReadError(ReadError err);
   int SendOpenResult(OpenResult result);
   int SendFileContents(FileHandle handle);
+  int SendDirectory(FileHandle handle);
   void TryOpeningMemFile();
   int SendMemory(pid_t pid, VMAddress address, VMSize size);
-  int ReceiveAndOpenFilePath(VMSize path_length, ScopedFileHandle* handle);
+  int ReceiveAndOpenFilePath(VMSize path_length,
+                             bool is_directory,
+                             ScopedFileHandle* handle);
 
   char file_root_buffer_[32];
   Ptracer ptracer_;

util/linux/ptrace_broker_test.cc

@@ -155,6 +155,17 @@ class SameBitnessTest : public Multiprocess {
         client_sock.get(), ChildPID(), /* try_direct_memory= */ false));
 
     EXPECT_EQ(client.GetProcessID(), ChildPID());
+
+    std::vector<pid_t> threads;
+    ASSERT_TRUE(client.Threads(&threads));
+    EXPECT_EQ(threads.size(), 2u);
+    if (threads[0] == ChildPID()) {
+      EXPECT_EQ(threads[1], child2_tid);
+    } else {
+      EXPECT_EQ(threads[0], child2_tid);
+      EXPECT_EQ(threads[1], ChildPID());
+    }
+
     EXPECT_TRUE(client.Attach(child2_tid));
     EXPECT_EQ(client.Is64Bit(), am_64_bit);
 

util/linux/ptrace_client.cc

@@ -20,6 +20,7 @@
 #include <string>
 
 #include "base/logging.h"
+#include "base/strings/string_number_conversions.h"
 #include "util/file/file_io.h"
 #include "util/linux/ptrace_broker.h"
 #include "util/process/process_memory_linux.h"
@@ -80,6 +81,48 @@ bool AttachImpl(int sock, pid_t tid) {
   return true;
 }
 
+struct Dirent64 {
+  ino64_t d_ino;
+  off64_t d_off;
+  unsigned short d_reclen;
+  unsigned char d_type;
+  char d_name[];
+};
+
+void ReadDentsAsThreadIDs(char* buffer,
+                          size_t size,
+                          std::vector<pid_t>* threads) {
+  while (size > sizeof(Dirent64)) {
+    auto dirent = reinterpret_cast<Dirent64*>(buffer);
+    if (size < dirent->d_reclen) {
+      LOG(ERROR) << "short dirent";
+      break;
+    }
+    buffer += dirent->d_reclen;
+    size -= dirent->d_reclen;
+
+    const size_t max_name_length =
+        dirent->d_reclen - offsetof(Dirent64, d_name);
+    size_t name_len = strnlen(dirent->d_name, max_name_length);
+    if (name_len >= max_name_length) {
+      LOG(ERROR) << "format error";
+      break;
+    }
+
+    if (strcmp(dirent->d_name, ".") == 0 || strcmp(dirent->d_name, "..") == 0) {
+      continue;
+    }
+
+    pid_t tid;
+    if (!base::StringToInt(dirent->d_name, &tid)) {
+      LOG(ERROR) << "format error";
+      continue;
+    }
+    threads->push_back(tid);
+  }
+  DCHECK_EQ(size, 0u);
+}
+
 }  // namespace
 
 PtraceClient::PtraceClient()
@@ -218,6 +261,51 @@ ProcessMemory* PtraceClient::Memory() {
   return memory_.get();
 }
 
+bool PtraceClient::Threads(std::vector<pid_t>* threads) {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  DCHECK(threads->empty());
+
+  // If the broker is unable to read thread IDs, fall-back to just the main
+  // thread's ID.
+  threads->push_back(pid_);
+
+  char path[32];
+  snprintf(path, arraysize(path), "/proc/%d/task", pid_);
+
+  PtraceBroker::Request request;
+  request.type = PtraceBroker::Request::kTypeListDirectory;
+  request.path.path_length = strlen(path);
+
+  if (!LoggingWriteFile(sock_, &request, sizeof(request)) ||
+      !SendFilePath(path, request.path.path_length)) {
+    return false;
+  }
+
+  std::vector<pid_t> local_threads;
+  int32_t read_result;
+  do {
+    if (!LoggingReadFileExactly(sock_, &read_result, sizeof(read_result))) {
+      return false;
+    }
+
+    if (read_result < 0) {
+      return ReceiveAndLogReadError(sock_, "Threads");
+    }
+
+    if (read_result > 0) {
+      auto buffer = std::make_unique<char[]>(read_result);
+      if (!LoggingReadFileExactly(sock_, buffer.get(), read_result)) {
+        return false;
+      }
+
+      ReadDentsAsThreadIDs(buffer.get(), read_result, &local_threads);
+    }
+  } while (read_result > 0);
+
+  threads->swap(local_threads);
+  return true;
+}
+
 PtraceClient::BrokeredMemory::BrokeredMemory(PtraceClient* client)
     : ProcessMemory(), client_(client) {}
 

util/linux/ptrace_client.h

@@ -61,6 +61,7 @@ class PtraceClient : public PtraceConnection {
   bool ReadFileContents(const base::FilePath& path,
                         std::string* contents) override;
   ProcessMemory* Memory() override;
+  bool Threads(std::vector<pid_t>* threads) override;
 
  private:
   class BrokeredMemory : public ProcessMemory {

util/linux/ptrace_connection.h

@@ -18,6 +18,7 @@
 #include <sys/types.h>
 
 #include <string>
+#include <vector>
 
 #include "base/files/file_path.h"
 #include "util/linux/thread_info.h"
@@ -64,6 +65,14 @@ class PtraceConnection {
   //! The caller does not take ownership of the reader. The reader is valid for
   //! the lifetime of the PtraceConnection that created it.
   virtual ProcessMemory* Memory() = 0;
+
+  //! \brief Determines the thread IDs of the threads in the connected process.
+  //!
+  //! \param[out] threads The list of thread IDs.
+  //! \return `true` on success, `false` on failure with a message logged. If
+  //!     this method returns `false`, \a threads may contain a partial list of
+  //!     thread IDs.
+  virtual bool Threads(std::vector<pid_t>* threads) = 0;
 };
 
 }  // namespace crashpad