diff --git a/compiler-rt/lib/scudo/standalone/combined.h b/compiler-rt/lib/scudo/standalone/combined.h index 25c597199a658..72e5caa026e4d 100644 --- a/compiler-rt/lib/scudo/standalone/combined.h +++ b/compiler-rt/lib/scudo/standalone/combined.h @@ -827,10 +827,15 @@ class Allocator { // for it, which then forces realloc to copy the usable size of a chunk as // opposed to its actual size. uptr getUsableSize(const void *Ptr) { - initThreadMaybe(); if (UNLIKELY(!Ptr)) return 0; + return getAllocSize(Ptr); + } + + uptr getAllocSize(const void *Ptr) { + initThreadMaybe(); + #ifdef GWP_ASAN_HOOKS if (UNLIKELY(GuardedAlloc.pointerIsMine(Ptr))) return GuardedAlloc.getSize(Ptr); @@ -839,9 +844,11 @@ class Allocator { Ptr = getHeaderTaggedPointer(const_cast(Ptr)); Chunk::UnpackedHeader Header; Chunk::loadHeader(Cookie, Ptr, &Header); - // Getting the usable size of a chunk only makes sense if it's allocated. + + // Getting the alloc size of a chunk only makes sense if it's allocated. if (UNLIKELY(Header.State != Chunk::State::Allocated)) reportInvalidChunkState(AllocatorAction::Sizing, const_cast(Ptr)); + return getSize(Ptr, &Header); } diff --git a/compiler-rt/lib/scudo/standalone/include/scudo/interface.h b/compiler-rt/lib/scudo/standalone/include/scudo/interface.h index 6c0c521f8d82f..260f1a7bd84bb 100644 --- a/compiler-rt/lib/scudo/standalone/include/scudo/interface.h +++ b/compiler-rt/lib/scudo/standalone/include/scudo/interface.h @@ -17,7 +17,6 @@ extern "C" { __attribute__((weak)) const char *__scudo_default_options(void); // Post-allocation & pre-deallocation hooks. -// They must be thread-safe and not use heap related functions. __attribute__((weak)) void __scudo_allocate_hook(void *ptr, size_t size); __attribute__((weak)) void __scudo_deallocate_hook(void *ptr); diff --git a/compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp b/compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp index 623550535b6c8..150688b5b70a5 100644 --- a/compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp +++ b/compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp @@ -83,9 +83,12 @@ class ScudoWrappersCTest : public Test { printf("Hooks are enabled but hooks tests are disabled.\n"); } - void invalidateAllocHookPtrAs(UNUSED void *Ptr) { - if (SCUDO_ENABLE_HOOKS_TESTS) - AC.Ptr = Ptr; + void invalidateHookPtrs() { + if (SCUDO_ENABLE_HOOKS_TESTS) { + void *InvalidPtr = reinterpret_cast(0xdeadbeef); + AC.Ptr = InvalidPtr; + DC.Ptr = InvalidPtr; + } } void verifyAllocHookPtr(UNUSED void *Ptr) { if (SCUDO_ENABLE_HOOKS_TESTS) @@ -258,6 +261,7 @@ TEST_F(ScudoWrappersCTest, AlignedAlloc) { } TEST_F(ScudoWrappersCDeathTest, Realloc) { + invalidateHookPtrs(); // realloc(nullptr, N) is malloc(N) void *P = realloc(nullptr, Size); EXPECT_NE(P, nullptr); @@ -266,6 +270,7 @@ TEST_F(ScudoWrappersCDeathTest, Realloc) { free(P); verifyDeallocHookPtr(P); + invalidateHookPtrs(); P = malloc(Size); EXPECT_NE(P, nullptr); // realloc(P, 0U) is free(P) and returns nullptr @@ -277,7 +282,7 @@ TEST_F(ScudoWrappersCDeathTest, Realloc) { EXPECT_LE(Size, malloc_usable_size(P)); memset(P, 0x42, Size); - invalidateAllocHookPtrAs(reinterpret_cast(0xdeadbeef)); + invalidateHookPtrs(); void *OldP = P; P = realloc(P, Size * 2U); EXPECT_NE(P, nullptr); @@ -285,14 +290,15 @@ TEST_F(ScudoWrappersCDeathTest, Realloc) { for (size_t I = 0; I < Size; I++) EXPECT_EQ(0x42, (reinterpret_cast(P))[I]); if (OldP == P) { - verifyAllocHookPtr(reinterpret_cast(0xdeadbeef)); + verifyDeallocHookPtr(OldP); + verifyAllocHookPtr(OldP); } else { verifyAllocHookPtr(P); verifyAllocHookSize(Size * 2U); verifyDeallocHookPtr(OldP); } - invalidateAllocHookPtrAs(reinterpret_cast(0xdeadbeef)); + invalidateHookPtrs(); OldP = P; P = realloc(P, Size / 2U); EXPECT_NE(P, nullptr); @@ -300,7 +306,8 @@ TEST_F(ScudoWrappersCDeathTest, Realloc) { for (size_t I = 0; I < Size / 2U; I++) EXPECT_EQ(0x42, (reinterpret_cast(P))[I]); if (OldP == P) { - verifyAllocHookPtr(reinterpret_cast(0xdeadbeef)); + verifyDeallocHookPtr(OldP); + verifyAllocHookPtr(OldP); } else { verifyAllocHookPtr(P); verifyAllocHookSize(Size / 2U); diff --git a/compiler-rt/lib/scudo/standalone/wrappers_c.inc b/compiler-rt/lib/scudo/standalone/wrappers_c.inc index e213da73393b5..0413ea49eac08 100644 --- a/compiler-rt/lib/scudo/standalone/wrappers_c.inc +++ b/compiler-rt/lib/scudo/standalone/wrappers_c.inc @@ -175,10 +175,24 @@ INTERFACE WEAK void *SCUDO_PREFIX(realloc)(void *ptr, size_t size) { return nullptr; } + // Given that the reporting of deallocation and allocation are not atomic, we + // always pretend the old pointer will be released so that the user doesn't + // need to worry about the false double-use case from the view of hooks. + // + // For example, assume that `realloc` releases the old pointer and allocates a + // new pointer. Before the reporting of both operations has been done, another + // thread may get the old pointer from `malloc`. It may be misinterpreted as + // double-use if it's not handled properly on the hook side. + reportDeallocation(ptr); void *NewPtr = SCUDO_ALLOCATOR.reallocate(ptr, size, SCUDO_MALLOC_ALIGNMENT); - if (NewPtr != ptr) { + if (NewPtr != nullptr) { + // Note that even if NewPtr == ptr, the size has changed. We still need to + // report the new size. reportAllocation(NewPtr, size); - reportDeallocation(ptr); + } else { + // If `realloc` fails, the old pointer is not released. Report the old + // pointer as allocated back. + reportAllocation(ptr, SCUDO_ALLOCATOR.getAllocSize(ptr)); } return scudo::setErrnoOnNull(NewPtr);