From 10fd672bde9d8c4283da732b8ac0a58decbcc533 Mon Sep 17 00:00:00 2001 From: Joshua Peraza Date: Thu, 5 Apr 2018 13:00:15 -0700 Subject: [PATCH] linux: Enable brokered memory reading This change: 1. Updates the broker's memory reading protocol to enable short reads. 2. Updates Ptracer to allow short reads. 3. Updates the broker to allow reading from a memory file. 4. Updates the broker's default file root to be "/proc/[pid]/". 5. Adds PtraceConnection::Memory() to produce a suitable memory reader for a connection type. Bug: crashpad:30 Change-Id: I8c004016065d981acd1fa74ad1b8e51ce07c7c85 Reviewed-on: https://chromium-review.googlesource.com/991455 Reviewed-by: Mark Mentovai Commit-Queue: Joshua Peraza --- snapshot/linux/process_reader_linux.cc | 6 - snapshot/linux/process_reader_linux.h | 4 +- test/linux/fake_ptrace_connection.cc | 15 +++ test/linux/fake_ptrace_connection.h | 7 ++ util/linux/direct_ptrace_connection.cc | 10 ++ util/linux/direct_ptrace_connection.h | 3 + util/linux/exception_handler_client.cc | 2 +- util/linux/ptrace_broker.cc | 113 +++++++++++++++--- util/linux/ptrace_broker.h | 39 ++++-- util/linux/ptrace_broker_test.cc | 159 +++++++++++++++---------- util/linux/ptrace_client.cc | 134 +++++++++++++++------ util/linux/ptrace_client.h | 44 ++++--- util/linux/ptrace_connection.h | 7 ++ util/linux/ptracer.cc | 69 +++++++---- util/linux/ptracer.h | 19 ++- util/process/process_memory.h | 3 +- 16 files changed, 442 insertions(+), 192 deletions(-) diff --git a/snapshot/linux/process_reader_linux.cc b/snapshot/linux/process_reader_linux.cc index f59b54f1..8e25e77d 100644 --- a/snapshot/linux/process_reader_linux.cc +++ b/snapshot/linux/process_reader_linux.cc @@ -182,7 +182,6 @@ ProcessReaderLinux::ProcessReaderLinux() threads_(), modules_(), elf_readers_(), - process_memory_(), is_64_bit_(false), initialized_threads_(false), initialized_modules_(false), @@ -203,11 +202,6 @@ bool ProcessReaderLinux::Initialize(PtraceConnection* connection) { return false; } - pid_t pid = connection->GetProcessID(); - if (!process_memory_.Initialize(pid)) { - return false; - } - is_64_bit_ = process_info_.Is64Bit(); INITIALIZATION_STATE_SET_VALID(initialized_); diff --git a/snapshot/linux/process_reader_linux.h b/snapshot/linux/process_reader_linux.h index 2e89655e..27e2cea8 100644 --- a/snapshot/linux/process_reader_linux.h +++ b/snapshot/linux/process_reader_linux.h @@ -32,7 +32,6 @@ #include "util/misc/initialization_state_dcheck.h" #include "util/posix/process_info.h" #include "util/process/process_memory.h" -#include "util/process/process_memory_linux.h" namespace crashpad { @@ -103,7 +102,7 @@ class ProcessReaderLinux { pid_t ParentProcessID() const { return process_info_.ParentProcessID(); } //! \brief Return a memory reader for the target process. - ProcessMemory* Memory() { return &process_memory_; } + ProcessMemory* Memory() { return connection_->Memory(); } //! \brief Return a memory map of the target process. MemoryMap* GetMemoryMap() { return &memory_map_; } @@ -146,7 +145,6 @@ class ProcessReaderLinux { std::vector threads_; std::vector modules_; std::vector> elf_readers_; - ProcessMemoryLinux process_memory_; bool is_64_bit_; bool initialized_threads_; bool initialized_modules_; diff --git a/test/linux/fake_ptrace_connection.cc b/test/linux/fake_ptrace_connection.cc index 831842eb..022b1327 100644 --- a/test/linux/fake_ptrace_connection.cc +++ b/test/linux/fake_ptrace_connection.cc @@ -14,6 +14,8 @@ #include "test/linux/fake_ptrace_connection.h" +#include + #include "build/build_config.h" #include "gtest/gtest.h" #include "util/file/file_io.h" @@ -77,5 +79,18 @@ bool FakePtraceConnection::ReadFileContents(const base::FilePath& path, return LoggingReadEntireFile(path, contents); } +ProcessMemory* FakePtraceConnection::Memory() { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + if (!memory_) { + auto mem = std::make_unique(); + if (mem->Initialize(pid_)) { + memory_ = std::move(mem); + } else { + ADD_FAILURE(); + } + } + return memory_.get(); +} + } // namespace test } // namespace crashpad diff --git a/test/linux/fake_ptrace_connection.h b/test/linux/fake_ptrace_connection.h index 04911dd4..6597c473 100644 --- a/test/linux/fake_ptrace_connection.h +++ b/test/linux/fake_ptrace_connection.h @@ -17,11 +17,13 @@ #include +#include #include #include "base/macros.h" #include "util/linux/ptrace_connection.h" #include "util/misc/initialization_state_dcheck.h" +#include "util/process/process_memory_linux.h" namespace crashpad { namespace test { @@ -56,8 +58,13 @@ class FakePtraceConnection : public PtraceConnection { bool ReadFileContents(const base::FilePath& path, std::string* contents) override; + //! \brief Attempts to create a ProcessMemory when called, calling + //! ADD_FAILURE() and returning `nullptr` on failure. + ProcessMemory* Memory() override; + private: std::set attachments_; + std::unique_ptr memory_; pid_t pid_; bool is_64_bit_; InitializationStateDcheck initialized_; diff --git a/util/linux/direct_ptrace_connection.cc b/util/linux/direct_ptrace_connection.cc index f2cf40a3..a3df425f 100644 --- a/util/linux/direct_ptrace_connection.cc +++ b/util/linux/direct_ptrace_connection.cc @@ -23,6 +23,7 @@ namespace crashpad { DirectPtraceConnection::DirectPtraceConnection() : PtraceConnection(), attachments_(), + memory_(), pid_(-1), ptracer_(/* can_log= */ true), initialized_() {} @@ -37,6 +38,10 @@ bool DirectPtraceConnection::Initialize(pid_t pid) { } pid_ = pid; + if (!memory_.Initialize(pid)) { + return false; + } + INITIALIZATION_STATE_SET_VALID(initialized_); return true; } @@ -71,4 +76,9 @@ bool DirectPtraceConnection::ReadFileContents(const base::FilePath& path, return LoggingReadEntireFile(path, contents); } +ProcessMemory* DirectPtraceConnection::Memory() { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + return &memory_; +} + } // namespace crashpad diff --git a/util/linux/direct_ptrace_connection.h b/util/linux/direct_ptrace_connection.h index fca8cafa..53594ef9 100644 --- a/util/linux/direct_ptrace_connection.h +++ b/util/linux/direct_ptrace_connection.h @@ -25,6 +25,7 @@ #include "util/linux/ptracer.h" #include "util/linux/scoped_ptrace_attach.h" #include "util/misc/initialization_state_dcheck.h" +#include "util/process/process_memory_linux.h" namespace crashpad { @@ -54,9 +55,11 @@ class DirectPtraceConnection : public PtraceConnection { bool GetThreadInfo(pid_t tid, ThreadInfo* info) override; bool ReadFileContents(const base::FilePath& path, std::string* contents) override; + ProcessMemory* Memory() override; private: std::vector> attachments_; + ProcessMemoryLinux memory_; pid_t pid_; Ptracer ptracer_; InitializationStateDcheck initialized_; diff --git a/util/linux/exception_handler_client.cc b/util/linux/exception_handler_client.cc index cdfd0c5c..6333351c 100644 --- a/util/linux/exception_handler_client.cc +++ b/util/linux/exception_handler_client.cc @@ -130,7 +130,7 @@ int ExceptionHandlerClient::WaitForCrashDumpComplete() { constexpr bool am_64_bit = false; #endif // ARCH_CPU_64_BITS - PtraceBroker broker(server_sock_, am_64_bit); + PtraceBroker broker(server_sock_, getppid(), am_64_bit); _exit(broker.Run()); } diff --git a/util/linux/ptrace_broker.cc b/util/linux/ptrace_broker.cc index 67647c52..e9d30686 100644 --- a/util/linux/ptrace_broker.cc +++ b/util/linux/ptrace_broker.cc @@ -23,24 +23,65 @@ #include "base/logging.h" #include "base/posix/eintr_wrapper.h" -#include "util/file/file_io.h" namespace crashpad { -PtraceBroker::PtraceBroker(int sock, bool is_64_bit) +namespace { + +size_t FormatPID(char* buffer, pid_t pid) { + DCHECK_GE(pid, 0); + + char pid_buf[16]; + size_t length = 0; + do { + DCHECK_LT(length, sizeof(pid_buf)); + + pid_buf[length] = '0' + pid % 10; + pid /= 10; + ++length; + } while (pid > 0); + + for (size_t index = 0; index < length; ++index) { + buffer[index] = pid_buf[length - index - 1]; + } + + return length; +} + +} // namespace + +PtraceBroker::PtraceBroker(int sock, pid_t pid, bool is_64_bit) : ptracer_(is_64_bit, /* can_log= */ false), - file_root_("/proc/"), + file_root_(file_root_buffer_), attachments_(nullptr), attach_count_(0), attach_capacity_(0), - sock_(sock) { + memory_file_(), + sock_(sock), + memory_pid_(pid), + tried_opening_mem_file_(false) { AllocateAttachments(); + + static constexpr char kProc[] = "/proc/"; + size_t root_length = strlen(kProc); + memcpy(file_root_buffer_, kProc, root_length); + + if (pid >= 0) { + root_length += FormatPID(file_root_buffer_ + root_length, pid); + DCHECK_LT(root_length, sizeof(file_root_buffer_)); + file_root_buffer_[root_length] = '/'; + ++root_length; + } + + DCHECK_LT(root_length, sizeof(file_root_buffer_)); + file_root_buffer_[root_length] = '\0'; } PtraceBroker::~PtraceBroker() = default; void PtraceBroker::SetFileRoot(const char* new_root) { DCHECK_EQ(new_root[strlen(new_root) - 1], '/'); + memory_pid_ = -1; file_root_ = new_root; } @@ -189,12 +230,12 @@ int PtraceBroker::SendError(Errno err) { return WriteFile(sock_, &err, sizeof(err)) ? 0 : errno; } -int PtraceBroker::SendReadError(Errno err) { +int PtraceBroker::SendReadError(ReadError error) { int32_t rv = -1; - if (!WriteFile(sock_, &rv, sizeof(rv))) { - return errno; - } - return SendError(err); + return WriteFile(sock_, &rv, sizeof(rv)) && + WriteFile(sock_, &error, sizeof(error)) + ? 0 + : errno; } int PtraceBroker::SendOpenResult(OpenResult result) { @@ -208,7 +249,7 @@ int PtraceBroker::SendFileContents(FileHandle handle) { rv = ReadFile(handle, buffer, sizeof(buffer)); if (rv < 0) { - return SendReadError(errno); + return SendReadError(static_cast(errno)); } if (!WriteFile(sock_, &rv, sizeof(rv))) { @@ -225,25 +266,59 @@ int PtraceBroker::SendFileContents(FileHandle handle) { return 0; } +void PtraceBroker::TryOpeningMemFile() { + if (tried_opening_mem_file_) { + return; + } + tried_opening_mem_file_ = true; + + if (memory_pid_ < 0) { + return; + } + + char mem_path[32]; + size_t root_length = strlen(file_root_buffer_); + static constexpr char kMem[] = "mem"; + + DCHECK_LT(root_length + strlen(kMem) + 1, sizeof(mem_path)); + memcpy(mem_path, file_root_buffer_, root_length); + // Include the trailing NUL. + memcpy(mem_path + root_length, kMem, strlen(kMem) + 1); + memory_file_.reset( + HANDLE_EINTR(open(mem_path, O_RDONLY | O_CLOEXEC | O_NOCTTY))); +} + int PtraceBroker::SendMemory(pid_t pid, VMAddress address, VMSize size) { + if (memory_pid_ >= 0 && pid != memory_pid_) { + return SendReadError(kReadErrorAccessDenied); + } + + TryOpeningMemFile(); + auto read_memory = [this, pid](VMAddress address, size_t size, char* buffer) { + return this->memory_file_.is_valid() + ? HANDLE_EINTR( + pread64(this->memory_file_.get(), buffer, size, address)) + : this->ptracer_.ReadUpTo(pid, address, size, buffer); + }; + char buffer[4096]; while (size > 0) { - VMSize bytes_read = std::min(size, VMSize{sizeof(buffer)}); + size_t to_read = std::min(size, VMSize{sizeof(buffer)}); - if (!ptracer_.ReadMemory(pid, address, bytes_read, buffer)) { - bytes_read = 0; - Errno error = errno; - if (!WriteFile(sock_, &bytes_read, sizeof(bytes_read)) || - !WriteFile(sock_, &error, sizeof(error))) { - return errno; - } - return 0; + int32_t bytes_read = read_memory(address, to_read, buffer); + + if (bytes_read < 0) { + return SendReadError(static_cast(errno)); } if (!WriteFile(sock_, &bytes_read, sizeof(bytes_read))) { return errno; } + if (bytes_read == 0) { + return 0; + } + if (!WriteFile(sock_, buffer, bytes_read)) { return errno; } diff --git a/util/linux/ptrace_broker.h b/util/linux/ptrace_broker.h index a8d0e952..2e5decdf 100644 --- a/util/linux/ptrace_broker.h +++ b/util/linux/ptrace_broker.h @@ -62,12 +62,9 @@ class PtraceBroker { kTypeGetThreadInfo, //! \brief Reads memory from the attached process. The data is returned in - //! a series of messages. Each message begins with a VMSize indicating - //! the number of bytes being returned in this message, followed by - //! the requested bytes. The broker continues to send messages until - //! either all of the requested memory has been sent or an error - //! occurs, in which case it sends a message containing a VMSize equal - //! to zero, followed by an Errno. + //! a series of messages. Each message begins with an int32_t + //! indicating the number of bytes read, 0 for end-of-file, or -1 for + //! errors, followed by a ReadError. On success the bytes read follow. kTypeReadMemory, //! \brief Read a file's contents. The data is returned in a series of @@ -98,7 +95,7 @@ class PtraceBroker { VMSize size; } iov; - // \brief Specifies the file path to read for a kTypeReadFile request. + //! \brief Specifies the file path to read for a kTypeReadFile request. struct { //! \brief The number of bytes in #path. The path should not include a //! `NUL`-terminator. @@ -124,6 +121,14 @@ class PtraceBroker { kOpenResultSuccess = 0, }; + //! \brief A result used in operations that read from memory or files. + //! + //! Positive values of this enum are reserved for sending errno values. + enum ReadError : int32_t { + //! \brief Access to this data is denied. + kReadErrorAccessDenied = -1, + }; + //! \brief The response sent for a Request with type kTypeGetThreadInfo. struct GetThreadInfoResponse { //! \brief Information about the specified thread. Only valid if #success @@ -139,9 +144,15 @@ class PtraceBroker { //! //! \param[in] sock A socket on which to read requests from a connected //! PtraceClient. Does not take ownership of the socket. + //! \param[in] pid The process ID of the process the broker is expected to + //! trace. Setting this value exends the default file root to + //! "/proc/[pid]/" and enables memory reading via /proc/[pid]/mem. The + //! broker will deny any requests to read memory from processes whose + //! processID is not \a pid. If pid is -1, the broker will serve requests + //! to read memory from any process it is able to via `ptrace PEEKDATA`. //! \param[in] is_64_bit Whether this broker should be configured to trace a //! 64-bit process. - PtraceBroker(int sock, bool is_64_bit); + PtraceBroker(int sock, pid_t pid, bool is_64_bit); ~PtraceBroker(); @@ -149,7 +160,10 @@ class PtraceBroker { //! root. //! //! If this method is not called, the broker defaults to only serving files - //! under "/proc/". + //! under "/proc/" or "/proc/[pid]/" if a pid was set. + //! + //! Calling this function disables reading from a memory file if one has not + //! already been opened. //! //! \param[in] root A NUL-terminated c-string containing the path to the new //! root. \a root must not be `nullptr`, must end in a '/', and the caller @@ -173,18 +187,23 @@ class PtraceBroker { void ReleaseAttachments(); int RunImpl(); int SendError(Errno err); - int SendReadError(Errno err); + int SendReadError(ReadError err); int SendOpenResult(OpenResult result); int SendFileContents(FileHandle handle); + void TryOpeningMemFile(); int SendMemory(pid_t pid, VMAddress address, VMSize size); int ReceiveAndOpenFilePath(VMSize path_length, ScopedFileHandle* handle); + char file_root_buffer_[32]; Ptracer ptracer_; const char* file_root_; ScopedPtraceAttach* attachments_; size_t attach_count_; size_t attach_capacity_; + ScopedFileHandle memory_file_; int sock_; + pid_t memory_pid_; + bool tried_opening_mem_file_; DISALLOW_COPY_AND_ASSIGN(PtraceBroker); }; diff --git a/util/linux/ptrace_broker_test.cc b/util/linux/ptrace_broker_test.cc index 65fef9d9..9ed1973b 100644 --- a/util/linux/ptrace_broker_test.cc +++ b/util/linux/ptrace_broker_test.cc @@ -127,6 +127,85 @@ class SameBitnessTest : public Multiprocess { } private: + void BrokerTests(bool set_broker_pid, + LinuxVMAddress child1_tls, + LinuxVMAddress child2_tls, + pid_t child2_tid, + const base::FilePath& file_dir, + const base::FilePath& test_file, + const std::string& expected_file_contents) { + int socks[2]; + ASSERT_EQ(socketpair(AF_UNIX, SOCK_STREAM, 0, socks), 0); + ScopedFileHandle broker_sock(socks[0]); + ScopedFileHandle client_sock(socks[1]); + +#if defined(ARCH_CPU_64_BITS) + constexpr bool am_64_bit = true; +#else + constexpr bool am_64_bit = false; +#endif // ARCH_CPU_64_BITS + + PtraceBroker broker( + broker_sock.get(), set_broker_pid ? ChildPID() : -1, am_64_bit); + RunBrokerThread broker_thread(&broker); + broker_thread.Start(); + + PtraceClient client; + ASSERT_TRUE(client.Initialize( + client_sock.get(), ChildPID(), /* try_direct_memory= */ false)); + + EXPECT_EQ(client.GetProcessID(), ChildPID()); + EXPECT_TRUE(client.Attach(child2_tid)); + EXPECT_EQ(client.Is64Bit(), am_64_bit); + + ThreadInfo info1; + ASSERT_TRUE(client.GetThreadInfo(ChildPID(), &info1)); + EXPECT_EQ(info1.thread_specific_data_address, child1_tls); + + ThreadInfo info2; + ASSERT_TRUE(client.GetThreadInfo(child2_tid, &info2)); + EXPECT_EQ(info2.thread_specific_data_address, child2_tls); + + ProcessMemory* memory = client.Memory(); + ASSERT_TRUE(memory); + + auto buffer = std::make_unique(mapping_.len()); + ASSERT_TRUE(memory->Read( + mapping_.addr_as(), mapping_.len(), buffer.get())); + auto expected_buffer = mapping_.addr_as(); + for (size_t index = 0; index < mapping_.len(); ++index) { + EXPECT_EQ(buffer[index], expected_buffer[index]); + } + + char first; + ASSERT_TRUE( + memory->Read(mapping_.addr_as(), sizeof(first), &first)); + EXPECT_EQ(first, expected_buffer[0]); + + char last; + ASSERT_TRUE(memory->Read(mapping_.addr_as() + mapping_.len() - 1, + sizeof(last), + &last)); + EXPECT_EQ(last, expected_buffer[mapping_.len() - 1]); + + char unmapped; + EXPECT_FALSE(memory->Read(mapping_.addr_as() + mapping_.len(), + sizeof(unmapped), + &unmapped)); + + std::string file_root = file_dir.value() + '/'; + broker.SetFileRoot(file_root.c_str()); + + std::string file_contents; + ASSERT_TRUE(client.ReadFileContents(test_file, &file_contents)); + EXPECT_EQ(file_contents, expected_file_contents); + + ScopedTempDir temp_dir2; + base::FilePath test_file2(temp_dir2.path().Append("test_file2")); + ASSERT_TRUE(CreateFile(test_file2)); + EXPECT_FALSE(client.ReadFileContents(test_file2, &file_contents)); + } + void MultiprocessParent() override { LinuxVMAddress child1_tls; ASSERT_TRUE(LoggingReadFileExactly( @@ -140,11 +219,6 @@ class SameBitnessTest : public Multiprocess { ASSERT_TRUE(LoggingReadFileExactly( ReadPipeHandle(), &child2_tls, sizeof(child2_tls))); - int socks[2]; - ASSERT_EQ(socketpair(AF_UNIX, SOCK_STREAM, 0, socks), 0); - ScopedFileHandle broker_sock(socks[0]); - ScopedFileHandle client_sock(socks[1]); - ScopedTempDir temp_dir; base::FilePath file_path(temp_dir.path().Append("test_file")); std::string expected_file_contents; @@ -162,67 +236,20 @@ class SameBitnessTest : public Multiprocess { expected_file_contents.size())); } -#if defined(ARCH_CPU_64_BITS) - constexpr bool am_64_bit = true; -#else - constexpr bool am_64_bit = false; -#endif // ARCH_CPU_64_BITS - PtraceBroker broker(broker_sock.get(), am_64_bit); - RunBrokerThread broker_thread(&broker); - broker_thread.Start(); - - { - PtraceClient client; - ASSERT_TRUE(client.Initialize(client_sock.get(), ChildPID())); - - EXPECT_EQ(client.GetProcessID(), ChildPID()); - EXPECT_TRUE(client.Attach(child2_tid)); - EXPECT_EQ(client.Is64Bit(), am_64_bit); - - ThreadInfo info1; - ASSERT_TRUE(client.GetThreadInfo(ChildPID(), &info1)); - EXPECT_EQ(info1.thread_specific_data_address, child1_tls); - - ThreadInfo info2; - ASSERT_TRUE(client.GetThreadInfo(child2_tid, &info2)); - EXPECT_EQ(info2.thread_specific_data_address, child2_tls); - - auto buffer = std::make_unique(mapping_.len()); - ASSERT_TRUE(client.Read( - mapping_.addr_as(), mapping_.len(), buffer.get())); - auto expected_buffer = mapping_.addr_as(); - for (size_t index = 0; index < mapping_.len(); ++index) { - EXPECT_EQ(buffer[index], expected_buffer[index]); - } - - char first; - ASSERT_TRUE( - client.Read(mapping_.addr_as(), sizeof(first), &first)); - EXPECT_EQ(first, expected_buffer[0]); - - char last; - ASSERT_TRUE( - client.Read(mapping_.addr_as() + mapping_.len() - 1, - sizeof(last), - &last)); - EXPECT_EQ(last, expected_buffer[mapping_.len() - 1]); - - char unmapped; - EXPECT_FALSE(client.Read(mapping_.addr_as() + mapping_.len(), - sizeof(unmapped), - &unmapped)); - - std::string file_root = temp_dir.path().value() + '/'; - broker.SetFileRoot(file_root.c_str()); - std::string file_contents; - ASSERT_TRUE(client.ReadFileContents(file_path, &file_contents)); - EXPECT_EQ(file_contents, expected_file_contents); - - ScopedTempDir temp_dir2; - base::FilePath test_file2(temp_dir2.path().Append("test_file2")); - ASSERT_TRUE(CreateFile(test_file2)); - EXPECT_FALSE(client.ReadFileContents(test_file2, &file_contents)); - } + BrokerTests(true, + child1_tls, + child2_tls, + child2_tid, + temp_dir.path(), + file_path, + expected_file_contents); + BrokerTests(false, + child1_tls, + child2_tls, + child2_tid, + temp_dir.path(), + file_path, + expected_file_contents); } void MultiprocessChild() override { diff --git a/util/linux/ptrace_client.cc b/util/linux/ptrace_client.cc index ef880b2e..b5ccebaf 100644 --- a/util/linux/ptrace_client.cc +++ b/util/linux/ptrace_client.cc @@ -15,12 +15,14 @@ #include "util/linux/ptrace_client.h" #include +#include #include #include "base/logging.h" #include "util/file/file_io.h" #include "util/linux/ptrace_broker.h" +#include "util/process/process_memory_linux.h" namespace crashpad { @@ -36,6 +38,27 @@ bool ReceiveAndLogError(int sock, const std::string& operation) { return true; } +bool ReceiveAndLogReadError(int sock, const std::string& operation) { + PtraceBroker::ReadError err; + if (!LoggingReadFileExactly(sock, &err, sizeof(err))) { + return false; + } + switch (err) { + case PtraceBroker::kReadErrorAccessDenied: + LOG(ERROR) << operation << " access denied"; + return true; + default: + if (err <= 0) { + LOG(ERROR) << operation << " invalid error " << err; + DCHECK(false); + return false; + } + errno = err; + PLOG(ERROR) << operation; + return true; + } +} + bool AttachImpl(int sock, pid_t tid) { PtraceBroker::Request request; request.type = PtraceBroker::Request::kTypeAttach; @@ -61,6 +84,7 @@ bool AttachImpl(int sock, pid_t tid) { PtraceClient::PtraceClient() : PtraceConnection(), + memory_(), sock_(kInvalidFileHandle), pid_(-1), is_64_bit_(false), @@ -74,7 +98,7 @@ PtraceClient::~PtraceClient() { } } -bool PtraceClient::Initialize(int sock, pid_t pid) { +bool PtraceClient::Initialize(int sock, pid_t pid, bool try_direct_memory) { INITIALIZATION_STATE_SET_INITIALIZING(initialized_); sock_ = sock; pid_ = pid; @@ -97,46 +121,20 @@ bool PtraceClient::Initialize(int sock, pid_t pid) { } is_64_bit_ = is_64_bit == kBoolTrue; + if (try_direct_memory) { + auto direct_mem = std::make_unique(); + if (direct_mem->Initialize(pid)) { + memory_.reset(direct_mem.release()); + } + } + if (!memory_) { + memory_ = std::make_unique(this); + } + INITIALIZATION_STATE_SET_VALID(initialized_); return true; } -bool PtraceClient::Read(VMAddress address, size_t size, void* buffer) { - INITIALIZATION_STATE_DCHECK_VALID(initialized_); - char* buffer_c = reinterpret_cast(buffer); - - PtraceBroker::Request request; - request.type = PtraceBroker::Request::kTypeReadMemory; - request.tid = pid_; - request.iov.base = address; - request.iov.size = size; - - if (!LoggingWriteFile(sock_, &request, sizeof(request))) { - return false; - } - - while (size > 0) { - VMSize bytes_read; - if (!LoggingReadFileExactly(sock_, &bytes_read, sizeof(bytes_read))) { - return false; - } - - if (!bytes_read) { - ReceiveAndLogError(sock_, "PtraceBroker ReadMemory"); - return false; - } - - if (!LoggingReadFileExactly(sock_, buffer_c, bytes_read)) { - return false; - } - - size -= bytes_read; - buffer_c += bytes_read; - } - - return true; -} - pid_t PtraceClient::GetProcessID() { INITIALIZATION_STATE_DCHECK_VALID(initialized_); return pid_; @@ -197,7 +195,7 @@ bool PtraceClient::ReadFileContents(const base::FilePath& path, } if (read_result < 0) { - ReceiveAndLogError(sock_, "ReadFileContents"); + ReceiveAndLogReadError(sock_, "ReadFileContents"); return false; } @@ -215,6 +213,66 @@ bool PtraceClient::ReadFileContents(const base::FilePath& path, return true; } +ProcessMemory* PtraceClient::Memory() { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + return memory_.get(); +} + +PtraceClient::BrokeredMemory::BrokeredMemory(PtraceClient* client) + : ProcessMemory(), client_(client) {} + +PtraceClient::BrokeredMemory::~BrokeredMemory() = default; + +ssize_t PtraceClient::BrokeredMemory::ReadUpTo(VMAddress address, + size_t size, + void* buffer) const { + return client_->ReadUpTo(address, size, buffer); +} + +ssize_t PtraceClient::ReadUpTo(VMAddress address, + size_t size, + void* buffer) const { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + char* buffer_c = reinterpret_cast(buffer); + + PtraceBroker::Request request; + request.type = PtraceBroker::Request::kTypeReadMemory; + request.tid = pid_; + request.iov.base = address; + request.iov.size = size; + + if (!LoggingWriteFile(sock_, &request, sizeof(request))) { + return false; + } + + ssize_t total_read = 0; + while (size > 0) { + int32_t bytes_read; + if (!LoggingReadFileExactly(sock_, &bytes_read, sizeof(bytes_read))) { + return -1; + } + + if (bytes_read < 0) { + ReceiveAndLogReadError(sock_, "PtraceBroker ReadMemory"); + return -1; + } + + if (bytes_read == 0) { + return total_read; + } + + if (!LoggingReadFileExactly(sock_, buffer_c, bytes_read)) { + return -1; + } + + size -= bytes_read; + buffer_c += bytes_read; + total_read += bytes_read; + } + + return total_read; +} + bool PtraceClient::SendFilePath(const char* path, size_t length) { if (!LoggingWriteFile(sock_, path, length)) { return false; diff --git a/util/linux/ptrace_client.h b/util/linux/ptrace_client.h index 8260412d..a7abc0cf 100644 --- a/util/linux/ptrace_client.h +++ b/util/linux/ptrace_client.h @@ -17,10 +17,13 @@ #include +#include + #include "base/macros.h" #include "util/linux/ptrace_connection.h" #include "util/misc/address_types.h" #include "util/misc/initialization_state_dcheck.h" +#include "util/process/process_memory.h" namespace crashpad { @@ -43,26 +46,11 @@ class PtraceClient : public PtraceConnection { //! ownership of the socket. //! \param[in] pid The process ID of the process to form a PtraceConnection //! with. + //! \param[in] try_direct_memory If `true` the client will attempt to support + //! memory reading operations by directly acessing the target process' + //! /proc/[pid]/mem file. //! \return `true` on success. `false` on failure with a message logged. - bool Initialize(int sock, pid_t pid); - - //! \brief Copies memory from the target process into a caller-provided buffer - //! in the current process. - //! - //! TODO(jperaza): In order for this to be usable, PtraceConnection will need - //! to surface it, possibly by inheriting from ProcessMemory, or providing a - //! method to return a ProcessMemory*. - //! - //! \param[in] address The address, in the target process' address space, of - //! the memory region to copy. - //! \param[in] size The size, in bytes, of the memory region to copy. - //! \a buffer must be at least this size. - //! \param[out] buffer The buffer into which the contents of the other - //! process' memory will be copied. - //! - //! \return `true` on success, with \a buffer filled appropriately. `false` on - //! failure, with a message logged. - bool Read(VMAddress address, size_t size, void* buffer); + bool Initialize(int sock, pid_t pid, bool try_direct_memory = true); // PtraceConnection: @@ -72,10 +60,28 @@ class PtraceClient : public PtraceConnection { bool GetThreadInfo(pid_t tid, ThreadInfo* info) override; bool ReadFileContents(const base::FilePath& path, std::string* contents) override; + ProcessMemory* Memory() override; private: + class BrokeredMemory : public ProcessMemory { + public: + explicit BrokeredMemory(PtraceClient* client); + ~BrokeredMemory(); + + ssize_t ReadUpTo(VMAddress address, + size_t size, + void* buffer) const override; + + private: + PtraceClient* client_; + + DISALLOW_COPY_AND_ASSIGN(BrokeredMemory); + }; + + ssize_t ReadUpTo(VMAddress address, size_t size, void* buffer) const; bool SendFilePath(const char* path, size_t length); + std::unique_ptr memory_; int sock_; pid_t pid_; bool is_64_bit_; diff --git a/util/linux/ptrace_connection.h b/util/linux/ptrace_connection.h index e3111112..86dc2dd9 100644 --- a/util/linux/ptrace_connection.h +++ b/util/linux/ptrace_connection.h @@ -21,6 +21,7 @@ #include "base/files/file_path.h" #include "util/linux/thread_info.h" +#include "util/process/process_memory.h" namespace crashpad { @@ -57,6 +58,12 @@ class PtraceConnection { //! \return `true` on success. `false` on failure with a message logged. virtual bool ReadFileContents(const base::FilePath& path, std::string* contents) = 0; + + //! \brief Returns a memory reader for the connected process. + //! + //! 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; }; } // namespace crashpad diff --git a/util/linux/ptracer.cc b/util/linux/ptracer.cc index 84447362..63bee1e0 100644 --- a/util/linux/ptracer.cc +++ b/util/linux/ptracer.cc @@ -375,10 +375,11 @@ bool Ptracer::GetThreadInfo(pid_t tid, ThreadInfo* info) { can_log_); } -bool Ptracer::ReadMemory(pid_t pid, - LinuxVMAddress address, - size_t size, - char* buffer) { +ssize_t Ptracer::ReadUpTo(pid_t pid, + LinuxVMAddress address, + size_t size, + char* buffer) { + size_t bytes_read = 0; while (size > 0) { errno = 0; @@ -386,46 +387,66 @@ bool Ptracer::ReadMemory(pid_t pid, *reinterpret_cast(buffer) = ptrace(PTRACE_PEEKDATA, pid, address, nullptr); + if (errno == EIO) { + ssize_t last_bytes = ReadLastBytes(pid, address, size, buffer); + return last_bytes >= 0 ? bytes_read + last_bytes : -1; + } + if (errno != 0) { PLOG_IF(ERROR, can_log_) << "ptrace"; - return false; + return -1; } size -= sizeof(long); buffer += sizeof(long); address += sizeof(long); + bytes_read += sizeof(long); } else { long word = ptrace(PTRACE_PEEKDATA, pid, address, nullptr); if (errno == 0) { memcpy(buffer, reinterpret_cast(&word), size); - return true; + return bytes_read + size; } - if (errno != EIO) { - PLOG_IF(ERROR, can_log_); - return false; - } - - // A read smaller than a word at the end of a mapping might spill over - // into unmapped memory. Try aligning the read so that the requested - // data is at the end of the word instead. - errno = 0; - word = - ptrace(PTRACE_PEEKDATA, pid, address - sizeof(word) + size, nullptr); - - if (errno == 0) { - memcpy( - buffer, reinterpret_cast(&word) + sizeof(word) - size, size); - return true; + if (errno == EIO) { + ssize_t last_bytes = ReadLastBytes(pid, address, size, buffer); + return last_bytes >= 0 ? bytes_read + last_bytes : -1; } PLOG_IF(ERROR, can_log_); - return false; + return -1; } } - return true; + return bytes_read; +} + +// Handles an EIO by reading at most size bytes from address into buffer if +// address was within a word of a possible page boundary, by aligning to read +// the last word of the page and extracting the desired bytes. +ssize_t Ptracer::ReadLastBytes(pid_t pid, + LinuxVMAddress address, + size_t size, + char* buffer) { + LinuxVMAddress aligned = ((address + 4095) & ~4095) - sizeof(long); + if (aligned >= address || aligned == address - sizeof(long)) { + PLOG_IF(ERROR, can_log_) << "ptrace"; + return -1; + } + DCHECK_GT(aligned, address - sizeof(long)); + + errno = 0; + long word = ptrace(PTRACE_PEEKDATA, pid, aligned, nullptr); + if (errno != 0) { + PLOG_IF(ERROR, can_log_) << "ptrace"; + return -1; + } + + size_t bytes_read = address - aligned; + size_t last_bytes = std::min(sizeof(long) - bytes_read, size); + memcpy(buffer, reinterpret_cast(&word) + bytes_read, last_bytes); + return last_bytes; } } // namespace crashpad diff --git a/util/linux/ptracer.h b/util/linux/ptracer.h index daa7a01f..33eeb3ae 100644 --- a/util/linux/ptracer.h +++ b/util/linux/ptracer.h @@ -72,20 +72,29 @@ class Ptracer { bool GetThreadInfo(pid_t tid, ThreadInfo* info); //! \brief Uses `ptrace` to read memory from the process with process ID \a - //! pid. + //! pid, up to a maximum number of bytes. //! //! The target process should already be attached before calling this method. //! \see ScopedPtraceAttach //! //! \param[in] pid The process ID whose memory to read. //! \param[in] address The base address of the region to read. - //! \param[in] size The size of the memory region to read. + //! \param[in] size The size of the memory region to read. \a buffer must be + //! at least this size. //! \param[out] buffer The buffer to fill with the data read. - //! \return `true` on success. `false` on failure with a message logged, if - //! enabled. - bool ReadMemory(pid_t pid, LinuxVMAddress address, size_t size, char* buffer); + //! \return the number of bytes read, 0 if there are no more bytes to read, or + //! -1 on failure with a message logged if logging is enabled. + ssize_t ReadUpTo(pid_t pid, + LinuxVMAddress address, + size_t size, + char* buffer); private: + ssize_t ReadLastBytes(pid_t pid, + LinuxVMAddress address, + size_t size, + char* buffer); + bool is_64_bit_; bool can_log_; InitializationStateDcheck initialized_; diff --git a/util/process/process_memory.h b/util/process/process_memory.h index 34561718..d67c3828 100644 --- a/util/process/process_memory.h +++ b/util/process/process_memory.h @@ -78,9 +78,10 @@ class ProcessMemory { return ReadCStringInternal(address, true, size, string); } + virtual ~ProcessMemory() = default; + protected: ProcessMemory() = default; - ~ProcessMemory() = default; private: //! \brief Copies memory from the target process into a caller-provided buffer