From e26b25f8b1fdbc088abdfab933d909960989c64b Mon Sep 17 00:00:00 2001 From: Mitch Phillips <31459023+hctim@users.noreply.github.com> Date: Tue, 9 Jun 2020 11:57:24 -0700 Subject: [PATCH] [HWASan] Add sizeof(global) in report even if symbols missing. Summary: Refactor the current global header iteration to be callback-based, and add a feature that reports the size of the global variable during reporting. This allows binaries without symbols to still report the size of the global variable, which is always available in the HWASan globals PT_NOTE metadata. Reviewers: eugenis, pcc Reviewed By: pcc Subscribers: mgorny, llvm-commits, #sanitizers Tags: #sanitizers, #llvm Differential Revision: https://reviews.llvm.org/D80599 --- compiler-rt/lib/hwasan/CMakeLists.txt | 2 + compiler-rt/lib/hwasan/hwasan.cpp | 110 ++++----------------- compiler-rt/lib/hwasan/hwasan_globals.cpp | 91 +++++++++++++++++ compiler-rt/lib/hwasan/hwasan_globals.h | 49 +++++++++ compiler-rt/lib/hwasan/hwasan_report.cpp | 58 ++++++++++- .../lib/sanitizer_common/sanitizer_common.h | 14 +++ compiler-rt/test/hwasan/TestCases/global.c | 4 +- llvm/utils/gn/build/libs/pthread/BUILD.gn | 7 +- .../gn/secondary/compiler-rt/lib/hwasan/BUILD.gn | 2 + 9 files changed, 239 insertions(+), 98 deletions(-) create mode 100644 compiler-rt/lib/hwasan/hwasan_globals.cpp create mode 100644 compiler-rt/lib/hwasan/hwasan_globals.h diff --git a/compiler-rt/lib/hwasan/CMakeLists.txt b/compiler-rt/lib/hwasan/CMakeLists.txt index 03863e4..d294579 100644 --- a/compiler-rt/lib/hwasan/CMakeLists.txt +++ b/compiler-rt/lib/hwasan/CMakeLists.txt @@ -6,6 +6,7 @@ set(HWASAN_RTL_SOURCES hwasan_allocator.cpp hwasan_dynamic_shadow.cpp hwasan_exceptions.cpp + hwasan_globals.cpp hwasan_interceptors.cpp hwasan_interceptors_vfork.S hwasan_linux.cpp @@ -29,6 +30,7 @@ set(HWASAN_RTL_HEADERS hwasan_dynamic_shadow.h hwasan_flags.h hwasan_flags.inc + hwasan_globals.h hwasan_interface_internal.h hwasan_malloc_bisect.h hwasan_mapping.h diff --git a/compiler-rt/lib/hwasan/hwasan.cpp b/compiler-rt/lib/hwasan/hwasan.cpp index 6f64829..d67a88d 100644 --- a/compiler-rt/lib/hwasan/hwasan.cpp +++ b/compiler-rt/lib/hwasan/hwasan.cpp @@ -12,8 +12,10 @@ //===----------------------------------------------------------------------===// #include "hwasan.h" + #include "hwasan_checks.h" #include "hwasan_dynamic_shadow.h" +#include "hwasan_globals.h" #include "hwasan_poisoning.h" #include "hwasan_report.h" #include "hwasan_thread.h" @@ -194,93 +196,20 @@ void __sanitizer::BufferedStackTrace::UnwindImpl( request_fast); } -struct hwasan_global { - s32 gv_relptr; - u32 info; -}; - -static void InitGlobals(const hwasan_global *begin, const hwasan_global *end) { - for (auto *desc = begin; desc != end; ++desc) { - uptr gv = reinterpret_cast(desc) + desc->gv_relptr; - uptr size = desc->info & 0xffffff; - uptr full_granule_size = RoundDownTo(size, 16); - u8 tag = desc->info >> 24; - TagMemoryAligned(gv, full_granule_size, tag); - if (size % 16) - TagMemoryAligned(gv + full_granule_size, 16, size % 16); - } -} - -enum { NT_LLVM_HWASAN_GLOBALS = 3 }; - -struct hwasan_global_note { - s32 begin_relptr; - s32 end_relptr; -}; - -// Check that the given library meets the code model requirements for tagged -// globals. These properties are not checked at link time so they need to be -// checked at runtime. -static void CheckCodeModel(ElfW(Addr) base, const ElfW(Phdr) * phdr, - ElfW(Half) phnum) { - ElfW(Addr) min_addr = -1ull, max_addr = 0; - for (unsigned i = 0; i != phnum; ++i) { - if (phdr[i].p_type != PT_LOAD) - continue; - ElfW(Addr) lo = base + phdr[i].p_vaddr, hi = lo + phdr[i].p_memsz; - if (min_addr > lo) - min_addr = lo; - if (max_addr < hi) - max_addr = hi; - } - - if (max_addr - min_addr > 1ull << 32) { - Report("FATAL: HWAddressSanitizer: library size exceeds 2^32\n"); - Die(); - } - if (max_addr > 1ull << 48) { - Report("FATAL: HWAddressSanitizer: library loaded above address 2^48\n"); - Die(); - } -} - -static void InitGlobalsFromPhdrs(ElfW(Addr) base, const ElfW(Phdr) * phdr, - ElfW(Half) phnum) { - for (unsigned i = 0; i != phnum; ++i) { - if (phdr[i].p_type != PT_NOTE) - continue; - const char *note = reinterpret_cast(base + phdr[i].p_vaddr); - const char *nend = note + phdr[i].p_memsz; - while (note < nend) { - auto *nhdr = reinterpret_cast(note); - const char *name = note + sizeof(ElfW(Nhdr)); - const char *desc = name + RoundUpTo(nhdr->n_namesz, 4); - if (nhdr->n_type != NT_LLVM_HWASAN_GLOBALS || - internal_strcmp(name, "LLVM") != 0) { - note = desc + RoundUpTo(nhdr->n_descsz, 4); - continue; - } - - // Only libraries with instrumented globals need to be checked against the - // code model since they use relocations that aren't checked at link time. - CheckCodeModel(base, phdr, phnum); - - auto *global_note = reinterpret_cast(desc); - auto *global_begin = reinterpret_cast( - note + global_note->begin_relptr); - auto *global_end = reinterpret_cast( - note + global_note->end_relptr); - InitGlobals(global_begin, global_end); - return; - } - } +static bool InitializeSingleGlobal(const hwasan_global &global) { + uptr full_granule_size = RoundDownTo(global.size(), 16); + TagMemoryAligned(global.addr(), full_granule_size, global.tag()); + if (global.size() % 16) + TagMemoryAligned(global.addr() + full_granule_size, 16, global.size() % 16); + return false; } static void InitLoadedGlobals() { dl_iterate_phdr( - [](dl_phdr_info *info, size_t size, void *data) { - InitGlobalsFromPhdrs(info->dlpi_addr, info->dlpi_phdr, - info->dlpi_phnum); + [](dl_phdr_info *info, size_t /* size */, void * /* data */) -> int { + for (const hwasan_global &global : HwasanGlobalsFor( + info->dlpi_addr, info->dlpi_phdr, info->dlpi_phnum)) + InitializeSingleGlobal(global); return 0; }, nullptr); @@ -321,11 +250,13 @@ void __hwasan_init_static() { // Fortunately, since this is a statically linked executable we can use the // linker-defined symbol __ehdr_start to find the only relevant set of phdrs. extern ElfW(Ehdr) __ehdr_start; - InitGlobalsFromPhdrs( - 0, - reinterpret_cast( - reinterpret_cast(&__ehdr_start) + __ehdr_start.e_phoff), - __ehdr_start.e_phnum); + for (const hwasan_global &global : HwasanGlobalsFor( + /* base */ 0, + reinterpret_cast( + reinterpret_cast(&__ehdr_start) + + __ehdr_start.e_phoff), + __ehdr_start.e_phnum)) + InitializeSingleGlobal(global); } void __hwasan_init() { @@ -384,7 +315,8 @@ void __hwasan_init() { void __hwasan_library_loaded(ElfW(Addr) base, const ElfW(Phdr) * phdr, ElfW(Half) phnum) { - InitGlobalsFromPhdrs(base, phdr, phnum); + for (const hwasan_global &global : HwasanGlobalsFor(base, phdr, phnum)) + InitializeSingleGlobal(global); } void __hwasan_library_unloaded(ElfW(Addr) base, const ElfW(Phdr) * phdr, diff --git a/compiler-rt/lib/hwasan/hwasan_globals.cpp b/compiler-rt/lib/hwasan/hwasan_globals.cpp new file mode 100644 index 0000000..d71bcd7 --- /dev/null +++ b/compiler-rt/lib/hwasan/hwasan_globals.cpp @@ -0,0 +1,91 @@ +//===-- hwasan_globals.cpp ------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This file is a part of HWAddressSanitizer. +// +// HWAddressSanitizer globals-specific runtime. +//===----------------------------------------------------------------------===// + +#include "hwasan_globals.h" + +namespace __hwasan { + +enum { NT_LLVM_HWASAN_GLOBALS = 3 }; +struct hwasan_global_note { + s32 begin_relptr; + s32 end_relptr; +}; + +// Check that the given library meets the code model requirements for tagged +// globals. These properties are not checked at link time so they need to be +// checked at runtime. +static void CheckCodeModel(ElfW(Addr) base, const ElfW(Phdr) * phdr, + ElfW(Half) phnum) { + ElfW(Addr) min_addr = -1ull, max_addr = 0; + for (unsigned i = 0; i != phnum; ++i) { + if (phdr[i].p_type != PT_LOAD) + continue; + ElfW(Addr) lo = base + phdr[i].p_vaddr, hi = lo + phdr[i].p_memsz; + if (min_addr > lo) + min_addr = lo; + if (max_addr < hi) + max_addr = hi; + } + + if (max_addr - min_addr > 1ull << 32) { + Report("FATAL: HWAddressSanitizer: library size exceeds 2^32\n"); + Die(); + } + if (max_addr > 1ull << 48) { + Report("FATAL: HWAddressSanitizer: library loaded above address 2^48\n"); + Die(); + } +} + +ArrayRef HwasanGlobalsFor(ElfW(Addr) base, + const ElfW(Phdr) * phdr, + ElfW(Half) phnum) { + // Read the phdrs from this DSO. + for (unsigned i = 0; i != phnum; ++i) { + if (phdr[i].p_type != PT_NOTE) + continue; + + const char *note = reinterpret_cast(base + phdr[i].p_vaddr); + const char *nend = note + phdr[i].p_memsz; + + // Traverse all the notes until we find a HWASan note. + while (note < nend) { + auto *nhdr = reinterpret_cast(note); + const char *name = note + sizeof(ElfW(Nhdr)); + const char *desc = name + RoundUpTo(nhdr->n_namesz, 4); + + // Discard non-HWASan-Globals notes. + if (nhdr->n_type != NT_LLVM_HWASAN_GLOBALS || + internal_strcmp(name, "LLVM") != 0) { + note = desc + RoundUpTo(nhdr->n_descsz, 4); + continue; + } + + // Only libraries with instrumented globals need to be checked against the + // code model since they use relocations that aren't checked at link time. + CheckCodeModel(base, phdr, phnum); + + auto *global_note = reinterpret_cast(desc); + auto *globals_begin = reinterpret_cast( + note + global_note->begin_relptr); + auto *globals_end = reinterpret_cast( + note + global_note->end_relptr); + + return {globals_begin, globals_end}; + } + } + + return {}; +} + +} // namespace __hwasan diff --git a/compiler-rt/lib/hwasan/hwasan_globals.h b/compiler-rt/lib/hwasan/hwasan_globals.h new file mode 100644 index 0000000..fd7adf7 --- /dev/null +++ b/compiler-rt/lib/hwasan/hwasan_globals.h @@ -0,0 +1,49 @@ +//===-- hwasan_globals.h ----------------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This file is a part of HWAddressSanitizer. +// +// Private Hwasan header. +//===----------------------------------------------------------------------===// + +#ifndef HWASAN_GLOBALS_H +#define HWASAN_GLOBALS_H + +#include + +#include "sanitizer_common/sanitizer_common.h" +#include "sanitizer_common/sanitizer_internal_defs.h" + +namespace __hwasan { +// This object should only ever be casted over the global (i.e. not constructed) +// in the ELF PT_NOTE in order for `addr()` to work correctly. +struct hwasan_global { + // The size of this global variable. Note that the size in the descriptor is + // max 1 << 24. Larger globals have multiple descriptors. + uptr size() const { return info & 0xffffff; } + // The fully-relocated address of this global. + uptr addr() const { return reinterpret_cast(this) + gv_relptr; } + // The static tag of this global. + u8 tag() const { return info >> 24; }; + + // The relative address between the start of the descriptor for the HWASan + // global (in the PT_NOTE), and the fully relocated address of the global. + s32 gv_relptr; + u32 info; +}; + +// Walk through the specific DSO (as specified by the base, phdr, and phnum), +// and return the range of the [beginning, end) of the HWASan globals descriptor +// array. +ArrayRef HwasanGlobalsFor(ElfW(Addr) base, + const ElfW(Phdr) * phdr, + ElfW(Half) phnum); + +} // namespace __hwasan + +#endif // HWASAN_GLOBALS_H diff --git a/compiler-rt/lib/hwasan/hwasan_report.cpp b/compiler-rt/lib/hwasan/hwasan_report.cpp index 21a24c4..4608adf 100644 --- a/compiler-rt/lib/hwasan/hwasan_report.cpp +++ b/compiler-rt/lib/hwasan/hwasan_report.cpp @@ -11,10 +11,14 @@ // Error reporting. //===----------------------------------------------------------------------===// +#include "hwasan_report.h" + +#include + #include "hwasan.h" #include "hwasan_allocator.h" +#include "hwasan_globals.h" #include "hwasan_mapping.h" -#include "hwasan_report.h" #include "hwasan_thread.h" #include "hwasan_thread_list.h" #include "sanitizer_common/sanitizer_allocator_internal.h" @@ -243,6 +247,42 @@ static bool TagsEqual(tag_t tag, tag_t *tag_ptr) { return tag == inline_tag; } +// HWASan globals store the size of the global in the descriptor. In cases where +// we don't have a binary with symbols, we can't grab the size of the global +// from the debug info - but we might be able to retrieve it from the +// descriptor. Returns zero if the lookup failed. +static uptr GetGlobalSizeFromDescriptor(uptr ptr) { + // Find the ELF object that this global resides in. + Dl_info info; + dladdr(reinterpret_cast(ptr), &info); + auto *ehdr = reinterpret_cast(info.dli_fbase); + auto *phdr = reinterpret_cast( + reinterpret_cast(ehdr) + ehdr->e_phoff); + + // Get the load bias. This is normally the same as the dli_fbase address on + // position-independent code, but can be different on non-PIE executables, + // binaries using LLD's partitioning feature, or binaries compiled with a + // linker script. + ElfW(Addr) load_bias = 0; + for (const auto &phdr : + ArrayRef(phdr, phdr + ehdr->e_phnum)) { + if (phdr.p_type != PT_LOAD || phdr.p_offset != 0) + continue; + load_bias = reinterpret_cast(ehdr) - phdr.p_vaddr; + break; + } + + // Walk all globals in this ELF object, looking for the one we're interested + // in. Once we find it, we can stop iterating and return the size of the + // global we're interested in. + for (const hwasan_global &global : + HwasanGlobalsFor(load_bias, phdr, ehdr->e_phnum)) + if (global.addr() <= ptr && ptr < global.addr() + global.size()) + return global.size(); + + return 0; +} + void PrintAddressDescription( uptr tagged_addr, uptr access_size, StackAllocationsRingBuffer *current_stack_allocations) { @@ -319,9 +359,19 @@ void PrintAddressDescription( candidate == left ? "right" : "left", info.size, info.name, info.start, info.start + info.size, module_name); } else { - Printf("%p is located to the %s of a global variable in (%s+0x%x)\n", - untagged_addr, candidate == left ? "right" : "left", - module_name, module_address); + uptr size = GetGlobalSizeFromDescriptor(mem); + if (size == 0) + // We couldn't find the size of the global from the descriptors. + Printf( + "%p is located to the %s of a global variable in (%s+0x%x)\n", + untagged_addr, candidate == left ? "right" : "left", + module_name, module_address); + else + Printf( + "%p is located to the %s of a %zd-byte global variable in " + "(%s+0x%x)\n", + untagged_addr, candidate == left ? "right" : "left", size, + module_name, module_address); } num_descriptions_printed++; } diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common.h b/compiler-rt/lib/sanitizer_common/sanitizer_common.h index ac16e0e..07b307a 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_common.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_common.h @@ -978,6 +978,20 @@ INLINE u32 GetNumberOfCPUsCached() { return NumberOfCPUsCached; } +template +class ArrayRef { + public: + ArrayRef() {} + ArrayRef(T *begin, T *end) : begin_(begin), end_(end) {} + + T *begin() { return begin_; } + T *end() { return end_; } + + private: + T *begin_ = nullptr; + T *end_ = nullptr; +}; + } // namespace __sanitizer inline void *operator new(__sanitizer::operator_new_size_type size, diff --git a/compiler-rt/test/hwasan/TestCases/global.c b/compiler-rt/test/hwasan/TestCases/global.c index f1aef22..7b6bbad 100644 --- a/compiler-rt/test/hwasan/TestCases/global.c +++ b/compiler-rt/test/hwasan/TestCases/global.c @@ -9,9 +9,9 @@ int x = 1; int main(int argc, char **argv) { // RSYM: is located 0 bytes to the right of 4-byte global variable x {{.*}} in {{.*}}global.c.tmp - // RNOSYM: is located to the right of a global variable in ({{.*}}global.c.tmp+{{.*}}) + // RNOSYM: is located to the right of a 4-byte global variable in ({{.*}}global.c.tmp+{{.*}}) // LSYM: is located 4 bytes to the left of 4-byte global variable x {{.*}} in {{.*}}global.c.tmp - // LNOSYM: is located to the left of a global variable in ({{.*}}global.c.tmp+{{.*}}) + // LNOSYM: is located to the left of a 4-byte global variable in ({{.*}}global.c.tmp+{{.*}}) // CHECK-NOT: can not describe (&x)[atoi(argv[1])] = 1; } diff --git a/llvm/utils/gn/build/libs/pthread/BUILD.gn b/llvm/utils/gn/build/libs/pthread/BUILD.gn index 69290ba..7708d31 100644 --- a/llvm/utils/gn/build/libs/pthread/BUILD.gn +++ b/llvm/utils/gn/build/libs/pthread/BUILD.gn @@ -2,11 +2,12 @@ import("//llvm/utils/gn/build/libs/pthread/enable.gni") config("pthread_config") { visibility = [ ":pthread" ] - ldflags = [ "-pthread" ] + libs = [ "pthread" ] } group("pthread") { - if (llvm_enable_threads && current_os != "win") { - all_dependent_configs = [ ":pthread_config" ] + # On Android, bionic has built-in support for pthreads. + if (llvm_enable_threads && current_os != "win" && current_os != "android") { + public_configs = [ ":pthread_config" ] } } diff --git a/llvm/utils/gn/secondary/compiler-rt/lib/hwasan/BUILD.gn b/llvm/utils/gn/secondary/compiler-rt/lib/hwasan/BUILD.gn index b3b1fdc..5b425a4 100644 --- a/llvm/utils/gn/secondary/compiler-rt/lib/hwasan/BUILD.gn +++ b/llvm/utils/gn/secondary/compiler-rt/lib/hwasan/BUILD.gn @@ -43,6 +43,8 @@ source_set("sources") { "hwasan_dynamic_shadow.h", "hwasan_exceptions.cpp", "hwasan_flags.h", + "hwasan_globals.cpp", + "hwasan_globals.h", "hwasan_interceptors.cpp", "hwasan_interceptors_vfork.S", "hwasan_interface_internal.h", -- 2.7.4