diff --git a/compiler-rt/lib/gwp_asan/guarded_pool_allocator.h b/compiler-rt/lib/gwp_asan/guarded_pool_allocator.h --- a/compiler-rt/lib/gwp_asan/guarded_pool_allocator.h +++ b/compiler-rt/lib/gwp_asan/guarded_pool_allocator.h @@ -98,14 +98,22 @@ // pool using the provided options. See options.inc for runtime configuration // options. void init(const options::Options &Opts); + void uninitTestOnly(); + + void disable(); + void enable(); // Return whether the allocation should be randomly chosen for sampling. GWP_ASAN_ALWAYS_INLINE bool shouldSample() { // NextSampleCounter == 0 means we "should regenerate the counter". // == 1 means we "should sample this allocation". + // AdjustedSampleRatePlusOne is designed to intentionally underflow. This + // class must be valid when zero-initialised, and we wish to sample as + // infrequently as possible when this is the case, hence we underflow to + // UINT32_MAX. if (GWP_ASAN_UNLIKELY(ThreadLocals.NextSampleCounter == 0)) ThreadLocals.NextSampleCounter = - (getRandomUnsigned32() % AdjustedSampleRate) + 1; + (getRandomUnsigned32() % (AdjustedSampleRatePlusOne - 1)) + 1; return GWP_ASAN_UNLIKELY(--ThreadLocals.NextSampleCounter == 0); } @@ -114,7 +122,7 @@ // is owned by this pool. GWP_ASAN_ALWAYS_INLINE bool pointerIsMine(const void *Ptr) const { uintptr_t P = reinterpret_cast(Ptr); - return GuardedPagePool <= P && P < GuardedPagePoolEnd; + return P < GuardedPagePoolEnd && GuardedPagePool <= P; } // Allocate memory in a guarded slot, and return a pointer to the new @@ -156,6 +164,7 @@ // mappings, call mapMemory() followed by markReadWrite() on the returned // pointer. void *mapMemory(size_t Size) const; + void unmapMemory(void *Addr, size_t Size) const; void markReadWrite(void *Ptr, size_t Size) const; void markInaccessible(void *Ptr, size_t Size) const; @@ -169,6 +178,7 @@ // signal(), we have to use platform-specific signal handlers to obtain the // address that caused the SIGSEGV exception. static void installSignalHandlers(); + static void uninstallSignalHandlers(); // Returns the index of the slot that this pointer resides in. If the pointer // is not owned by this pool, the result is undefined. @@ -210,6 +220,11 @@ void reportErrorInternal(uintptr_t AccessPtr, Error E); + static GuardedPoolAllocator *getSingleton(); + + // Install a pthread_atfork handler. + void installAtFork(); + // Cached page size for this system in bytes. size_t PageSize = 0; @@ -223,7 +238,7 @@ size_t NumSampledAllocations = 0; // Pointer to the pool of guarded slots. Note that this points to the start of // the pool (which is a guard page), not a pointer to the first guarded page. - uintptr_t GuardedPagePool = UINTPTR_MAX; + uintptr_t GuardedPagePool = 0; uintptr_t GuardedPagePoolEnd = 0; // Pointer to the allocation metadata (allocation/deallocation stack traces), // if any. @@ -250,7 +265,7 @@ // where we would calculate modulo zero. This value is set UINT32_MAX, as when // GWP-ASan is disabled, we wish to never spend wasted cycles recalculating // the sample rate. - uint32_t AdjustedSampleRate = UINT32_MAX; + uint32_t AdjustedSampleRatePlusOne = 0; // Pack the thread local variables into a struct to ensure that they're in // the same cache line for performance reasons. These are the most touched diff --git a/compiler-rt/lib/gwp_asan/guarded_pool_allocator.cpp b/compiler-rt/lib/gwp_asan/guarded_pool_allocator.cpp --- a/compiler-rt/lib/gwp_asan/guarded_pool_allocator.cpp +++ b/compiler-rt/lib/gwp_asan/guarded_pool_allocator.cpp @@ -58,7 +58,9 @@ // Gets the singleton implementation of this class. Thread-compatible until // init() is called, thread-safe afterwards. -GuardedPoolAllocator *getSingleton() { return SingletonPtr; } +GuardedPoolAllocator *GuardedPoolAllocator::getSingleton() { + return SingletonPtr; +} void GuardedPoolAllocator::AllocationMetadata::RecordAllocation( uintptr_t AllocAddr, size_t AllocSize, options::Backtrace_t Backtrace) { @@ -156,9 +158,9 @@ // Multiply the sample rate by 2 to give a good, fast approximation for (1 / // SampleRate) chance of sampling. if (Opts.SampleRate != 1) - AdjustedSampleRate = static_cast(Opts.SampleRate) * 2; + AdjustedSampleRatePlusOne = static_cast(Opts.SampleRate) * 2 + 1; else - AdjustedSampleRate = 1; + AdjustedSampleRatePlusOne = 2; GuardedPagePool = reinterpret_cast(GuardedPoolMemory); GuardedPagePoolEnd = @@ -169,6 +171,31 @@ // race to members if received during init(). if (Opts.InstallSignalHandlers) installSignalHandlers(); + + if (Opts.InstallForkHandlers) + installAtFork(); +} + +void GuardedPoolAllocator::disable() { PoolMutex.lock(); } + +void GuardedPoolAllocator::enable() { PoolMutex.unlock(); } + +void GuardedPoolAllocator::uninitTestOnly() { + if (GuardedPagePool) { + unmapMemory(reinterpret_cast(GuardedPagePool), + GuardedPagePoolEnd - GuardedPagePool); + GuardedPagePool = 0; + GuardedPagePoolEnd = 0; + } + if (Metadata) { + unmapMemory(Metadata, MaxSimultaneousAllocations * sizeof(*Metadata)); + Metadata = nullptr; + } + if (FreeSlots) { + unmapMemory(FreeSlots, MaxSimultaneousAllocations * sizeof(*FreeSlots)); + FreeSlots = nullptr; + } + uninstallSignalHandlers(); } void *GuardedPoolAllocator::allocate(size_t Size) { diff --git a/compiler-rt/lib/gwp_asan/options.inc b/compiler-rt/lib/gwp_asan/options.inc --- a/compiler-rt/lib/gwp_asan/options.inc +++ b/compiler-rt/lib/gwp_asan/options.inc @@ -39,3 +39,7 @@ "programs that install further signal handlers should make sure they do " "the same. Note, if the previously installed SIGSEGV handler is SIG_IGN, " "we terminate the process after dumping the error report.") + +GWP_ASAN_OPTION(bool, InstallForkHandlers, true, + "Install GWP-ASan atfork handlers to acquire internal locks " + "before fork and release them after.") diff --git a/compiler-rt/lib/gwp_asan/platform_specific/guarded_pool_allocator_posix.cpp b/compiler-rt/lib/gwp_asan/platform_specific/guarded_pool_allocator_posix.cpp --- a/compiler-rt/lib/gwp_asan/platform_specific/guarded_pool_allocator_posix.cpp +++ b/compiler-rt/lib/gwp_asan/platform_specific/guarded_pool_allocator_posix.cpp @@ -9,6 +9,7 @@ #include "gwp_asan/guarded_pool_allocator.h" #include +#include #include #include #include @@ -30,6 +31,16 @@ return Ptr; } +void GuardedPoolAllocator::unmapMemory(void *Addr, size_t Size) const { + int Res = munmap(Addr, Size); + + if (Res != 0) { + Printf("Failed to unmap guarded pool allocator memory, errno: %d\n", errno); + Printf(" unmmap(%p, %zu, ...) failed.\n", Addr, Size); + exit(EXIT_FAILURE); + } +} + void GuardedPoolAllocator::markReadWrite(void *Ptr, size_t Size) const { if (mprotect(Ptr, Size, PROT_READ | PROT_WRITE) != 0) { Printf("Failed to set guarded pool allocator memory at as RW, errno: %d\n", @@ -58,6 +69,7 @@ } struct sigaction PreviousHandler; +bool SignalHandlerInstalled; static void sigSegvHandler(int sig, siginfo_t *info, void *ucontext) { gwp_asan::GuardedPoolAllocator::reportError( @@ -78,11 +90,31 @@ } } +void GuardedPoolAllocator::installAtFork() { + auto Disable = []() { + if (auto *S = getSingleton()) + S->disable(); + }; + auto Enable = []() { + if (auto *S = getSingleton()) + S->enable(); + }; + pthread_atfork(Disable, Enable, Enable); +} + void GuardedPoolAllocator::installSignalHandlers() { struct sigaction Action; Action.sa_sigaction = sigSegvHandler; Action.sa_flags = SA_SIGINFO; sigaction(SIGSEGV, &Action, &PreviousHandler); + SignalHandlerInstalled = true; +} + +void GuardedPoolAllocator::uninstallSignalHandlers() { + if (SignalHandlerInstalled) { + sigaction(SIGSEGV, &PreviousHandler, nullptr); + SignalHandlerInstalled = false; + } } uint64_t GuardedPoolAllocator::getThreadID() { diff --git a/compiler-rt/lib/gwp_asan/tests/CMakeLists.txt b/compiler-rt/lib/gwp_asan/tests/CMakeLists.txt --- a/compiler-rt/lib/gwp_asan/tests/CMakeLists.txt +++ b/compiler-rt/lib/gwp_asan/tests/CMakeLists.txt @@ -17,7 +17,9 @@ driver.cpp mutex_test.cpp slot_reuse.cpp - thread_contention.cpp) + thread_contention.cpp + harness.cpp + enable_disable.cpp) set(GWP_ASAN_UNIT_TEST_HEADERS ${GWP_ASAN_HEADERS} diff --git a/compiler-rt/lib/gwp_asan/tests/enable_disable.cpp b/compiler-rt/lib/gwp_asan/tests/enable_disable.cpp new file mode 100644 --- /dev/null +++ b/compiler-rt/lib/gwp_asan/tests/enable_disable.cpp @@ -0,0 +1,86 @@ +//===-- enable_disable.cpp --------------------------------------*- 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 +// +//===----------------------------------------------------------------------===// + +#include "gwp_asan/tests/harness.h" + +constexpr size_t Size = 100; + +TEST_F(DefaultGuardedPoolAllocator, Fork) { + void *P; + pid_t Pid = fork(); + EXPECT_GE(Pid, 0); + if (Pid == 0) { + P = GPA.allocate(Size); + EXPECT_NE(P, nullptr); + memset(P, 0x42, Size); + GPA.deallocate(P); + _exit(0); + } + waitpid(Pid, nullptr, 0); + P = GPA.allocate(Size); + EXPECT_NE(P, nullptr); + memset(P, 0x42, Size); + GPA.deallocate(P); + + // fork should stall if the allocator has been disabled. + EXPECT_DEATH( + { + GPA.disable(); + alarm(1); + Pid = fork(); + EXPECT_GE(Pid, 0); + }, + ""); +} + +namespace { +pthread_mutex_t Mutex; +pthread_cond_t Conditional = PTHREAD_COND_INITIALIZER; +bool ThreadReady = false; + +void *enableMalloc(void *arg) { + auto &GPA = *reinterpret_cast(arg); + + // Signal the main thread we are ready. + pthread_mutex_lock(&Mutex); + ThreadReady = true; + pthread_cond_signal(&Conditional); + pthread_mutex_unlock(&Mutex); + + // Wait for the malloc_disable & fork, then enable the allocator again. + sleep(1); + GPA.enable(); + + return nullptr; +} + +TEST_F(DefaultGuardedPoolAllocator, DisableForkEnable) { + pthread_t ThreadId; + EXPECT_EQ(pthread_create(&ThreadId, nullptr, &enableMalloc, &GPA), 0); + + // Do not lock the allocator right away, the other thread may need it to start + // up. + pthread_mutex_lock(&Mutex); + while (!ThreadReady) + pthread_cond_wait(&Conditional, &Mutex); + pthread_mutex_unlock(&Mutex); + + // Disable the allocator and fork. fork should succeed after malloc_enable. + GPA.disable(); + pid_t Pid = fork(); + EXPECT_GE(Pid, 0); + if (Pid == 0) { + void *P = GPA.allocate(Size); + EXPECT_NE(P, nullptr); + GPA.deallocate(P); + _exit(0); + } + waitpid(Pid, nullptr, 0); + EXPECT_EQ(pthread_join(ThreadId, 0), 0); +} +} diff --git a/compiler-rt/lib/gwp_asan/tests/harness.h b/compiler-rt/lib/gwp_asan/tests/harness.h --- a/compiler-rt/lib/gwp_asan/tests/harness.h +++ b/compiler-rt/lib/gwp_asan/tests/harness.h @@ -24,20 +24,28 @@ // `optional/printf_sanitizer_common.cpp` which supplies the __sanitizer::Printf // for this purpose. options::Printf_t getPrintfFunction(); + +// First call returns true, all the following calls return false. +bool OnlyOnce(); + }; // namespace test }; // namespace gwp_asan class DefaultGuardedPoolAllocator : public ::testing::Test { public: - DefaultGuardedPoolAllocator() { + void SetUp() override { gwp_asan::options::Options Opts; Opts.setDefaults(); MaxSimultaneousAllocations = Opts.MaxSimultaneousAllocations; Opts.Printf = gwp_asan::test::getPrintfFunction(); + Opts.InstallForkHandlers = gwp_asan::test::OnlyOnce(); GPA.init(Opts); } + void TearDown() override { + GPA.uninitTestOnly(); + } protected: gwp_asan::GuardedPoolAllocator GPA; decltype(gwp_asan::options::Options::MaxSimultaneousAllocations) @@ -56,10 +64,15 @@ MaxSimultaneousAllocations = MaxSimultaneousAllocationsArg; Opts.Printf = gwp_asan::test::getPrintfFunction(); + Opts.InstallForkHandlers = gwp_asan::test::OnlyOnce(); GPA.init(Opts); } -protected: + void TearDown() override { + GPA.uninitTestOnly(); + } + + protected: gwp_asan::GuardedPoolAllocator GPA; decltype(gwp_asan::options::Options::MaxSimultaneousAllocations) MaxSimultaneousAllocations; @@ -67,16 +80,20 @@ class BacktraceGuardedPoolAllocator : public ::testing::Test { public: - BacktraceGuardedPoolAllocator() { + void SetUp() override { gwp_asan::options::Options Opts; Opts.setDefaults(); Opts.Printf = gwp_asan::test::getPrintfFunction(); Opts.Backtrace = gwp_asan::options::getBacktraceFunction(); Opts.PrintBacktrace = gwp_asan::options::getPrintBacktraceFunction(); + Opts.InstallForkHandlers = gwp_asan::test::OnlyOnce(); GPA.init(Opts); } + void TearDown() override { + GPA.uninitTestOnly(); + } protected: gwp_asan::GuardedPoolAllocator GPA; }; diff --git a/compiler-rt/lib/gwp_asan/tests/harness.cpp b/compiler-rt/lib/gwp_asan/tests/harness.cpp new file mode 100644 --- /dev/null +++ b/compiler-rt/lib/gwp_asan/tests/harness.cpp @@ -0,0 +1,10 @@ +#include "harness.h" + +namespace gwp_asan { +namespace test { +bool OnlyOnce() { + static int x = 0; + return !x++; +} +} +} diff --git a/compiler-rt/lib/scudo/standalone/CMakeLists.txt b/compiler-rt/lib/scudo/standalone/CMakeLists.txt --- a/compiler-rt/lib/scudo/standalone/CMakeLists.txt +++ b/compiler-rt/lib/scudo/standalone/CMakeLists.txt @@ -1,6 +1,5 @@ add_compiler_rt_component(scudo_standalone) -# FIXME: GWP-ASan is temporarily disabled, re-enable once issues are fixed. -if (FALSE AND COMPILER_RT_HAS_GWP_ASAN) +if (COMPILER_RT_HAS_GWP_ASAN) add_dependencies(scudo_standalone gwp_asan) endif() @@ -107,7 +106,7 @@ set(SCUDO_OBJECT_LIBS) -if (FALSE AND COMPILER_RT_HAS_GWP_ASAN) +if (COMPILER_RT_HAS_GWP_ASAN) list(APPEND SCUDO_OBJECT_LIBS RTGwpAsan) list(APPEND SCUDO_CFLAGS -DGWP_ASAN_HOOKS) endif() diff --git a/compiler-rt/lib/scudo/standalone/combined.h b/compiler-rt/lib/scudo/standalone/combined.h --- a/compiler-rt/lib/scudo/standalone/combined.h +++ b/compiler-rt/lib/scudo/standalone/combined.h @@ -24,12 +24,6 @@ #ifdef GWP_ASAN_HOOKS #include "gwp_asan/guarded_pool_allocator.h" -// GWP-ASan is declared here in order to avoid indirect call overhead. It's also -// instantiated outside of the Allocator class, as the allocator is only -// zero-initialised. GWP-ASan requires constant initialisation, and the Scudo -// allocator doesn't have a constexpr constructor (see discussion here: -// https://reviews.llvm.org/D69265#inline-624315). -static gwp_asan::GuardedPoolAllocator GuardedAlloc; #endif // GWP_ASAN_HOOKS extern "C" inline void EmptyCallback() {} @@ -153,7 +147,11 @@ Quarantine.init( static_cast(getFlags()->quarantine_size_kb << 10), static_cast(getFlags()->thread_local_quarantine_size_kb << 10)); + } + // Initialize the embedded GWP-ASan instance. Requires the main allocator to + // be functional, best called from PostInitCallback. + void initGwpAsan() { #ifdef GWP_ASAN_HOOKS gwp_asan::options::Options Opt; Opt.Enabled = getFlags()->GWP_ASAN_Enabled; @@ -166,6 +164,10 @@ getFlags()->GWP_ASAN_MaxSimultaneousAllocations; Opt.SampleRate = getFlags()->GWP_ASAN_SampleRate; Opt.InstallSignalHandlers = getFlags()->GWP_ASAN_InstallSignalHandlers; + // Embedded GWP-ASan is locked through the Scudo atfork handler (via + // Allocator::disable calling GWPASan.disable). Disable GWP-ASan's atfork + // handler. + Opt.InstallForkHandlers = false; Opt.Printf = Printf; GuardedAlloc.init(Opt); #endif // GWP_ASAN_HOOKS @@ -176,6 +178,9 @@ void unmapTestOnly() { TSDRegistry.unmapTestOnly(); Primary.unmapTestOnly(); +#ifdef GWP_ASAN_HOOKS + GuardedAlloc.uninitTestOnly(); +#endif // GWP_ASAN_HOOKS } TSDRegistryT *getTSDRegistry() { return &TSDRegistry; } @@ -509,6 +514,9 @@ // this function finishes. We will revisit that later. void disable() { initThreadMaybe(); +#ifdef GWP_ASAN_HOOKS + GuardedAlloc.disable(); +#endif TSDRegistry.disable(); Stats.disable(); Quarantine.disable(); @@ -523,6 +531,9 @@ Quarantine.enable(); Stats.enable(); TSDRegistry.enable(); +#ifdef GWP_ASAN_HOOKS + GuardedAlloc.enable(); +#endif } // The function returns the amount of bytes required to store the statistics, @@ -676,6 +687,10 @@ u32 QuarantineMaxChunkSize; // quarantine_max_chunk_size } Options; +#ifdef GWP_ASAN_HOOKS + gwp_asan::GuardedPoolAllocator GuardedAlloc; +#endif // GWP_ASAN_HOOKS + // The following might get optimized out by the compiler. NOINLINE void performSanityChecks() { // Verify that the header offset field can hold the maximum offset. In the diff --git a/compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt b/compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt --- a/compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt +++ b/compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt @@ -20,8 +20,7 @@ list(APPEND SCUDO_UNITTEST_CFLAGS -fno-emulated-tls) endif() -# FIXME: GWP-ASan is temporarily disabled, re-enable once issues are fixed. -if (FALSE AND COMPILER_RT_HAS_GWP_ASAN) +if (COMPILER_RT_HAS_GWP_ASAN) list(APPEND SCUDO_UNITTEST_CFLAGS -DGWP_ASAN_HOOKS) endif() @@ -43,7 +42,7 @@ macro(add_scudo_unittest testname) cmake_parse_arguments(TEST "" "" "SOURCES;ADDITIONAL_RTOBJECTS" ${ARGN}) - if (FALSE AND COMPILER_RT_HAS_GWP_ASAN) + if (COMPILER_RT_HAS_GWP_ASAN) list(APPEND TEST_ADDITIONAL_RTOBJECTS RTGwpAsan) endif() diff --git a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp --- a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp +++ b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp @@ -264,6 +264,17 @@ EXPECT_NE(Stats.find("Stats: Quarantine"), std::string::npos); } +// Test that multiple instantiations of the allocator have not messed up the +// process's signal handlers (GWP-ASan used to do this). +void testSEGV() { + const scudo::uptr Size = 4 * scudo::getPageSizeCached(); + scudo::MapPlatformData Data = {}; + void *P = scudo::map(nullptr, Size, "testSEGV", MAP_NOACCESS, &Data); + EXPECT_NE(P, nullptr); + EXPECT_DEATH(memset(P, 0xaa, Size), ""); + scudo::unmap(P, Size, UNMAP_ALL, &Data); +} + TEST(ScudoCombinedTest, BasicCombined) { UseQuarantine = false; testAllocator(); @@ -273,6 +284,7 @@ testAllocator(); UseQuarantine = true; testAllocator(); + testSEGV(); #endif } diff --git a/compiler-rt/lib/scudo/standalone/wrappers_c.inc b/compiler-rt/lib/scudo/standalone/wrappers_c.inc --- a/compiler-rt/lib/scudo/standalone/wrappers_c.inc +++ b/compiler-rt/lib/scudo/standalone/wrappers_c.inc @@ -150,6 +150,7 @@ } void SCUDO_PREFIX(malloc_postinit)() { + SCUDO_ALLOCATOR.initGwpAsan(); pthread_atfork(SCUDO_PREFIX(malloc_disable), SCUDO_PREFIX(malloc_enable), SCUDO_PREFIX(malloc_enable)); }