[GWP-ASan] enable/disable and fork support.

Summary:
* Implement enable() and disable() in GWP-ASan.
* Setup atfork handler.
* Improve test harness sanity and re-enable GWP-ASan in Scudo.

Scudo_standalone disables embedded GWP-ASan as necessary around fork().
Standalone GWP-ASan sets the atfork handler in init() if asked to. This
requires a working malloc(), therefore GWP-ASan initialization in Scudo
is delayed to the post-init callback.

Test harness changes are about setting up a single global instance of
the GWP-ASan allocator so that pthread_atfork() does not create
dangling pointers.

Test case shamelessly stolen from D72470.

Reviewers: cryptoad, hctim, jfb

Subscribers: mgorny, jfb, #sanitizers, llvm-commits

Tags: #sanitizers, #llvm

Differential Revision: https://reviews.llvm.org/D73294

compiler-rt/lib/gwp_asan/guarded_pool_allocator.cpp

@@ -58,7 +58,9 @@ void defaultPrintStackTrace(uintptr_t *Trace, size_t TraceLength,
 
 // 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 @@ void GuardedPoolAllocator::init(const options::Options &Opts) {
   // 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<uint32_t>(Opts.SampleRate) * 2;
+    AdjustedSampleRatePlusOne = static_cast<uint32_t>(Opts.SampleRate) * 2 + 1;
   else
-    AdjustedSampleRate = 1;
+    AdjustedSampleRatePlusOne = 2;
 
   GuardedPagePool = reinterpret_cast<uintptr_t>(GuardedPoolMemory);
   GuardedPagePoolEnd =
@@ -169,6 +171,31 @@ void GuardedPoolAllocator::init(const options::Options &Opts) {
   // 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<void *>(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) {

compiler-rt/lib/gwp_asan/guarded_pool_allocator.h

@@ -98,14 +98,22 @@ public:
   // 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 @@ public:
   // is owned by this pool.
   GWP_ASAN_ALWAYS_INLINE bool pointerIsMine(const void *Ptr) const {
     uintptr_t P = reinterpret_cast<uintptr_t>(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 @@ private:
   // 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 @@ private:
   // 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 @@ private:
 
   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 @@ private:
   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 @@ private:
   // 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

compiler-rt/lib/gwp_asan/options.inc

@@ -39,3 +39,7 @@ GWP_ASAN_OPTION(
     "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.")

compiler-rt/lib/gwp_asan/platform_specific/guarded_pool_allocator_posix.cpp

@@ -8,9 +8,10 @@
 
 #include "gwp_asan/guarded_pool_allocator.h"
 
-#include <stdlib.h>
 #include <errno.h>
 #include <signal.h>
+#include <stdlib.h>
+#include <string.h>
 #include <sys/mman.h>
 #include <sys/syscall.h>
 #include <sys/types.h>
@@ -30,6 +31,16 @@ void *GuardedPoolAllocator::mapMemory(size_t Size) const {
   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 @@ size_t GuardedPoolAllocator::getPlatformPageSize() {
 }
 
 struct sigaction PreviousHandler;
+bool SignalHandlerInstalled;
 
 static void sigSegvHandler(int sig, siginfo_t *info, void *ucontext) {
   gwp_asan::GuardedPoolAllocator::reportError(
@@ -78,11 +90,31 @@ static void sigSegvHandler(int sig, siginfo_t *info, void *ucontext) {
   }
 }
 
+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() {

compiler-rt/lib/gwp_asan/tests/CMakeLists.txt

@@ -17,7 +17,9 @@ set(GWP_ASAN_UNITTESTS
   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}

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<gwp_asan::GuardedPoolAllocator *>(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);
+}
+} // namespace

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++;
+}
+} // namespace test
+} // namespace gwp_asan

compiler-rt/lib/gwp_asan/tests/harness.h

@@ -24,20 +24,27 @@ namespace test {
 // `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,9 +63,12 @@ public:
     MaxSimultaneousAllocations = MaxSimultaneousAllocationsArg;
 
     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)
@@ -67,16 +77,19 @@ protected:
 
 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;
 };

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_SOURCES_CXX_WRAPPERS
 
 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()

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 @@ public:
     Quarantine.init(
         static_cast<uptr>(getFlags()->quarantine_size_kb << 10),
         static_cast<uptr>(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 @@ public:
         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 @@ public:
   void unmapTestOnly() {
     TSDRegistry.unmapTestOnly();
     Primary.unmapTestOnly();
+#ifdef GWP_ASAN_HOOKS
+    GuardedAlloc.uninitTestOnly();
+#endif // GWP_ASAN_HOOKS
   }
 
   TSDRegistryT *getTSDRegistry() { return &TSDRegistry; }
@@ -509,6 +514,9 @@ public:
   //                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 @@ public:
     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 @@ private:
     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

compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt

@@ -20,8 +20,7 @@ if(ANDROID)
   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 @@ endforeach()
 
 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()
 

compiler-rt/lib/scudo/standalone/tests/combined_test.cpp

@@ -264,6 +264,17 @@ template <class Config> static void testAllocator() {
   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<scudo::AndroidSvelteConfig>();
@@ -273,6 +284,7 @@ TEST(ScudoCombinedTest, BasicCombined) {
   testAllocator<scudo::DefaultConfig>();
   UseQuarantine = true;
   testAllocator<scudo::AndroidConfig>();
+  testSEGV();
 #endif
 }
 

compiler-rt/lib/scudo/standalone/wrappers_c.inc

@@ -150,6 +150,7 @@ INTERFACE WEAK void SCUDO_PREFIX(malloc_disable)() {
 }
 
 void SCUDO_PREFIX(malloc_postinit)() {
+  SCUDO_ALLOCATOR.initGwpAsan();
   pthread_atfork(SCUDO_PREFIX(malloc_disable), SCUDO_PREFIX(malloc_enable),
                  SCUDO_PREFIX(malloc_enable));
 }