From 0a8985cd20d3d287a4a425c091448e926896513a Mon Sep 17 00:00:00 2001 From: Joshua Peraza Date: Tue, 12 Oct 2021 15:55:03 -0700 Subject: [PATCH] linux,arm: support memory tagging 64-bit ARM's Top-Byte-Ignore enables features such as memory tagging. https://www.kernel.org/doc/html/latest/arm64/tagged-address-abi.html Android 11 will start using memory tagging on some devices. https://source.android.com/devices/tech/debug/tagged-pointers Crashpad needs to remove the tags from pointers before comparing to addresses or using with system calls. Bug: crashpad:364 Change-Id: I67c6b9a4a86d090e1d139de727eb06d9e222cc25 Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3078500 Reviewed-by: Mark Mentovai Commit-Queue: Joshua Peraza --- snapshot/linux/process_reader_linux.cc | 13 +++++++------ snapshot/linux/process_reader_linux.h | 2 +- snapshot/linux/process_reader_linux_test.cc | 9 ++++++--- test/linux/fake_ptrace_connection.cc | 2 +- test/linux/fake_ptrace_connection.h | 2 +- util/linux/direct_ptrace_connection.cc | 2 +- util/linux/direct_ptrace_connection.h | 4 ++-- util/linux/memory_map.cc | 8 +++++--- util/linux/memory_map.h | 1 + util/linux/ptrace_client.cc | 2 +- util/linux/ptrace_client.h | 4 ++-- util/linux/ptrace_connection.h | 4 ++-- util/process/process_memory_linux.cc | 17 +++++++++++++++-- util/process/process_memory_linux.h | 8 +++++++- 14 files changed, 52 insertions(+), 26 deletions(-) diff --git a/snapshot/linux/process_reader_linux.cc b/snapshot/linux/process_reader_linux.cc index ee246e8b..a9098ac8 100644 --- a/snapshot/linux/process_reader_linux.cc +++ b/snapshot/linux/process_reader_linux.cc @@ -125,7 +125,8 @@ void ProcessReaderLinux::Thread::InitializeStackFromSP( LOG(WARNING) << "no stack mapping"; return; } - LinuxVMAddress stack_region_start = stack_pointer; + LinuxVMAddress stack_region_start = + reader->Memory()->PointerToAddress(stack_pointer); // We've hit what looks like a guard page; skip to the end and check for a // mapped stack region. @@ -177,11 +178,11 @@ void ProcessReaderLinux::Thread::InitializeStackFromSP( // at the high-address end of the stack so we can try using that to shrink // the stack region. stack_region_size = stack_end - stack_region_address; - if (tid != reader->ProcessID() && - thread_info.thread_specific_data_address > stack_region_address && - thread_info.thread_specific_data_address < stack_end) { - stack_region_size = - thread_info.thread_specific_data_address - stack_region_address; + VMAddress tls_address = reader->Memory()->PointerToAddress( + thread_info.thread_specific_data_address); + if (tid != reader->ProcessID() && tls_address > stack_region_address && + tls_address < stack_end) { + stack_region_size = tls_address - stack_region_address; } } diff --git a/snapshot/linux/process_reader_linux.h b/snapshot/linux/process_reader_linux.h index 0eb1d404..f44e15f5 100644 --- a/snapshot/linux/process_reader_linux.h +++ b/snapshot/linux/process_reader_linux.h @@ -123,7 +123,7 @@ class ProcessReaderLinux { pid_t ParentProcessID() const { return process_info_.ParentProcessID(); } //! \brief Return a memory reader for the target process. - const ProcessMemory* Memory() const { return connection_->Memory(); } + const ProcessMemoryLinux* Memory() const { return connection_->Memory(); } //! \brief Return a memory map of the target process. MemoryMap* GetMemoryMap() { return &memory_map_; } diff --git a/snapshot/linux/process_reader_linux_test.cc b/snapshot/linux/process_reader_linux_test.cc index 250f12da..f3791f85 100644 --- a/snapshot/linux/process_reader_linux_test.cc +++ b/snapshot/linux/process_reader_linux_test.cc @@ -291,9 +291,12 @@ void ExpectThreads(const ThreadMap& thread_map, #if !defined(ADDRESS_SANITIZER) // AddressSanitizer causes stack variables to be stored separately from the // call stack. - EXPECT_LE(thread.stack_region_address, iterator->second.stack_address); - EXPECT_GE(thread.stack_region_address + thread.stack_region_size, - iterator->second.stack_address); + EXPECT_LE( + thread.stack_region_address, + connection->Memory()->PointerToAddress(iterator->second.stack_address)); + EXPECT_GE( + thread.stack_region_address + thread.stack_region_size, + connection->Memory()->PointerToAddress(iterator->second.stack_address)); #endif // !defined(ADDRESS_SANITIZER) if (iterator->second.max_stack_size) { diff --git a/test/linux/fake_ptrace_connection.cc b/test/linux/fake_ptrace_connection.cc index 774a340a..33a8f08b 100644 --- a/test/linux/fake_ptrace_connection.cc +++ b/test/linux/fake_ptrace_connection.cc @@ -80,7 +80,7 @@ bool FakePtraceConnection::ReadFileContents(const base::FilePath& path, return LoggingReadEntireFile(path, contents); } -ProcessMemory* FakePtraceConnection::Memory() { +ProcessMemoryLinux* FakePtraceConnection::Memory() { INITIALIZATION_STATE_DCHECK_VALID(initialized_); if (!memory_) { memory_ = std::make_unique(this); diff --git a/test/linux/fake_ptrace_connection.h b/test/linux/fake_ptrace_connection.h index bf09d95d..f6ba7de3 100644 --- a/test/linux/fake_ptrace_connection.h +++ b/test/linux/fake_ptrace_connection.h @@ -63,7 +63,7 @@ class FakePtraceConnection : public PtraceConnection { //! \brief Attempts to create a ProcessMemory when called, calling //! ADD_FAILURE() and returning `nullptr` on failure. - ProcessMemory* Memory() override; + ProcessMemoryLinux* Memory() override; //! \todo Not yet implemented. bool Threads(std::vector* threads) override; diff --git a/util/linux/direct_ptrace_connection.cc b/util/linux/direct_ptrace_connection.cc index 5da01475..3cc41c3f 100644 --- a/util/linux/direct_ptrace_connection.cc +++ b/util/linux/direct_ptrace_connection.cc @@ -73,7 +73,7 @@ bool DirectPtraceConnection::ReadFileContents(const base::FilePath& path, return LoggingReadEntireFile(path, contents); } -ProcessMemory* DirectPtraceConnection::Memory() { +ProcessMemoryLinux* DirectPtraceConnection::Memory() { INITIALIZATION_STATE_DCHECK_VALID(initialized_); if (!memory_) { memory_ = std::make_unique(this); diff --git a/util/linux/direct_ptrace_connection.h b/util/linux/direct_ptrace_connection.h index a120889a..391495ff 100644 --- a/util/linux/direct_ptrace_connection.h +++ b/util/linux/direct_ptrace_connection.h @@ -58,13 +58,13 @@ 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; + ProcessMemoryLinux* Memory() override; bool Threads(std::vector* threads) override; ssize_t ReadUpTo(VMAddress, size_t size, void* buffer) override; private: std::vector> attachments_; - std::unique_ptr memory_; + std::unique_ptr memory_; pid_t pid_; Ptracer ptracer_; InitializationStateDcheck initialized_; diff --git a/util/linux/memory_map.cc b/util/linux/memory_map.cc index dabc114d..e15a1c5e 100644 --- a/util/linux/memory_map.cc +++ b/util/linux/memory_map.cc @@ -240,7 +240,7 @@ MemoryMap::Mapping::Mapping() executable(false), shareable(false) {} -MemoryMap::MemoryMap() : mappings_(), initialized_() {} +MemoryMap::MemoryMap() : mappings_(), connection_(nullptr), initialized_() {} MemoryMap::~MemoryMap() {} @@ -256,6 +256,7 @@ bool MemoryMap::Mapping::Equals(const Mapping& other) const { bool MemoryMap::Initialize(PtraceConnection* connection) { INITIALIZATION_STATE_SET_INITIALIZING(initialized_); + connection_ = connection; // If the maps file is not read atomically, entries can be read multiple times // or missed entirely. The kernel reads entries from this file into a page @@ -268,8 +269,8 @@ bool MemoryMap::Initialize(PtraceConnection* connection) { do { std::string contents; char path[32]; - snprintf(path, sizeof(path), "/proc/%d/maps", connection->GetProcessID()); - if (!connection->ReadFileContents(base::FilePath(path), &contents)) { + snprintf(path, sizeof(path), "/proc/%d/maps", connection_->GetProcessID()); + if (!connection_->ReadFileContents(base::FilePath(path), &contents)) { return false; } @@ -298,6 +299,7 @@ bool MemoryMap::Initialize(PtraceConnection* connection) { const MemoryMap::Mapping* MemoryMap::FindMapping(LinuxVMAddress address) const { INITIALIZATION_STATE_DCHECK_VALID(initialized_); + address = connection_->Memory()->PointerToAddress(address); for (const auto& mapping : mappings_) { if (mapping.range.Base() <= address && mapping.range.End() > address) { diff --git a/util/linux/memory_map.h b/util/linux/memory_map.h index d43b7af4..f6f7cc84 100644 --- a/util/linux/memory_map.h +++ b/util/linux/memory_map.h @@ -129,6 +129,7 @@ class MemoryMap { private: std::vector mappings_; + PtraceConnection* connection_; InitializationStateDcheck initialized_; }; diff --git a/util/linux/ptrace_client.cc b/util/linux/ptrace_client.cc index cb1a1a5d..9a34722a 100644 --- a/util/linux/ptrace_client.cc +++ b/util/linux/ptrace_client.cc @@ -248,7 +248,7 @@ bool PtraceClient::ReadFileContents(const base::FilePath& path, return true; } -ProcessMemory* PtraceClient::Memory() { +ProcessMemoryLinux* PtraceClient::Memory() { INITIALIZATION_STATE_DCHECK_VALID(initialized_); if (!memory_) { memory_ = std::make_unique(this); diff --git a/util/linux/ptrace_client.h b/util/linux/ptrace_client.h index 299aafc9..cd40f6bd 100644 --- a/util/linux/ptrace_client.h +++ b/util/linux/ptrace_client.h @@ -60,14 +60,14 @@ 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; + ProcessMemoryLinux* Memory() override; bool Threads(std::vector* threads) override; ssize_t ReadUpTo(VMAddress address, size_t size, void* buffer) override; private: bool SendFilePath(const char* path, size_t length); - std::unique_ptr memory_; + 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 9b31d52a..15594f81 100644 --- a/util/linux/ptrace_connection.h +++ b/util/linux/ptrace_connection.h @@ -22,7 +22,7 @@ #include "base/files/file_path.h" #include "util/linux/thread_info.h" -#include "util/process/process_memory.h" +#include "util/process/process_memory_linux.h" namespace crashpad { @@ -64,7 +64,7 @@ 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; + virtual ProcessMemoryLinux* Memory() = 0; //! \brief Determines the thread IDs of the threads in the connected process. //! diff --git a/util/process/process_memory_linux.cc b/util/process/process_memory_linux.cc index 56ac0ec8..b67b9c09 100644 --- a/util/process/process_memory_linux.cc +++ b/util/process/process_memory_linux.cc @@ -24,11 +24,20 @@ #include "base/logging.h" #include "base/posix/eintr_wrapper.h" +#include "build/build_config.h" +#include "util/file/filesystem.h" +#include "util/linux/ptrace_connection.h" namespace crashpad { ProcessMemoryLinux::ProcessMemoryLinux(PtraceConnection* connection) - : ProcessMemory(), mem_fd_() { + : ProcessMemory(), mem_fd_(), ignore_top_byte_(false) { +#if defined(ARCH_CPU_ARM_FAMILY) + if (connection->Is64Bit()) { + ignore_top_byte_ = true; + } +#endif // ARCH_CPU_ARM_FAMILY + char path[32]; snprintf(path, sizeof(path), "/proc/%d/mem", connection->GetProcessID()); mem_fd_.reset(HANDLE_EINTR(open(path, O_RDONLY | O_NOCTTY | O_CLOEXEC))); @@ -53,11 +62,15 @@ ProcessMemoryLinux::ProcessMemoryLinux(PtraceConnection* connection) ProcessMemoryLinux::~ProcessMemoryLinux() {} +VMAddress ProcessMemoryLinux::PointerToAddress(VMAddress address) const { + return ignore_top_byte_ ? address & 0x00ffffffffffffff : address; +} + ssize_t ProcessMemoryLinux::ReadUpTo(VMAddress address, size_t size, void* buffer) const { DCHECK_LE(size, size_t{std::numeric_limits::max()}); - return read_up_to_(address, size, buffer); + return read_up_to_(PointerToAddress(address), size, buffer); } } // namespace crashpad diff --git a/util/process/process_memory_linux.h b/util/process/process_memory_linux.h index 496cf2d5..f62f2b4f 100644 --- a/util/process/process_memory_linux.h +++ b/util/process/process_memory_linux.h @@ -21,12 +21,13 @@ #include #include "base/files/scoped_file.h" -#include "util/linux/ptrace_connection.h" #include "util/misc/address_types.h" #include "util/process/process_memory.h" namespace crashpad { +class PtraceConnection; + //! \brief Accesses the memory of another Linux process. class ProcessMemoryLinux final : public ProcessMemory { public: @@ -37,11 +38,16 @@ class ProcessMemoryLinux final : public ProcessMemory { ~ProcessMemoryLinux(); + //! \brief Returns the input pointer with any non-addressing bits, such as + //! tags removed. + VMAddress PointerToAddress(VMAddress address) const; + private: ssize_t ReadUpTo(VMAddress address, size_t size, void* buffer) const override; std::function read_up_to_; base::ScopedFD mem_fd_; + bool ignore_top_byte_; }; } // namespace crashpad