From b918119ca20a4b5c2b4a1a7dc59690944bce1140 Mon Sep 17 00:00:00 2001 From: Joshua Peraza Date: Tue, 11 Sep 2018 14:20:03 -0700 Subject: [PATCH] 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 Reviewed-by: Mark Mentovai --- snapshot/linux/process_reader_linux.cc | 26 ++------ test/linux/fake_ptrace_connection.cc | 6 ++ test/linux/fake_ptrace_connection.h | 3 + util/linux/direct_ptrace_connection.cc | 37 +++++++++++ util/linux/direct_ptrace_connection.h | 1 + util/linux/ptrace_broker.cc | 57 ++++++++++++++++- util/linux/ptrace_broker.h | 15 ++++- util/linux/ptrace_broker_test.cc | 11 ++++ util/linux/ptrace_client.cc | 88 ++++++++++++++++++++++++++ util/linux/ptrace_client.h | 1 + util/linux/ptrace_connection.h | 9 +++ 11 files changed, 228 insertions(+), 26 deletions(-) diff --git a/snapshot/linux/process_reader_linux.cc b/snapshot/linux/process_reader_linux.cc index 43590116..1e0550d2 100644 --- a/snapshot/linux/process_reader_linux.cc +++ b/snapshot/linux/process_reader_linux.cc @@ -17,7 +17,6 @@ #include #include #include -#include #include #include #include @@ -25,13 +24,10 @@ #include #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; - if (!reader.Open(base::FilePath(path))) { - return; - } - 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; - } - + std::vector thread_ids; + bool result = connection_->Threads(&thread_ids); + DCHECK(result); + for (pid_t tid : thread_ids) { 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); } diff --git a/test/linux/fake_ptrace_connection.cc b/test/linux/fake_ptrace_connection.cc index 022b1327..b03228a2 100644 --- a/test/linux/fake_ptrace_connection.cc +++ b/test/linux/fake_ptrace_connection.cc @@ -92,5 +92,11 @@ ProcessMemory* FakePtraceConnection::Memory() { return memory_.get(); } +bool FakePtraceConnection::Threads(std::vector* threads) { + // TODO(jperaza): Implement this if/when it's needed. + NOTREACHED(); + return false; +} + } // namespace test } // namespace crashpad diff --git a/test/linux/fake_ptrace_connection.h b/test/linux/fake_ptrace_connection.h index 6597c473..01a6f925 100644 --- a/test/linux/fake_ptrace_connection.h +++ b/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* threads) override; + private: std::set attachments_; std::unique_ptr memory_; diff --git a/util/linux/direct_ptrace_connection.cc b/util/linux/direct_ptrace_connection.cc index a3df425f..ac44bf13 100644 --- a/util/linux/direct_ptrace_connection.cc +++ b/util/linux/direct_ptrace_connection.cc @@ -14,9 +14,15 @@ #include "util/linux/direct_ptrace_connection.h" +#include + #include +#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* 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 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 diff --git a/util/linux/direct_ptrace_connection.h b/util/linux/direct_ptrace_connection.h index 53594ef9..10f8370c 100644 --- a/util/linux/direct_ptrace_connection.h +++ b/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* threads) override; private: std::vector> attachments_; diff --git a/util/linux/ptrace_broker.cc b/util/linux/ptrace_broker.cc index e9d30686..4d7e4ced 100644 --- a/util/linux/ptrace_broker.cc +++ b/util/linux/ptrace_broker.cc @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -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(errno)); + } + + if (!WriteFile(sock_, &rv, sizeof(rv))) { + return errno; + } + + if (rv > 0) { + if (!WriteFile(sock_, buffer, static_cast(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( - HANDLE_EINTR(open(path, O_RDONLY | O_CLOEXEC | O_NOCTTY))); + int flags = 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(errno)); } diff --git a/util/linux/ptrace_broker.h b/util/linux/ptrace_broker.h index 2e5decdf..47dd4d9b 100644 --- a/util/linux/ptrace_broker.h +++ b/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_; diff --git a/util/linux/ptrace_broker_test.cc b/util/linux/ptrace_broker_test.cc index 9ed1973b..341f2c80 100644 --- a/util/linux/ptrace_broker_test.cc +++ b/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 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); diff --git a/util/linux/ptrace_client.cc b/util/linux/ptrace_client.cc index b5ccebaf..aa3e0244 100644 --- a/util/linux/ptrace_client.cc +++ b/util/linux/ptrace_client.cc @@ -20,6 +20,7 @@ #include #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* threads) { + while (size > sizeof(Dirent64)) { + auto dirent = reinterpret_cast(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* 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 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(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) {} diff --git a/util/linux/ptrace_client.h b/util/linux/ptrace_client.h index a7abc0cf..b4f27ae7 100644 --- a/util/linux/ptrace_client.h +++ b/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* threads) override; private: class BrokeredMemory : public ProcessMemory { diff --git a/util/linux/ptrace_connection.h b/util/linux/ptrace_connection.h index 86dc2dd9..21117475 100644 --- a/util/linux/ptrace_connection.h +++ b/util/linux/ptrace_connection.h @@ -18,6 +18,7 @@ #include #include +#include #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* threads) = 0; }; } // namespace crashpad