Merge "[MTE] Cleanup tagged si_addr refs to fix mappings OOB bug."

This commit is contained in:
Mitch Phillips 2021-01-25 17:20:40 +00:00 committed by Gerrit Code Review
commit 1d792bf90a
5 changed files with 15 additions and 10 deletions

View file

@ -40,6 +40,7 @@
#include <android-base/stringprintf.h> #include <android-base/stringprintf.h>
#include <android-base/strings.h> #include <android-base/strings.h>
#include <android-base/unique_fd.h> #include <android-base/unique_fd.h>
#include <bionic/macros.h>
#include <bionic/reserved_signals.h> #include <bionic/reserved_signals.h>
#include <cutils/sockets.h> #include <cutils/sockets.h>
#include <log/log.h> #include <log/log.h>
@ -299,7 +300,9 @@ static void ReadCrashInfo(unique_fd& fd, siginfo_t* siginfo,
*siginfo = crash_info->data.s.siginfo; *siginfo = crash_info->data.s.siginfo;
if (signal_has_si_addr(siginfo)) { if (signal_has_si_addr(siginfo)) {
process_info->has_fault_address = true; process_info->has_fault_address = true;
process_info->fault_address = reinterpret_cast<uintptr_t>(siginfo->si_addr); process_info->maybe_tagged_fault_address = reinterpret_cast<uintptr_t>(siginfo->si_addr);
process_info->untagged_fault_address =
untag_address(reinterpret_cast<uintptr_t>(siginfo->si_addr));
} }
regs->reset(unwindstack::Regs::CreateFromUcontext(unwindstack::Regs::CurrentArch(), regs->reset(unwindstack::Regs::CreateFromUcontext(unwindstack::Regs::CurrentArch(),
&crash_info->data.s.ucontext)); &crash_info->data.s.ucontext));

View file

@ -72,8 +72,8 @@ GwpAsanCrashData::GwpAsanCrashData(unwindstack::Memory* process_memory,
// Get the external crash address from the thread info. // Get the external crash address from the thread info.
crash_address_ = 0u; crash_address_ = 0u;
if (signal_has_si_addr(thread_info.siginfo)) { if (process_info.has_fault_address) {
crash_address_ = reinterpret_cast<uintptr_t>(thread_info.siginfo->si_addr); crash_address_ = process_info.untagged_fault_address;
} }
// Ensure the error belongs to GWP-ASan. // Ensure the error belongs to GWP-ASan.

View file

@ -46,5 +46,6 @@ struct ProcessInfo {
uintptr_t scudo_region_info = 0; uintptr_t scudo_region_info = 0;
bool has_fault_address = false; bool has_fault_address = false;
uintptr_t fault_address = 0; uintptr_t untagged_fault_address = 0;
uintptr_t maybe_tagged_fault_address = 0;
}; };

View file

@ -44,7 +44,7 @@ ScudoCrashData::ScudoCrashData(unwindstack::Memory* process_memory,
auto region_info = AllocAndReadFully(process_memory, process_info.scudo_region_info, auto region_info = AllocAndReadFully(process_memory, process_info.scudo_region_info,
__scudo_get_region_info_size()); __scudo_get_region_info_size());
untagged_fault_addr_ = untag_address(process_info.fault_address); untagged_fault_addr_ = process_info.untagged_fault_address;
uintptr_t fault_page = untagged_fault_addr_ & ~(PAGE_SIZE - 1); uintptr_t fault_page = untagged_fault_addr_ & ~(PAGE_SIZE - 1);
uintptr_t memory_begin = fault_page - PAGE_SIZE * 16; uintptr_t memory_begin = fault_page - PAGE_SIZE * 16;
@ -67,7 +67,7 @@ ScudoCrashData::ScudoCrashData(unwindstack::Memory* process_memory,
memory_tags[(i - memory_begin) / kTagGranuleSize] = process_memory->ReadTag(i); memory_tags[(i - memory_begin) / kTagGranuleSize] = process_memory->ReadTag(i);
} }
__scudo_get_error_info(&error_info_, process_info.fault_address, stack_depot.get(), __scudo_get_error_info(&error_info_, process_info.maybe_tagged_fault_address, stack_depot.get(),
region_info.get(), memory.get(), memory_tags.get(), memory_begin, region_info.get(), memory.get(), memory_tags.get(), memory_begin,
memory_end - memory_begin); memory_end - memory_begin);
} }

View file

@ -151,7 +151,9 @@ static void dump_signal_info(log_t* log, const ThreadInfo& thread_info,
const ProcessInfo& process_info, unwindstack::Memory* process_memory) { const ProcessInfo& process_info, unwindstack::Memory* process_memory) {
char addr_desc[64]; // ", fault addr 0x1234" char addr_desc[64]; // ", fault addr 0x1234"
if (process_info.has_fault_address) { if (process_info.has_fault_address) {
size_t addr = process_info.fault_address; // SIGILL faults will never have tagged addresses, so okay to
// indiscriminately use the tagged address here.
size_t addr = process_info.maybe_tagged_fault_address;
if (thread_info.siginfo->si_signo == SIGILL) { if (thread_info.siginfo->si_signo == SIGILL) {
uint32_t instruction = {}; uint32_t instruction = {};
process_memory->Read(addr, &instruction, sizeof(instruction)); process_memory->Read(addr, &instruction, sizeof(instruction));
@ -433,9 +435,8 @@ static bool dump_thread(log_t* log, unwindstack::Unwinder* unwinder, const Threa
thread_info.registers.get()); thread_info.registers.get());
if (maps != nullptr) { if (maps != nullptr) {
uint64_t addr = 0; uint64_t addr = 0;
siginfo_t* si = thread_info.siginfo; if (process_info.has_fault_address) {
if (signal_has_si_addr(si)) { addr = process_info.untagged_fault_address;
addr = reinterpret_cast<uint64_t>(si->si_addr);
} }
dump_all_maps(log, unwinder, addr); dump_all_maps(log, unwinder, addr);
} }