[GWP-ASan] [heapprofd] Use ephemeral dispatch table when death prof.
GWP-ASan + heapprofd don't currently play nice together in some
circumstances. heapprofd thinks it's still an only child, and refuses to
accept the existence of its little brother, GWP-ASan.
If GWP-ASan is installed before heapprofd, then heapprofd is *required*
to respect that libc has a favourite child. If an allocation/free is passed
to heapprofd, then heapprofd *must* (eventually) pass that allocation/free to
GWP-ASan. If heapprofd doesn't do this, then a free() of a GWP-ASan
allocation can be passed to the system allocator.
This can happen in two places right now:
1. The heapprofd hooks simply clobber any trace of what was
previously in the default_dispatch_table when enabled through the
heapprofd signal.
2. Heapprofd can die when the system is under significant pressure.
Some pipes can timeout, which ends up in the client calling ShutdownLazy()
-> mallopt(M_RESET_HOOKS) -> DispatchReset(). This also clobbers any
trace of the previous default_dispatch_table.
To fix both these problems, we fix heapprofd to restore the previous
default_dispatch_table whenever either circumstance happens. We do some
tricky copying to avoid race conditions on the malloc_dispatch_table in
fixing #1.
Bug: 135634846
Test: Run HeapprofdEndToEnd.NativeProfilingActiveAtProcessExit/ForkMode
a significant number of times with large amounts of system pressure (I
just run bionic-unit-tests-scudo in parallel). You will see some test
failures where heapprofd died due to system pressure, but never a death
from the allocator. Tests should never fail when the system isn't under
immense pressure.
Change-Id: I20ab340d4bdc35d6d1012da5ee1a25634428d097
diff --git a/libc/bionic/gwp_asan_wrappers.cpp b/libc/bionic/gwp_asan_wrappers.cpp
index 1feb871..2eb6a7e 100644
--- a/libc/bionic/gwp_asan_wrappers.cpp
+++ b/libc/bionic/gwp_asan_wrappers.cpp
@@ -259,3 +259,7 @@
return true;
}
+
+bool DispatchIsGwpAsan(const MallocDispatch* dispatch) {
+ return dispatch == &gwp_asan_dispatch;
+}
diff --git a/libc/bionic/gwp_asan_wrappers.h b/libc/bionic/gwp_asan_wrappers.h
index fd9c547..a39d50b 100644
--- a/libc/bionic/gwp_asan_wrappers.h
+++ b/libc/bionic/gwp_asan_wrappers.h
@@ -37,3 +37,8 @@
// Maybe initialize GWP-ASan. Set force_init to true to bypass process sampling.
bool MaybeInitGwpAsan(libc_globals* globals, bool force_init = false);
+
+// Returns whether GWP-ASan is the provided dispatch table pointer. Used in
+// heapprofd's signal-initialization sequence to determine the intermediate
+// dispatch pointer to use when initing.
+bool DispatchIsGwpAsan(const MallocDispatch* dispatch);
diff --git a/libc/bionic/malloc_heapprofd.cpp b/libc/bionic/malloc_heapprofd.cpp
index bf4c63a..198d2f0 100644
--- a/libc/bionic/malloc_heapprofd.cpp
+++ b/libc/bionic/malloc_heapprofd.cpp
@@ -42,6 +42,7 @@
#include <private/bionic_malloc_dispatch.h>
#include <sys/system_properties.h>
+#include "gwp_asan_wrappers.h"
#include "malloc_common.h"
#include "malloc_common_dynamic.h"
#include "malloc_heapprofd.h"
@@ -86,30 +87,6 @@
extern "C" void* MallocInitHeapprofdHook(size_t);
-static constexpr MallocDispatch __heapprofd_init_dispatch
- __attribute__((unused)) = {
- Malloc(calloc),
- Malloc(free),
- Malloc(mallinfo),
- MallocInitHeapprofdHook, // malloc replacement
- Malloc(malloc_usable_size),
- Malloc(memalign),
- Malloc(posix_memalign),
-#if defined(HAVE_DEPRECATED_MALLOC_FUNCS)
- Malloc(pvalloc),
-#endif
- Malloc(realloc),
-#if defined(HAVE_DEPRECATED_MALLOC_FUNCS)
- Malloc(valloc),
-#endif
- Malloc(malloc_iterate),
- Malloc(malloc_disable),
- Malloc(malloc_enable),
- Malloc(mallopt),
- Malloc(aligned_alloc),
- Malloc(malloc_info),
- };
-
constexpr char kHeapprofdProgramPropertyPrefix[] = "heapprofd.enable.";
constexpr size_t kHeapprofdProgramPropertyPrefixSize = sizeof(kHeapprofdProgramPropertyPrefix) - 1;
constexpr size_t kMaxCmdlineSize = 512;
@@ -176,6 +153,15 @@
// is loaded synchronously).
// In both cases, the caller is responsible for verifying that the process is
// considered profileable.
+
+// Previously installed default dispatch table, if it exists. This is used to
+// load heapprofd properly when GWP-ASan was already installed. If GWP-ASan was
+// already installed, heapprofd will take over the dispatch table, but will use
+// GWP-ASan as the backing dispatch. This variable is atomically protected by
+// gHeapprofdInitInProgress.
+static const MallocDispatch* gPreviousDefaultDispatchTable = nullptr;
+static MallocDispatch gEphemeralDispatch;
+
void HandleHeapprofdSignal() {
if (atomic_load_explicit(&gHeapprofdIncompatibleHooks, memory_order_acquire)) {
error_log("%s: not enabling heapprofd, malloc_debug/malloc_hooks are enabled.", getprogname());
@@ -187,11 +173,29 @@
// not ever have a conflict modifying the globals.
if (!atomic_exchange(&gGlobalsMutating, true)) {
if (!atomic_exchange(&gHeapprofdInitInProgress, true)) {
+ // If the backing dispatch is GWP-ASan, we should use GWP-ASan as the
+ // intermediate dispatch table during initialisation. It may be possible
+ // at this point in time that heapprofd is *already* the default dispatch,
+ // and as such we don't want to use heapprofd as the backing store
+ // (otherwise infinite recursion occurs).
+ gPreviousDefaultDispatchTable = nullptr;
+ const MallocDispatch* default_dispatch = GetDefaultDispatchTable();
+ if (DispatchIsGwpAsan(default_dispatch)) {
+ gPreviousDefaultDispatchTable = default_dispatch;
+ }
+
__libc_globals.mutate([](libc_globals* globals) {
- atomic_store(&globals->default_dispatch_table, &__heapprofd_init_dispatch);
- auto dispatch_table = GetDispatchTable();
- if (!MallocLimitInstalled() || dispatch_table == &globals->malloc_dispatch_table) {
- atomic_store(&globals->current_dispatch_table, &__heapprofd_init_dispatch);
+ // Wholesale copy the malloc dispatch table here. If the current/default
+ // dispatch table is pointing to the malloc_dispatch_table, we can't
+ // modify it as it may be racy. This dispatch table copy is ephemeral,
+ // and the dispatch tables will be resolved back to the global
+ // malloc_dispatch_table after initialization finishes.
+ gEphemeralDispatch = globals->malloc_dispatch_table;
+ gEphemeralDispatch.malloc = MallocInitHeapprofdHook;
+
+ atomic_store(&globals->default_dispatch_table, &gEphemeralDispatch);
+ if (!MallocLimitInstalled()) {
+ atomic_store(&globals->current_dispatch_table, &gEphemeralDispatch);
}
});
}
@@ -241,6 +245,12 @@
return;
}
+ // Before we set the new default_dispatch_table in FinishInstallHooks, save
+ // the previous dispatch table. If DispatchReset() gets called later, we want
+ // to be able to restore the dispatch. We're still under
+ // gHeapprofdInitInProgress locks at this point.
+ gPreviousDefaultDispatchTable = GetDefaultDispatchTable();
+
if (FinishInstallHooks(globals, nullptr, kHeapprofdPrefix)) {
atomic_store(&gHeapprofdHandle, impl_handle);
} else if (!reusing_handle) {
@@ -274,10 +284,9 @@
if (!atomic_exchange(&gHeapprofdInitHookInstalled, true)) {
pthread_mutex_lock(&gGlobalsMutateLock);
__libc_globals.mutate([](libc_globals* globals) {
- auto old_dispatch = GetDefaultDispatchTable();
- atomic_store(&globals->default_dispatch_table, nullptr);
- if (GetDispatchTable() == old_dispatch) {
- atomic_store(&globals->current_dispatch_table, nullptr);
+ atomic_store(&globals->default_dispatch_table, gPreviousDefaultDispatchTable);
+ if (!MallocLimitInstalled()) {
+ atomic_store(&globals->current_dispatch_table, gPreviousDefaultDispatchTable);
}
});
pthread_mutex_unlock(&gGlobalsMutateLock);
@@ -292,7 +301,10 @@
error_log("%s: heapprod: failed to pthread_setname_np", getprogname());
}
}
- return Malloc(malloc)(bytes);
+ // Get an allocation from libc malloc. If we had a previous dispatch table,
+ // this will come from it - otherwise, we'll get it from the system
+ // allocator.
+ return malloc(bytes);
}
bool HeapprofdInitZygoteChildProfiling() {
@@ -309,10 +321,9 @@
if (!atomic_exchange(&gHeapprofdInitInProgress, true)) {
pthread_mutex_lock(&gGlobalsMutateLock);
__libc_globals.mutate([](libc_globals* globals) {
- auto old_dispatch = GetDefaultDispatchTable();
- atomic_store(&globals->default_dispatch_table, nullptr);
- if (GetDispatchTable() == old_dispatch) {
- atomic_store(&globals->current_dispatch_table, nullptr);
+ atomic_store(&globals->default_dispatch_table, gPreviousDefaultDispatchTable);
+ if (!MallocLimitInstalled()) {
+ atomic_store(&globals->current_dispatch_table, gPreviousDefaultDispatchTable);
}
});
pthread_mutex_unlock(&gGlobalsMutateLock);