Merge "Read maps data before using and do safe reads." into main
diff --git a/libc/malloc_debug/MapData.cpp b/libc/malloc_debug/MapData.cpp
index b22c109..c58882a 100644
--- a/libc/malloc_debug/MapData.cpp
+++ b/libc/malloc_debug/MapData.cpp
@@ -34,6 +34,8 @@
#include <stdlib.h>
#include <string.h>
#include <sys/mman.h>
+#include <sys/uio.h>
+#include <unistd.h>
#include <vector>
@@ -69,148 +71,132 @@
MapEntry* entry = new MapEntry(start, end, offset, name, name_len, flags);
if (!(flags & PROT_READ)) {
- // Any unreadable map will just get a zero load bias.
- entry->load_bias = 0;
- entry->init = true;
- entry->valid = false;
+ // This will make sure that an unreadable map will prevent attempts to read
+ // elf data from the map.
+ entry->SetInvalid();
}
return entry;
}
-template <typename T>
-static inline bool get_val(MapEntry* entry, uintptr_t addr, T* store) {
- if (!(entry->flags & PROT_READ) || addr < entry->start || addr + sizeof(T) > entry->end) {
- return false;
+void MapEntry::Init() {
+ if (init_) {
+ return;
}
- // Make sure the address is aligned properly.
- if (addr & (sizeof(T) - 1)) {
- return false;
- }
- *store = *reinterpret_cast<T*>(addr);
- return true;
-}
+ init_ = true;
-static bool valid_elf(MapEntry* entry) {
- uintptr_t addr = entry->start;
- uintptr_t end;
- if (__builtin_add_overflow(addr, SELFMAG, &end) || end >= entry->end) {
- return false;
+ uintptr_t end_addr;
+ if (__builtin_add_overflow(start_, SELFMAG, &end_addr) || end_addr >= end_) {
+ return;
}
- return memcmp(reinterpret_cast<void*>(addr), ELFMAG, SELFMAG) == 0;
-}
-
-static void read_loadbias(MapEntry* entry) {
- entry->load_bias = 0;
- uintptr_t addr = entry->start;
ElfW(Ehdr) ehdr;
- if (!get_val<ElfW(Half)>(entry, addr + offsetof(ElfW(Ehdr), e_phnum), &ehdr.e_phnum)) {
- return;
+ struct iovec src_io = {.iov_base = reinterpret_cast<void*>(start_), .iov_len = SELFMAG};
+ struct iovec dst_io = {.iov_base = ehdr.e_ident, .iov_len = SELFMAG};
+ ssize_t rc = process_vm_readv(getpid(), &dst_io, 1, &src_io, 1, 0);
+ valid_ = rc == SELFMAG && IS_ELF(ehdr);
+}
+
+uintptr_t MapEntry::GetLoadBias() {
+ if (!valid_) {
+ return 0;
}
- if (!get_val<ElfW(Off)>(entry, addr + offsetof(ElfW(Ehdr), e_phoff), &ehdr.e_phoff)) {
- return;
+
+ if (load_bias_read_) {
+ return load_bias_;
}
- addr += ehdr.e_phoff;
+
+ load_bias_read_ = true;
+
+ ElfW(Ehdr) ehdr;
+ struct iovec src_io = {.iov_base = reinterpret_cast<void*>(start_), .iov_len = sizeof(ehdr)};
+ struct iovec dst_io = {.iov_base = &ehdr, .iov_len = sizeof(ehdr)};
+ ssize_t rc = process_vm_readv(getpid(), &dst_io, 1, &src_io, 1, 0);
+ if (rc != sizeof(ehdr)) {
+ return 0;
+ }
+
+ uintptr_t addr = start_ + ehdr.e_phoff;
for (size_t i = 0; i < ehdr.e_phnum; i++) {
ElfW(Phdr) phdr;
- if (!get_val<ElfW(Word)>(entry, addr + offsetof(ElfW(Phdr), p_type), &phdr.p_type)) {
- return;
- }
- if (!get_val<ElfW(Word)>(entry, addr + offsetof(ElfW(Phdr), p_flags), &phdr.p_flags)) {
- return;
- }
- if (!get_val<ElfW(Off)>(entry, addr + offsetof(ElfW(Phdr), p_offset), &phdr.p_offset)) {
- return;
+
+ src_io.iov_base = reinterpret_cast<void*>(addr);
+ src_io.iov_len = sizeof(phdr);
+ dst_io.iov_base = &phdr;
+ dst_io.iov_len = sizeof(phdr);
+ rc = process_vm_readv(getpid(), &dst_io, 1, &src_io, 1, 0);
+ if (rc != sizeof(phdr)) {
+ return 0;
}
if ((phdr.p_type == PT_LOAD) && (phdr.p_flags & PF_X) ) {
- if (!get_val<ElfW(Addr)>(entry, addr + offsetof(ElfW(Phdr), p_vaddr), &phdr.p_vaddr)) {
- return;
- }
- entry->load_bias = phdr.p_vaddr - phdr.p_offset;
- return;
+ load_bias_ = phdr.p_vaddr - phdr.p_offset;
+ return load_bias_;
}
addr += sizeof(phdr);
}
+ return 0;
}
-static void inline init(MapEntry* entry) {
- if (entry->init) {
- return;
- }
- entry->init = true;
- if (valid_elf(entry)) {
- entry->valid = true;
- read_loadbias(entry);
- }
-}
-
-bool MapData::ReadMaps() {
+void MapData::ReadMaps() {
+ std::lock_guard<std::mutex> lock(m_);
FILE* fp = fopen("/proc/self/maps", "re");
if (fp == nullptr) {
- return false;
+ return;
}
+ ClearEntries();
+
std::vector<char> buffer(1024);
while (fgets(buffer.data(), buffer.size(), fp) != nullptr) {
MapEntry* entry = parse_line(buffer.data());
if (entry == nullptr) {
- fclose(fp);
- return false;
+ break;
}
-
- auto it = entries_.find(entry);
- if (it == entries_.end()) {
- entries_.insert(entry);
- } else {
- delete entry;
- }
+ entries_.insert(entry);
}
fclose(fp);
- return true;
}
-MapData::~MapData() {
+void MapData::ClearEntries() {
for (auto* entry : entries_) {
delete entry;
}
entries_.clear();
}
+MapData::~MapData() {
+ ClearEntries();
+}
+
// Find the containing map info for the PC.
const MapEntry* MapData::find(uintptr_t pc, uintptr_t* rel_pc) {
MapEntry pc_entry(pc);
std::lock_guard<std::mutex> lock(m_);
-
auto it = entries_.find(&pc_entry);
if (it == entries_.end()) {
- ReadMaps();
- }
- it = entries_.find(&pc_entry);
- if (it == entries_.end()) {
return nullptr;
}
MapEntry* entry = *it;
- init(entry);
+ entry->Init();
if (rel_pc != nullptr) {
// Need to check to see if this is a read-execute map and the read-only
// map is the previous one.
- if (!entry->valid && it != entries_.begin()) {
+ if (!entry->valid() && it != entries_.begin()) {
MapEntry* prev_entry = *--it;
- if (prev_entry->flags == PROT_READ && prev_entry->offset < entry->offset &&
- prev_entry->name == entry->name) {
- init(prev_entry);
+ if (prev_entry->flags() == PROT_READ && prev_entry->offset() < entry->offset() &&
+ prev_entry->name() == entry->name()) {
+ prev_entry->Init();
- if (prev_entry->valid) {
- entry->elf_start_offset = prev_entry->offset;
- *rel_pc = pc - entry->start + entry->offset + prev_entry->load_bias;
+ if (prev_entry->valid()) {
+ entry->set_elf_start_offset(prev_entry->offset());
+ *rel_pc = pc - entry->start() + entry->offset() + prev_entry->GetLoadBias();
return entry;
}
}
}
- *rel_pc = pc - entry->start + entry->offset + entry->load_bias;
+ *rel_pc = pc - entry->start() + entry->offset() + entry->GetLoadBias();
}
return entry;
}
diff --git a/libc/malloc_debug/MapData.h b/libc/malloc_debug/MapData.h
index f2b3c1c..13bf9cb 100644
--- a/libc/malloc_debug/MapData.h
+++ b/libc/malloc_debug/MapData.h
@@ -36,26 +36,50 @@
#include <platform/bionic/macros.h>
-struct MapEntry {
- MapEntry(uintptr_t start, uintptr_t end, uintptr_t offset, const char* name, size_t name_len, int flags)
- : start(start), end(end), offset(offset), name(name, name_len), flags(flags) {}
+class MapEntry {
+ public:
+ MapEntry() = default;
+ MapEntry(uintptr_t start, uintptr_t end, uintptr_t offset, const char* name, size_t name_len,
+ int flags)
+ : start_(start), end_(end), offset_(offset), name_(name, name_len), flags_(flags) {}
- explicit MapEntry(uintptr_t pc) : start(pc), end(pc) {}
+ explicit MapEntry(uintptr_t pc) : start_(pc), end_(pc) {}
- uintptr_t start;
- uintptr_t end;
- uintptr_t offset;
- uintptr_t load_bias;
- uintptr_t elf_start_offset = 0;
- std::string name;
- int flags;
- bool init = false;
- bool valid = false;
+ void Init();
+
+ uintptr_t GetLoadBias();
+
+ void SetInvalid() {
+ valid_ = false;
+ init_ = true;
+ load_bias_read_ = true;
+ }
+
+ bool valid() { return valid_; }
+ uintptr_t start() const { return start_; }
+ uintptr_t end() const { return end_; }
+ uintptr_t offset() const { return offset_; }
+ uintptr_t elf_start_offset() const { return elf_start_offset_; }
+ void set_elf_start_offset(uintptr_t elf_start_offset) { elf_start_offset_ = elf_start_offset; }
+ const std::string& name() const { return name_; }
+ int flags() const { return flags_; }
+
+ private:
+ uintptr_t start_;
+ uintptr_t end_;
+ uintptr_t offset_;
+ uintptr_t load_bias_ = 0;
+ uintptr_t elf_start_offset_ = 0;
+ std::string name_;
+ int flags_;
+ bool init_ = false;
+ bool valid_ = false;
+ bool load_bias_read_ = false;
};
// Ordering comparator that returns equivalence for overlapping entries
struct compare_entries {
- bool operator()(const MapEntry* a, const MapEntry* b) const { return a->end <= b->start; }
+ bool operator()(const MapEntry* a, const MapEntry* b) const { return a->end() <= b->start(); }
};
class MapData {
@@ -65,11 +89,15 @@
const MapEntry* find(uintptr_t pc, uintptr_t* rel_pc = nullptr);
- private:
- bool ReadMaps();
+ size_t NumMaps() { return entries_.size(); }
+ void ReadMaps();
+
+ private:
std::mutex m_;
std::set<MapEntry*, compare_entries> entries_;
+ void ClearEntries();
+
BIONIC_DISALLOW_COPY_AND_ASSIGN(MapData);
};
diff --git a/libc/malloc_debug/backtrace.cpp b/libc/malloc_debug/backtrace.cpp
index ecb3a80..6a32fca 100644
--- a/libc/malloc_debug/backtrace.cpp
+++ b/libc/malloc_debug/backtrace.cpp
@@ -50,7 +50,7 @@
typedef struct _Unwind_Context __unwind_context;
static MapData g_map_data;
-static const MapEntry* g_current_code_map = nullptr;
+static MapEntry g_current_code_map;
static _Unwind_Reason_Code find_current_map(__unwind_context* context, void*) {
uintptr_t ip = _Unwind_GetIP(context);
@@ -58,11 +58,15 @@
if (ip == 0) {
return _URC_END_OF_STACK;
}
- g_current_code_map = g_map_data.find(ip);
+ auto map = g_map_data.find(ip);
+ if (map != nullptr) {
+ g_current_code_map = *map;
+ }
return _URC_END_OF_STACK;
}
void backtrace_startup() {
+ g_map_data.ReadMaps();
_Unwind_Backtrace(find_current_map, nullptr);
}
@@ -98,7 +102,8 @@
}
// Do not record the frames that fall in our own shared library.
- if (g_current_code_map && (ip >= g_current_code_map->start) && ip < g_current_code_map->end) {
+ if (g_current_code_map.start() != 0 && (ip >= g_current_code_map.start()) &&
+ ip < g_current_code_map.end()) {
return _URC_NO_REASON;
}
@@ -113,6 +118,10 @@
}
std::string backtrace_string(const uintptr_t* frames, size_t frame_count) {
+ if (g_map_data.NumMaps() == 0) {
+ g_map_data.ReadMaps();
+ }
+
std::string str;
for (size_t frame_num = 0; frame_num < frame_count; frame_num++) {
@@ -130,14 +139,15 @@
uintptr_t rel_pc = offset;
const MapEntry* entry = g_map_data.find(frames[frame_num], &rel_pc);
- const char* soname = (entry != nullptr) ? entry->name.c_str() : info.dli_fname;
+ const char* soname = (entry != nullptr) ? entry->name().c_str() : info.dli_fname;
if (soname == nullptr) {
soname = "<unknown>";
}
char offset_buf[128];
- if (entry != nullptr && entry->elf_start_offset != 0) {
- snprintf(offset_buf, sizeof(offset_buf), " (offset 0x%" PRIxPTR ")", entry->elf_start_offset);
+ if (entry != nullptr && entry->elf_start_offset() != 0) {
+ snprintf(offset_buf, sizeof(offset_buf), " (offset 0x%" PRIxPTR ")",
+ entry->elf_start_offset());
} else {
offset_buf[0] = '\0';
}
@@ -167,5 +177,6 @@
}
void backtrace_log(const uintptr_t* frames, size_t frame_count) {
+ g_map_data.ReadMaps();
error_log_string(backtrace_string(frames, frame_count).c_str());
}