From 1948e4acc62646252ba732278c2699bcaf4fca22 Mon Sep 17 00:00:00 2001 From: Mark Brand Date: Fri, 1 Dec 2023 10:33:05 +0100 Subject: [PATCH 1/3] [ASan] [HWASan] Add __sanitizer_ignore_free_hook() This change adds a new weak API function which makes the sanitizer ignore the call to free(), and implements the functionality in ASan and HWAsan. The runtime that implements this hook can then call free() at a later point again on the same pointer (and making sure the hook returns zero so that the memory will actually be freed) when it's actually ready for the memory to be cleaned up. This is needed in order to implement an sanitizer-compatible version of Chrome's BackupRefPtr algorithm, since process-wide double-shimming of malloc/free does not work on some platforms. Requested and designed by @c01db33f (Mark) from Project Zero. --- .../include/sanitizer/allocator_interface.h | 15 ++- compiler-rt/lib/asan/asan_allocator.cpp | 10 +- compiler-rt/lib/hwasan/hwasan_allocator.cpp | 5 +- .../sanitizer_allocator_interface.h | 2 + .../lib/sanitizer_common/sanitizer_common.cpp | 15 ++- .../lib/sanitizer_common/sanitizer_common.h | 2 +- .../sanitizer_common_interface.inc | 1 + .../asan/TestCases/Posix/ignore_free_hook.cpp | 119 ++++++++++++++++++ .../TestCases/Posix/ignore_free_hook.cpp | 101 +++++++++++++++ .../TestCases/malloc_hook.cpp | 15 +++ 10 files changed, 276 insertions(+), 9 deletions(-) create mode 100644 compiler-rt/test/asan/TestCases/Posix/ignore_free_hook.cpp create mode 100644 compiler-rt/test/hwasan/TestCases/Posix/ignore_free_hook.cpp diff --git a/compiler-rt/include/sanitizer/allocator_interface.h b/compiler-rt/include/sanitizer/allocator_interface.h index a792e9f0136e68..1696a92681e359 100644 --- a/compiler-rt/include/sanitizer/allocator_interface.h +++ b/compiler-rt/include/sanitizer/allocator_interface.h @@ -62,13 +62,20 @@ size_t SANITIZER_CDECL __sanitizer_get_free_bytes(void); size_t SANITIZER_CDECL __sanitizer_get_unmapped_bytes(void); /* Malloc hooks that may be optionally provided by user. - __sanitizer_malloc_hook(ptr, size) is called immediately after - allocation of "size" bytes, which returned "ptr". - __sanitizer_free_hook(ptr) is called immediately before - deallocation of "ptr". */ + - __sanitizer_malloc_hook(ptr, size) is called immediately after allocation + of "size" bytes, which returned "ptr". + - __sanitizer_free_hook(ptr) is called immediately before deallocation of + "ptr". + - __sanitizer_ignore_free_hook(ptr) is called immediately before deallocation + of "ptr", and if it returns a non-zero value, the deallocation of "ptr" + will not take place. This allows software to make free a no-op until it + calls free() again in the same pointer at a later time. Hint: read this as + "ignore the free" rather than "ignore the hook". +*/ void SANITIZER_CDECL __sanitizer_malloc_hook(const volatile void *ptr, size_t size); void SANITIZER_CDECL __sanitizer_free_hook(const volatile void *ptr); +int SANITIZER_CDECL __sanitizer_ignore_free_hook(const volatile void *ptr); /* Installs a pair of hooks for malloc/free. Several (currently, 5) hook pairs may be installed, they are executed diff --git a/compiler-rt/lib/asan/asan_allocator.cpp b/compiler-rt/lib/asan/asan_allocator.cpp index 22dcf6132707b5..9e66f77217ec6b 100644 --- a/compiler-rt/lib/asan/asan_allocator.cpp +++ b/compiler-rt/lib/asan/asan_allocator.cpp @@ -717,7 +717,15 @@ struct Allocator { return; } - RunFreeHooks(ptr); + if (RunFreeHooks(ptr)) { + // Someone used __sanitizer_ignore_free_hook() and decided that they + // didn't want the memory to __sanitizer_ignore_free_hook freed right now. + // When they call free() on this pointer again at a later time, we should + // ignore the alloc-type mismatch and allow them to deallocate the pointer + // through free(), rather than the initial alloc type. + m->alloc_type = FROM_MALLOC; + return; + } // Must mark the chunk as quarantined before any changes to its metadata. // Do not quarantine given chunk if we failed to set CHUNK_QUARANTINE flag. diff --git a/compiler-rt/lib/hwasan/hwasan_allocator.cpp b/compiler-rt/lib/hwasan/hwasan_allocator.cpp index d21ba024a20e12..7771127731de88 100644 --- a/compiler-rt/lib/hwasan/hwasan_allocator.cpp +++ b/compiler-rt/lib/hwasan/hwasan_allocator.cpp @@ -289,6 +289,9 @@ static void HwasanDeallocate(StackTrace *stack, void *tagged_ptr) { CHECK(tagged_ptr); void *untagged_ptr = UntagPtr(tagged_ptr); + if (RunFreeHooks(tagged_ptr)) + return; + if (CheckInvalidFree(stack, untagged_ptr, tagged_ptr)) return; @@ -302,8 +305,6 @@ static void HwasanDeallocate(StackTrace *stack, void *tagged_ptr) { return; } - RunFreeHooks(tagged_ptr); - uptr orig_size = meta->GetRequestedSize(); u32 free_context_id = StackDepotPut(*stack); u32 alloc_context_id = meta->GetAllocStackId(); diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_interface.h b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_interface.h index de2b271fb0ed9a..d1a33a40bae1ca 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_interface.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_interface.h @@ -40,6 +40,8 @@ SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE void __sanitizer_malloc_hook(void *ptr, uptr size); SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE void __sanitizer_free_hook(void *ptr); +SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE int +__sanitizer_ignore_free_hook(void *ptr); SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE void __sanitizer_purge_allocator(); diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_common.cpp index 5efdd864295bec..6cd69a53093e75 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_common.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_common.cpp @@ -347,7 +347,13 @@ void RunMallocHooks(void *ptr, uptr size) { } } -void RunFreeHooks(void *ptr) { +// Returns '1' if the call to free() should be ignored (based on +// __sanitizer_ignore_free_hook), or '0' otherwise. +int RunFreeHooks(void *ptr) { + if (__sanitizer_ignore_free_hook(ptr)) { + return 1; + } + __sanitizer_free_hook(ptr); for (int i = 0; i < kMaxMallocFreeHooks; i++) { auto hook = MFHooks[i].free_hook; @@ -355,6 +361,8 @@ void RunFreeHooks(void *ptr) { break; hook(ptr); } + + return 0; } static int InstallMallocFreeHooks(void (*malloc_hook)(const void *, uptr), @@ -419,4 +427,9 @@ SANITIZER_INTERFACE_WEAK_DEF(void, __sanitizer_free_hook, void *ptr) { (void)ptr; } +SANITIZER_INTERFACE_WEAK_DEF(int, __sanitizer_ignore_free_hook, void *ptr) { + (void)ptr; + return 0; +} + } // extern "C" diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common.h b/compiler-rt/lib/sanitizer_common/sanitizer_common.h index 2d1059140c303d..2428a8cd147947 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_common.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_common.h @@ -177,7 +177,7 @@ 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); +int RunFreeHooks(void *ptr); class ReservedAddressRange { public: diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common_interface.inc b/compiler-rt/lib/sanitizer_common/sanitizer_common_interface.inc index 557207fe62ac6d..d5981a38ff2921 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_common_interface.inc +++ b/compiler-rt/lib/sanitizer_common/sanitizer_common_interface.inc @@ -46,6 +46,7 @@ INTERFACE_FUNCTION(__sanitizer_purge_allocator) INTERFACE_FUNCTION(__sanitizer_print_memory_profile) INTERFACE_WEAK_FUNCTION(__sanitizer_free_hook) INTERFACE_WEAK_FUNCTION(__sanitizer_malloc_hook) +INTERFACE_WEAK_FUNCTION(__sanitizer_ignore_free_hook) // Memintrinsic functions. INTERFACE_FUNCTION(__sanitizer_internal_memcpy) INTERFACE_FUNCTION(__sanitizer_internal_memmove) diff --git a/compiler-rt/test/asan/TestCases/Posix/ignore_free_hook.cpp b/compiler-rt/test/asan/TestCases/Posix/ignore_free_hook.cpp new file mode 100644 index 00000000000000..2ef9feece0f6b0 --- /dev/null +++ b/compiler-rt/test/asan/TestCases/Posix/ignore_free_hook.cpp @@ -0,0 +1,119 @@ +// Check that free hook doesn't conflict with Realloc. +// RUN: %clangxx_asan -O2 %s -o %t -DTEST=basic_hook_works && not %run %t \ +// RUN: |& FileCheck %s -check-prefix=CHECK-BASIC +// RUN: %clangxx_asan -O2 %s -o %t -DTEST=ignore && %run %t \ +// RUN: |& FileCheck %s -check-prefix=CHECK-IGNORE +// RUN: %clangxx_asan -O2 %s -o %t -DTEST=ignore_twice && not %run %t \ +// RUN: |& FileCheck %s -check-prefix=CHECK-IGNORE-2 +// RUN: %clangxx_asan -O2 %s -o %t -DTEST=mismatch && not %run %t \ +// RUN: |& FileCheck %s -check-prefix=CHECK-MISMATCH +// RUN: %clangxx_asan -O2 %s -o %t -DTEST=ignore_mismatch && %run %t \ +// RUN: |& FileCheck %s -check-prefix=CHECK-IGNORE-MISMATCH +// RUN: %clangxx_asan -O2 %s -o %t -DTEST=double_delete && not %run %t \ +// RUN: |& FileCheck %s -check-prefix=CHECK-DOUBLE-DELETE + +#include +#include + +static char *volatile glob_ptr; +bool ignore_free = false; + +#if (__APPLE__) +// Required for dyld macOS 12.0+ +# define WEAK_ON_APPLE __attribute__((weak)) +#else // !(__APPLE__) +# define WEAK_ON_APPLE +#endif // (__APPLE__) + +extern "C" { +WEAK_ON_APPLE void __sanitizer_free_hook(const volatile void *ptr) { + if (ptr == glob_ptr) + fprintf(stderr, "Free Hook\n"); +} + +WEAK_ON_APPLE int __sanitizer_ignore_free_hook(const volatile void *ptr) { + if (ptr != glob_ptr) + return 0; + fprintf(stderr, ignore_free ? "Free Ignored\n" : "Free Respected\n"); + return ignore_free; +} +} // extern "C" + +void allocate() { glob_ptr = reinterpret_cast(malloc(100)); } +void deallocate() { free(reinterpret_cast(glob_ptr)); } + +void basic_hook_works() { + allocate(); + deallocate(); // CHECK-BASIC-NOT: Free Ignored + // CHECK-BASIC: Free Respected + // CHECK-BASIC: Free Hook + *glob_ptr = 0; // CHECK-BASIC: AddressSanitizer: heap-use-after-free +} + +void ignore() { + allocate(); + ignore_free = true; + deallocate(); + // CHECK-IGNORE: Free Ignored + // CHECK-IGNORE-NOT: Free Respected + // CHECK-IGNORE-NOT: Free Hook + // CHECK-IGNORE-NOT: AddressSanitizer + *glob_ptr = 0; +} + +void ignore_twice() { + allocate(); + ignore_free = true; + deallocate(); // CHECK-IGNORE-2: Free Ignored + *glob_ptr = 0; + ignore_free = false; + deallocate(); // CHECK-IGNORE-2-NOT: Free Ignored + // CHECK-IGNORE-2: Free Respected + // CHECK-IGNORE-2: Free Hook + *glob_ptr = 0; // CHECK-IGNORE-2: AddressSanitizer: heap-use-after-free +} + +void ignore_a_lot() { + allocate(); + ignore_free = true; + for (int i = 0; i < 10000; ++i) { + deallocate(); // CHECK-IGNORE-3: Free Ignored + *glob_ptr = 0; + } + ignore_free = false; + deallocate(); // CHECK-IGNORE-3: Free Respected + // CHECK-IGNORE-3: Free Hook + *glob_ptr = 0; // CHECK-IGNORE-3: AddressSanitizer: heap-use-after-free +} + +void mismatch() { + glob_ptr = new char; + deallocate(); // CHECK-MISMATCH: AddressSanitizer: alloc-dealloc-mismatch +} + +void ignore_mismatch() { + glob_ptr = new char; + ignore_free = true; + // Mismatch isn't detected when the free() is ignored. + deallocate(); + deallocate(); + ignore_free = false; + // And also isn't detected when the memory is free()-d for real. + deallocate(); // CHECK-IGNORE-MISMATCH-NOT: AddressSanitizer: alloc-dealloc-mismatch +} + +void double_delete() { + allocate(); + ignore_free = true; + deallocate(); // CHECK-DOUBLE-DELETE: Free Ignored + deallocate(); // CHECK-DOUBLE-DELETE: Free Ignored + ignore_free = false; + deallocate(); // CHECK-DOUBLE-DELETE: Free Respected + // CHECK-DOUBLE-DELETE: Free Hook + deallocate(); // CHECK-DOUBLE-DELETE: AddressSanitizer: attempting double-free +} + +int main() { + TEST(); + return 0; +} diff --git a/compiler-rt/test/hwasan/TestCases/Posix/ignore_free_hook.cpp b/compiler-rt/test/hwasan/TestCases/Posix/ignore_free_hook.cpp new file mode 100644 index 00000000000000..9a0bde24149e78 --- /dev/null +++ b/compiler-rt/test/hwasan/TestCases/Posix/ignore_free_hook.cpp @@ -0,0 +1,101 @@ +// Check that free hook doesn't conflict with Realloc. +// RUN: %clangxx_hwasan -O2 %s -o %t -DTEST=basic_hook_works && not %run %t \ +// RUN: |& FileCheck %s -check-prefix=CHECK-BASIC +// RUN: %clangxx_hwasan -O2 %s -o %t -DTEST=ignore && %run %t \ +// RUN: |& FileCheck %s -check-prefix=CHECK-IGNORE +// RUN: %clangxx_hwasan -O2 %s -o %t -DTEST=ignore_twice && not %run %t \ +// RUN: |& FileCheck %s -check-prefix=CHECK-IGNORE-2 +// RUN: %clangxx_hwasan -O2 %s -o %t -DTEST=double_delete && not %run %t \ +// RUN: |& FileCheck %s -check-prefix=CHECK-DOUBLE-DELETE + +#include +#include +#include + +static char *volatile glob_ptr; +bool ignore_free = false; + +#if (__APPLE__) +// Required for dyld macOS 12.0+ +# define WEAK_ON_APPLE __attribute__((weak)) +#else // !(__APPLE__) +# define WEAK_ON_APPLE +#endif // (__APPLE__) + +extern "C" { +WEAK_ON_APPLE void __sanitizer_free_hook(const volatile void *ptr) { + if (ptr == glob_ptr) + fprintf(stderr, "Free Hook\n"); +} + +WEAK_ON_APPLE int __sanitizer_ignore_free_hook(const volatile void *ptr) { + if (ptr != glob_ptr) + return 0; + fprintf(stderr, ignore_free ? "Free Ignored\n" : "Free Respected\n"); + return ignore_free; +} +} // extern "C" + +void allocate() { glob_ptr = reinterpret_cast(malloc(100)); } +void deallocate() { free(reinterpret_cast(glob_ptr)); } + +void basic_hook_works() { + allocate(); + deallocate(); // CHECK-BASIC-NOT: Free Ignored + // CHECK-BASIC: Free Respected + // CHECK-BASIC: Free Hook + *glob_ptr = 0; // CHECK-BASIC: HWAddressSanitizer: tag-mismatch +} + +void ignore() { + allocate(); + ignore_free = true; + deallocate(); + // CHECK-IGNORE: Free Ignored + // CHECK-IGNORE-NOT: Free Respected + // CHECK-IGNORE-NOT: Free Hook + // CHECK-IGNORE-NOT: HWAddressSanitizer + *glob_ptr = 0; +} + +void ignore_twice() { + allocate(); + ignore_free = true; + deallocate(); // CHECK-IGNORE-2: Free Ignored + *glob_ptr = 0; + ignore_free = false; + deallocate(); // CHECK-IGNORE-2-NOT: Free Ignored + // CHECK-IGNORE-2: Free Respected + // CHECK-IGNORE-2: Free Hook + *glob_ptr = 0; // CHECK-IGNORE-2: HWAddressSanitizer: tag-mismatch +} + +void ignore_a_lot() { + allocate(); + ignore_free = true; + for (int i = 0; i < 10000; ++i) { + deallocate(); // CHECK-IGNORE-3: Free Ignored + *glob_ptr = 0; + } + ignore_free = false; + deallocate(); // CHECK-IGNORE-3: Free Respected + // CHECK-IGNORE-3: Free Hook + *glob_ptr = 0; // CHECK-IGNORE-3: HWAddressSanitizer: tag-mismatch +} + +void double_delete() { + allocate(); + ignore_free = true; + deallocate(); // CHECK-DOUBLE-DELETE: Free Ignored + deallocate(); // CHECK-DOUBLE-DELETE: Free Ignored + ignore_free = false; + deallocate(); // CHECK-DOUBLE-DELETE: Free Respected + // CHECK-DOUBLE-DELETE: Free Hook + deallocate(); // CHECK-DOUBLE-DELETE: HWAddressSanitizer: invalid-free +} + +int main() { + __hwasan_enable_allocator_tagging(); + TEST(); + return 0; +} diff --git a/compiler-rt/test/sanitizer_common/TestCases/malloc_hook.cpp b/compiler-rt/test/sanitizer_common/TestCases/malloc_hook.cpp index 44dffcd47a51aa..285f16ee4ce993 100644 --- a/compiler-rt/test/sanitizer_common/TestCases/malloc_hook.cpp +++ b/compiler-rt/test/sanitizer_common/TestCases/malloc_hook.cpp @@ -12,6 +12,7 @@ extern "C" { const volatile void *global_ptr; +int ignore_free = 0; #define WRITE(s) write(1, s, sizeof(s)) @@ -27,6 +28,13 @@ void __sanitizer_free_hook(const volatile void *ptr) { if (__sanitizer_get_ownership(ptr) && ptr == global_ptr) WRITE("FreeHook\n"); } +int __sanitizer_ignore_free_hook(const volatile void *ptr) { + if (__sanitizer_get_ownership(ptr) && ignore_free) { + WRITE("IgnoreFreeHook\n"); + return 1; + } + return 0; +} } // extern "C" volatile int *x; @@ -64,6 +72,13 @@ int main() { x[0] = 0; x[127] = -1; + ignore_free = 1; + free((void *)x); + // CHECK: IgnoreFreeHook + + // Check that the memory is still valid now and hasn't been poisoned. + x[0] = 99; + ignore_free = 0; free((void *)x); // CHECK: FH1 // CHECK: FH2 From 38a366e9dcb29132ee4e921dff672996ac6a4671 Mon Sep 17 00:00:00 2001 From: Mitch Phillips Date: Wed, 26 Jun 2024 13:17:57 +0200 Subject: [PATCH 2/3] Don't test the hooks in sanitizer_common. These hooks are not implemented for all sanitizers, and so testing is individual for ASan/HWASan. --- .../sanitizer_common/TestCases/malloc_hook.cpp | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/compiler-rt/test/sanitizer_common/TestCases/malloc_hook.cpp b/compiler-rt/test/sanitizer_common/TestCases/malloc_hook.cpp index 285f16ee4ce993..44dffcd47a51aa 100644 --- a/compiler-rt/test/sanitizer_common/TestCases/malloc_hook.cpp +++ b/compiler-rt/test/sanitizer_common/TestCases/malloc_hook.cpp @@ -12,7 +12,6 @@ extern "C" { const volatile void *global_ptr; -int ignore_free = 0; #define WRITE(s) write(1, s, sizeof(s)) @@ -28,13 +27,6 @@ void __sanitizer_free_hook(const volatile void *ptr) { if (__sanitizer_get_ownership(ptr) && ptr == global_ptr) WRITE("FreeHook\n"); } -int __sanitizer_ignore_free_hook(const volatile void *ptr) { - if (__sanitizer_get_ownership(ptr) && ignore_free) { - WRITE("IgnoreFreeHook\n"); - return 1; - } - return 0; -} } // extern "C" volatile int *x; @@ -72,13 +64,6 @@ int main() { x[0] = 0; x[127] = -1; - ignore_free = 1; - free((void *)x); - // CHECK: IgnoreFreeHook - - // Check that the memory is still valid now and hasn't been poisoned. - x[0] = 99; - ignore_free = 0; free((void *)x); // CHECK: FH1 // CHECK: FH2 From d9b67744632a395b7330e1518eee5325c198dba9 Mon Sep 17 00:00:00 2001 From: Mitch Phillips Date: Fri, 12 Jul 2024 13:40:21 +0200 Subject: [PATCH 3/3] Remove bad comment --- compiler-rt/test/asan/TestCases/Posix/ignore_free_hook.cpp | 1 - compiler-rt/test/hwasan/TestCases/Posix/ignore_free_hook.cpp | 1 - 2 files changed, 2 deletions(-) diff --git a/compiler-rt/test/asan/TestCases/Posix/ignore_free_hook.cpp b/compiler-rt/test/asan/TestCases/Posix/ignore_free_hook.cpp index 2ef9feece0f6b0..1017d63772bab2 100644 --- a/compiler-rt/test/asan/TestCases/Posix/ignore_free_hook.cpp +++ b/compiler-rt/test/asan/TestCases/Posix/ignore_free_hook.cpp @@ -1,4 +1,3 @@ -// Check that free hook doesn't conflict with Realloc. // RUN: %clangxx_asan -O2 %s -o %t -DTEST=basic_hook_works && not %run %t \ // RUN: |& FileCheck %s -check-prefix=CHECK-BASIC // RUN: %clangxx_asan -O2 %s -o %t -DTEST=ignore && %run %t \ diff --git a/compiler-rt/test/hwasan/TestCases/Posix/ignore_free_hook.cpp b/compiler-rt/test/hwasan/TestCases/Posix/ignore_free_hook.cpp index 9a0bde24149e78..b6f17d89083b49 100644 --- a/compiler-rt/test/hwasan/TestCases/Posix/ignore_free_hook.cpp +++ b/compiler-rt/test/hwasan/TestCases/Posix/ignore_free_hook.cpp @@ -1,4 +1,3 @@ -// Check that free hook doesn't conflict with Realloc. // RUN: %clangxx_hwasan -O2 %s -o %t -DTEST=basic_hook_works && not %run %t \ // RUN: |& FileCheck %s -check-prefix=CHECK-BASIC // RUN: %clangxx_hwasan -O2 %s -o %t -DTEST=ignore && %run %t \