From 9cad8424ff7b0fa63b53cb9919eae31539b8561a Mon Sep 17 00:00:00 2001 From: Mitch Phillips Date: Wed, 20 Jan 2021 16:03:27 -0800 Subject: [PATCH] [MemInit] Remove old API, introduce new MemInit API. Introduces new heap-zero-init API. We've realised that it's better to be able to individually control MTE and heap zero-init. Having heap-zero-init not be controllable without affecting MTE affects our ability to turn off heap-zero-init in zygote-forked applications. Bug: 135772972 Test: On FVP: atest -s localhost:5555 malloc#zero_init \ Test: malloc#disable_mte heap_tagging_level Change-Id: I8c6722502733259934c699f4f1269eaf1641a09f --- libc/Android.bp | 2 +- ...itigation_state.cpp => heap_zero_init.cpp} | 39 ++--------- ...ry_mitigation_state.h => heap_zero_init.h} | 6 +- libc/bionic/malloc_common.cpp | 6 +- libc/bionic/malloc_common_dynamic.cpp | 2 +- libc/include/malloc.h | 17 ++--- tests/malloc_test.cpp | 66 ++++++++++++++++++- 7 files changed, 86 insertions(+), 52 deletions(-) rename libc/bionic/{memory_mitigation_state.cpp => heap_zero_init.cpp} (65%) rename libc/bionic/{memory_mitigation_state.h => heap_zero_init.h} (90%) diff --git a/libc/Android.bp b/libc/Android.bp index 8f07ef431..b1f9c3f1e 100644 --- a/libc/Android.bp +++ b/libc/Android.bp @@ -1060,6 +1060,7 @@ cc_library_static { "bionic/get_device_api_level.cpp", "bionic/grp_pwd.cpp", "bionic/grp_pwd_file.cpp", + "bionic/heap_zero_init.cpp", "bionic/iconv.cpp", "bionic/icu_wrappers.cpp", "bionic/ifaddrs.cpp", @@ -1078,7 +1079,6 @@ cc_library_static { "bionic/mblen.cpp", "bionic/mbrtoc16.cpp", "bionic/mbrtoc32.cpp", - "bionic/memory_mitigation_state.cpp", "bionic/mempcpy.cpp", "bionic/mkdir.cpp", "bionic/mkfifo.cpp", diff --git a/libc/bionic/memory_mitigation_state.cpp b/libc/bionic/heap_zero_init.cpp similarity index 65% rename from libc/bionic/memory_mitigation_state.cpp rename to libc/bionic/heap_zero_init.cpp index b262f8616..f5a05e57b 100644 --- a/libc/bionic/memory_mitigation_state.cpp +++ b/libc/bionic/heap_zero_init.cpp @@ -26,43 +26,14 @@ * SUCH DAMAGE. */ -#include "memory_mitigation_state.h" - -#include -#include -#include -#include -#include -#include -#include - -#include -#include - -#include -#include - -#include "heap_tagging.h" -#include "pthread_internal.h" +#include "heap_zero_init.h" extern "C" void scudo_malloc_set_zero_contents(int zero_contents); -bool DisableMemoryMitigations(int arg) { - if (arg != 0) { - return false; - } - +bool SetHeapZeroInitialize(bool zero_init __attribute__((__unused__))) { #ifdef USE_SCUDO - scudo_malloc_set_zero_contents(0); -#endif - - ScopedPthreadMutexLocker locker(&g_heap_tagging_lock); - - HeapTaggingLevel current_level = GetHeapTaggingLevel(); - if (current_level != M_HEAP_TAGGING_LEVEL_NONE && current_level != M_HEAP_TAGGING_LEVEL_TBI) { - HeapTaggingLevel level = M_HEAP_TAGGING_LEVEL_NONE; - SetHeapTaggingLevel(level); - } - + scudo_malloc_set_zero_contents(zero_init); return true; +#endif + return false; } diff --git a/libc/bionic/memory_mitigation_state.h b/libc/bionic/heap_zero_init.h similarity index 90% rename from libc/bionic/memory_mitigation_state.h rename to libc/bionic/heap_zero_init.h index 047b6ad3a..f78e01fa7 100644 --- a/libc/bionic/memory_mitigation_state.h +++ b/libc/bionic/heap_zero_init.h @@ -28,6 +28,6 @@ #pragma once -#include - -bool DisableMemoryMitigations(int arg); +// Sets heap zero initialization to on (true) or off (false). Returns false on +// failure, true otherwise. +bool SetHeapZeroInitialize(bool zero_init); diff --git a/libc/bionic/malloc_common.cpp b/libc/bionic/malloc_common.cpp index 863103deb..c91efa0f0 100644 --- a/libc/bionic/malloc_common.cpp +++ b/libc/bionic/malloc_common.cpp @@ -44,10 +44,10 @@ #include "gwp_asan_wrappers.h" #include "heap_tagging.h" +#include "heap_zero_init.h" #include "malloc_common.h" #include "malloc_limit.h" #include "malloc_tagged_pointers.h" -#include "memory_mitigation_state.h" // ============================================================================= // Global variables instantations. @@ -107,8 +107,8 @@ extern "C" int mallopt(int param, int value) { ScopedPthreadMutexLocker locker(&g_heap_tagging_lock); return SetHeapTaggingLevel(static_cast(value)); } - if (param == M_BIONIC_DISABLE_MEMORY_MITIGATIONS) { - return DisableMemoryMitigations(value); + if (param == M_BIONIC_ZERO_INIT) { + return SetHeapZeroInitialize(value); } // The rest we pass on... auto dispatch_table = GetDispatchTable(); diff --git a/libc/bionic/malloc_common_dynamic.cpp b/libc/bionic/malloc_common_dynamic.cpp index 7a221d891..3a6958cde 100644 --- a/libc/bionic/malloc_common_dynamic.cpp +++ b/libc/bionic/malloc_common_dynamic.cpp @@ -67,11 +67,11 @@ #include "gwp_asan_wrappers.h" #include "heap_tagging.h" +#include "heap_zero_init.h" #include "malloc_common.h" #include "malloc_common_dynamic.h" #include "malloc_heapprofd.h" #include "malloc_limit.h" -#include "memory_mitigation_state.h" // ============================================================================= // Global variables instantations. diff --git a/libc/include/malloc.h b/libc/include/malloc.h index 598a50bc5..bae1f6823 100644 --- a/libc/include/malloc.h +++ b/libc/include/malloc.h @@ -204,18 +204,19 @@ int malloc_info(int __must_be_zero, FILE* __fp) __INTRODUCED_IN(23); #define M_TSDS_COUNT_MAX (-202) /** - * mallopt() option to disable heap initialization across the whole process. - * If the hardware supports memory tagging, this also disables memory tagging. - * May be called at any time, including when multiple threads are running. The - * value is unused but must be set to 0. + * mallopt() option to decide whether heap memory is zero-initialized on + * allocation across the whole process. May be called at any time, including + * when multiple threads are running. An argument of zero indicates memory + * should not be zero-initialized, any other value indicates to initialize heap + * memory to zero. * - * Note that these memory mitigations are only implemented in scudo and - * therefore this will have no effect when using another allocator (such as - * jemalloc on Android Go devices). + * Note that this memory mitigations is only implemented in scudo and therefore + * this will have no effect when using another allocator (such as jemalloc on + * Android Go devices). * * Available since API level 31. */ -#define M_BIONIC_DISABLE_MEMORY_MITIGATIONS (-203) +#define M_BIONIC_ZERO_INIT (-203) /** * mallopt() option to change the heap tagging state. May be called at any diff --git a/tests/malloc_test.cpp b/tests/malloc_test.cpp index b35bba983..3a09258f7 100644 --- a/tests/malloc_test.cpp +++ b/tests/malloc_test.cpp @@ -32,8 +32,10 @@ #include #include +#include #include #include +#include #include @@ -1256,7 +1258,67 @@ TEST(android_mallopt, set_allocation_limit_multiple_threads) { #endif } -TEST(malloc, disable_memory_mitigations) { +void TestHeapZeroing(int num_iterations, int (*get_alloc_size)(int iteration)) { + std::vector allocs; + constexpr int kMaxBytesToCheckZero = 64; + const char kBlankMemory[kMaxBytesToCheckZero] = {}; + + for (int i = 0; i < num_iterations; ++i) { + int size = get_alloc_size(i); + allocs.push_back(malloc(size)); + memset(allocs.back(), 'X', std::min(size, kMaxBytesToCheckZero)); + } + + for (void* alloc : allocs) { + free(alloc); + } + allocs.clear(); + + for (int i = 0; i < num_iterations; ++i) { + int size = get_alloc_size(i); + allocs.push_back(malloc(size)); + ASSERT_EQ(0, memcmp(allocs.back(), kBlankMemory, std::min(size, kMaxBytesToCheckZero))); + } + + for (void* alloc : allocs) { + free(alloc); + } +} + +TEST(malloc, zero_init) { +#if defined(__BIONIC__) + SKIP_WITH_HWASAN << "hwasan does not implement mallopt"; + bool allocator_scudo; + GetAllocatorVersion(&allocator_scudo); + if (!allocator_scudo) { + GTEST_SKIP() << "scudo allocator only test"; + } + + mallopt(M_BIONIC_ZERO_INIT, 1); + + // Test using a block of 4K small (1-32 byte) allocations. + TestHeapZeroing(/* num_iterations */ 0x1000, [](int iteration) -> int { + return 1 + iteration % 32; + }); + + // Also test large allocations that land in the scudo secondary, as this is + // the only part of Scudo that's changed by enabling zero initialization with + // MTE. Uses 32 allocations, totalling 60MiB memory. Decay time (time to + // release secondary allocations back to the OS) was modified to 0ms/1ms by + // mallopt_decay. Ensure that we delay for at least a second before releasing + // pages to the OS in order to avoid implicit zeroing by the kernel. + mallopt(M_DECAY_TIME, 1000); + TestHeapZeroing(/* num_iterations */ 32, [](int iteration) -> int { + return 1 << (19 + iteration % 4); + }); + +#else + GTEST_SKIP() << "bionic-only test"; +#endif +} + +// Note that MTE is enabled on cc_tests on devices that support MTE. +TEST(malloc, disable_mte) { #if defined(__BIONIC__) if (!mte_supported()) { GTEST_SKIP() << "This function can only be tested with MTE"; @@ -1275,7 +1337,7 @@ TEST(malloc, disable_memory_mitigations) { }, &sem)); - ASSERT_EQ(1, mallopt(M_BIONIC_DISABLE_MEMORY_MITIGATIONS, 0)); + ASSERT_EQ(1, mallopt(M_BIONIC_SET_HEAP_TAGGING_LEVEL, M_HEAP_TAGGING_LEVEL_NONE)); ASSERT_EQ(0, sem_post(&sem)); int my_tagged_addr_ctrl = prctl(PR_GET_TAGGED_ADDR_CTRL, 0, 0, 0, 0);