From 7a70b0f1513d2787437aafc6593c97cbd0f2d94e Mon Sep 17 00:00:00 2001 From: Joshua Peraza Date: Thu, 4 Jun 2020 17:31:52 -0700 Subject: [PATCH] android: correct executable placement in debug rendezvous On Android P, Bionic mistakenly places the vdso first in the list where the executable should be. Also correctly set the section size in the section headers for test module string tables. Bug: chromium:1050178 Change-Id: I83581d05c5ed3e25a237d1ce4a27c45755a3ab3c Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/2231525 Reviewed-by: Mark Mentovai Commit-Queue: Joshua Peraza --- snapshot/linux/debug_rendezvous.cc | 16 ++++++++++++++++ snapshot/linux/debug_rendezvous_test.cc | 7 ++++--- snapshot/linux/process_reader_linux.cc | 4 ++-- snapshot/linux/process_reader_linux_test.cc | 21 +++++++++++++++------ 4 files changed, 37 insertions(+), 11 deletions(-) diff --git a/snapshot/linux/debug_rendezvous.cc b/snapshot/linux/debug_rendezvous.cc index e27768dd..42384f74 100644 --- a/snapshot/linux/debug_rendezvous.cc +++ b/snapshot/linux/debug_rendezvous.cc @@ -19,6 +19,11 @@ #include #include "base/logging.h" +#include "build/build_config.h" + +#if defined(OS_ANDROID) +#include +#endif namespace crashpad { @@ -137,6 +142,17 @@ bool DebugRendezvous::InitializeSpecific(const ProcessMemoryRange& memory, modules_.push_back(entry); } +#if defined(OS_ANDROID) + // Android P (API 28) mistakenly places the vdso in the first entry in the + // link map. + const int android_runtime_api = android_get_device_api_level(); + if (android_runtime_api == 28 && executable_.name == "[vdso]") { + LinkEntry executable = modules_[0]; + modules_[0] = executable_; + executable_ = executable; + } +#endif // OS_ANDROID + return true; } diff --git a/snapshot/linux/debug_rendezvous_test.cc b/snapshot/linux/debug_rendezvous_test.cc index bbedddfe..71a0a7ba 100644 --- a/snapshot/linux/debug_rendezvous_test.cc +++ b/snapshot/linux/debug_rendezvous_test.cc @@ -93,8 +93,8 @@ void TestAgainstTarget(PtraceConnection* connection) { std::string::npos); // Android's loader doesn't set the dynamic array for the executable in the - // link map until Android 9.0 (API 28). - if (android_runtime_api >= 28) { + // link map until Android 10.0 (API 29). + if (android_runtime_api >= 29) { EXPECT_EQ(debug.Executable()->dynamic_array, exe_dynamic_address); } else { EXPECT_EQ(debug.Executable()->dynamic_array, 0u); @@ -147,7 +147,8 @@ void TestAgainstTarget(PtraceConnection* connection) { while ((mapping = possible_mappings->Next())) { auto parsed_module = std::make_unique(); VMAddress dynamic_address; - if (parsed_module->Initialize(range, mapping->range.Base()) && + if (parsed_module->Initialize( + range, mapping->range.Base(), possible_mappings->Count() == 0) && parsed_module->GetDynamicArrayAddress(&dynamic_address) && dynamic_address == module.dynamic_array) { module_reader = std::move(parsed_module); diff --git a/snapshot/linux/process_reader_linux.cc b/snapshot/linux/process_reader_linux.cc index b96abfe7..ee246e8b 100644 --- a/snapshot/linux/process_reader_linux.cc +++ b/snapshot/linux/process_reader_linux.cc @@ -439,7 +439,7 @@ void ProcessReaderLinux::InitializeModules() { if (parsed_exe->Initialize( range, mapping->range.Base(), - /* verbose= */ possible_mappings->Count() == 1) && + /* verbose= */ possible_mappings->Count() == 0) && parsed_exe->GetProgramHeaderTableAddress() == phdrs) { exe_mapping = mapping; exe_reader = std::move(parsed_exe); @@ -508,7 +508,7 @@ void ProcessReaderLinux::InitializeModules() { if (parsed_module->Initialize( range, mapping->range.Base(), - /* verbose= */ possible_mappings->Count() == 1) && + /* verbose= */ possible_mappings->Count() == 0) && parsed_module->GetDynamicArrayAddress(&dynamic_address) && dynamic_address == entry.dynamic_array) { module_mapping = mapping; diff --git a/snapshot/linux/process_reader_linux_test.cc b/snapshot/linux/process_reader_linux_test.cc index e5710acb..73e350db 100644 --- a/snapshot/linux/process_reader_linux_test.cc +++ b/snapshot/linux/process_reader_linux_test.cc @@ -491,23 +491,29 @@ int ExpectFindModule(dl_phdr_info* info, size_t size, void* data) { auto modules = reinterpret_cast*>(data); - auto phdr_addr = FromPointerCast(info->dlpi_phdr); #if defined(OS_ANDROID) - // Bionic includes a null entry. - if (!phdr_addr) { - EXPECT_EQ(info->dlpi_name, nullptr); + // Prior to API 27, Bionic includes a null entry for /system/bin/linker. + if (!info->dlpi_name) { EXPECT_EQ(info->dlpi_addr, 0u); EXPECT_EQ(info->dlpi_phnum, 0u); + EXPECT_EQ(info->dlpi_phdr, nullptr); return 0; } #endif + // Bionic doesn't always set both of these addresses for the vdso and + // /system/bin/linker, but it does always set one of them. + VMAddress module_addr = info->dlpi_phdr + ? FromPointerCast(info->dlpi_phdr) + : info->dlpi_addr; + // TODO(jperaza): This can use a range map when one is available. bool found = false; for (const auto& module : *modules) { - if (module.elf_reader && phdr_addr >= module.elf_reader->Address() && - phdr_addr < module.elf_reader->Address() + module.elf_reader->Size()) { + if (module.elf_reader && module_addr >= module.elf_reader->Address() && + module_addr < + module.elf_reader->Address() + module.elf_reader->Size()) { found = true; break; } @@ -702,11 +708,14 @@ bool WriteTestModule(const base::FilePath& module_path, module.shdr_table.string_table.sh_type = SHT_STRTAB; module.shdr_table.string_table.sh_offset = offsetof(decltype(module), string_table); + module.shdr_table.string_table.sh_size = sizeof(module.string_table); module.shdr_table.section_header_string_table.sh_name = 0; module.shdr_table.section_header_string_table.sh_type = SHT_STRTAB; module.shdr_table.section_header_string_table.sh_offset = offsetof(decltype(module), section_header_string_table); + module.shdr_table.section_header_string_table.sh_size = + sizeof(module.section_header_string_table); FileWriter writer; if (!writer.Open(module_path,