From 6345d7f2a829faea56ad522a7d5180043f862a5c Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Fri, 8 Apr 2022 19:53:41 -0700 Subject: [PATCH] [sanitizer] Don't run malloc hooks for stacktraces Usually when we generated stacktraces the process is in error state, so running hooks may crash the process and prevent meaningfull error report. Symbolizer, unwinder and pthread are potential source of mallocs. https://b.corp.google.com/issues/228110771 Reviewed By: kda Differential Revision: https://reviews.llvm.org/D123566 --- compiler-rt/lib/msan/msan_allocator.cpp | 13 +++++---- .../lib/sanitizer_common/sanitizer_common.cpp | 11 ++++++++ .../lib/sanitizer_common/sanitizer_common.h | 9 ++++++ .../lib/sanitizer_common/sanitizer_fuchsia.cpp | 1 + .../sanitizer_common/sanitizer_linux_libcdep.cpp | 1 + compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp | 1 + .../lib/sanitizer_common/sanitizer_symbolizer.h | 2 ++ .../sanitizer_symbolizer_markup.cpp | 1 + .../sanitizer_unwind_linux_libcdep.cpp | 2 ++ .../TestCases/malloc_hook_skip.cpp | 33 ++++++++++++++++++++++ 10 files changed, 69 insertions(+), 5 deletions(-) create mode 100644 compiler-rt/test/sanitizer_common/TestCases/malloc_hook_skip.cpp diff --git a/compiler-rt/lib/msan/msan_allocator.cpp b/compiler-rt/lib/msan/msan_allocator.cpp index 0d5e853..7ff1bc9 100644 --- a/compiler-rt/lib/msan/msan_allocator.cpp +++ b/compiler-rt/lib/msan/msan_allocator.cpp @@ -194,16 +194,19 @@ static void *MsanAllocate(StackTrace *stack, uptr size, uptr alignment, __msan_set_origin(allocated, size, o.raw_id()); } } - UnpoisonParam(2); - RunMallocHooks(allocated, size); + if (!IsInSymbolizerOrUnwider()) { + UnpoisonParam(2); + RunMallocHooks(allocated, size); + } return allocated; } void MsanDeallocate(StackTrace *stack, void *p) { CHECK(p); - UnpoisonParam(1); - RunFreeHooks(p); - + if (!IsInSymbolizerOrUnwider()) { + UnpoisonParam(1); + RunFreeHooks(p); + } Metadata *meta = reinterpret_cast(allocator.GetMetaData(p)); uptr size = meta->requested_size; meta->requested_size = 0; diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_common.cpp index e30a93d..e625348 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_common.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_common.cpp @@ -310,7 +310,11 @@ struct MallocFreeHook { static MallocFreeHook MFHooks[kMaxMallocFreeHooks]; +static THREADLOCAL int disable_malloc_hooks; + void RunMallocHooks(void *ptr, uptr size) { + if (disable_malloc_hooks) + return; __sanitizer_malloc_hook(ptr, size); for (int i = 0; i < kMaxMallocFreeHooks; i++) { auto hook = MFHooks[i].malloc_hook; @@ -321,6 +325,8 @@ void RunMallocHooks(void *ptr, uptr size) { } void RunFreeHooks(void *ptr) { + if (disable_malloc_hooks) + return; __sanitizer_free_hook(ptr); for (int i = 0; i < kMaxMallocFreeHooks; i++) { auto hook = MFHooks[i].free_hook; @@ -330,6 +336,11 @@ void RunFreeHooks(void *ptr) { } } +ScopedDisableMallocHooks::ScopedDisableMallocHooks() { ++disable_malloc_hooks; } +ScopedDisableMallocHooks::~ScopedDisableMallocHooks() { + --disable_malloc_hooks; +} + static int InstallMallocFreeHooks(void (*malloc_hook)(const void *, uptr), void (*free_hook)(const void *)) { if (!malloc_hook || !free_hook) return 0; diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common.h b/compiler-rt/lib/sanitizer_common/sanitizer_common.h index 17570d6..0245687 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_common.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_common.h @@ -170,9 +170,18 @@ void SetShadowRegionHugePageMode(uptr addr, uptr length); bool DontDumpShadowMemory(uptr addr, uptr length); // Check if the built VMA size matches the runtime one. void CheckVMASize(); + void RunMallocHooks(void *ptr, uptr size); void RunFreeHooks(void *ptr); +// Prevents RunMallocHooks and RunFreeHooks. Can be used in places where hooks +// are undesirable, like in symbolizer or unwinder. +class ScopedDisableMallocHooks { + public: + ScopedDisableMallocHooks(); + ~ScopedDisableMallocHooks(); +}; + class ReservedAddressRange { public: uptr Init(uptr size, const char *name = nullptr, uptr fixed_addr = 0); diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cpp index 848953a..97bef87 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cpp @@ -75,6 +75,7 @@ void Abort() { abort(); } int Atexit(void (*function)(void)) { return atexit(function); } void GetThreadStackTopAndBottom(bool, uptr *stack_top, uptr *stack_bottom) { + ScopedDisableMallocHooks disable_hooks; // pthread can malloc. pthread_attr_t attr; CHECK_EQ(pthread_getattr_np(pthread_self(), &attr), 0); void *base; diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp index 25ad825..b757731 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp @@ -103,6 +103,7 @@ void GetThreadStackTopAndBottom(bool at_initialization, uptr *stack_top, uptr *stack_bottom) { CHECK(stack_top); CHECK(stack_bottom); + ScopedDisableMallocHooks disable_hooks; // pthread can malloc. if (at_initialization) { // This is the main thread. Libpthread may not be initialized yet. struct rlimit rl; diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp index 41b90b9..e8f7af4 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp @@ -410,6 +410,7 @@ void GetThreadStackTopAndBottom(bool at_initialization, uptr *stack_top, uptr *stack_bottom) { CHECK(stack_top); CHECK(stack_bottom); + ScopedDisableMallocHooks disable_hooks; // pthread can malloc. uptr stacksize = pthread_get_stacksize_np(pthread_self()); // pthread_get_stacksize_np() returns an incorrect stack size for the main // thread on Mavericks. See diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.h b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.h index bad4761..96b27c0 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.h @@ -212,6 +212,8 @@ class Symbolizer final { ~SymbolizerScope(); private: const Symbolizer *sym_; + + ScopedDisableMallocHooks disable_hooks_; // Symbolizer can malloc. }; }; diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp index 1ec0c5c..9670dcf 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp @@ -126,6 +126,7 @@ _Unwind_Reason_Code Unwind_Trace(struct _Unwind_Context *ctx, void *param) { void BufferedStackTrace::UnwindSlow(uptr pc, u32 max_depth) { CHECK_GE(max_depth, 2); size = 0; + ScopedDisableMallocHooks disable_hooks; // libunwind can malloc. UnwindTraceArg arg = {this, Min(max_depth + 1, kStackTraceMax)}; _Unwind_Backtrace(Unwind_Trace, &arg); CHECK_GT(size, 0); diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_unwind_linux_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_unwind_linux_libcdep.cpp index b2628dc..89db2a3 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_unwind_linux_libcdep.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_unwind_linux_libcdep.cpp @@ -126,6 +126,7 @@ void SanitizerInitializeUnwinder() { void BufferedStackTrace::UnwindSlow(uptr pc, u32 max_depth) { CHECK_GE(max_depth, 2); size = 0; + ScopedDisableMallocHooks disable_hooks; // libunwind can malloc. UnwindTraceArg arg = {this, Min(max_depth + 1, kStackTraceMax)}; _Unwind_Backtrace(Unwind_Trace, &arg); // We need to pop a few frames so that pc is on top. @@ -156,6 +157,7 @@ void BufferedStackTrace::UnwindSlow(uptr pc, void *context, u32 max_depth) { return; } + ScopedDisableMallocHooks disable_hooks; // Maybe unneeded, but won't hurt. void *map = acquire_my_map_info_list(); CHECK(map); InternalMmapVector frames(kStackTraceMax); diff --git a/compiler-rt/test/sanitizer_common/TestCases/malloc_hook_skip.cpp b/compiler-rt/test/sanitizer_common/TestCases/malloc_hook_skip.cpp new file mode 100644 index 0000000..4b6b3e0 --- /dev/null +++ b/compiler-rt/test/sanitizer_common/TestCases/malloc_hook_skip.cpp @@ -0,0 +1,33 @@ +// RUN: %clangxx -O0 -g %s -o %t && %run %t + +#include +#include +#include +#include + +static int hooks; +extern "C" { + +void __sanitizer_malloc_hook(const volatile void *ptr, size_t sz) { ++hooks; } + +void __sanitizer_free_hook(const volatile void *ptr) { ++hooks; } + +} // extern "C" + +void MallocHook(const volatile void *ptr, size_t sz) { ++hooks; } +void FreeHook(const volatile void *ptr) { ++hooks; } + +int main() { + int before; + + before = hooks; + __sanitizer_print_stack_trace(); + assert(before == hooks); + + __sanitizer_install_malloc_and_free_hooks(MallocHook, FreeHook); + before = hooks; + __sanitizer_print_stack_trace(); + assert(before == hooks); + + return 0; +} -- 2.7.4