[compiler-rt] [scudo] Add hooks to mark the range of realloc (PR #74353)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: None (ChiaHungDuan)

<details>
<summary>Changes</summary>

`realloc` may involve both allocation and deallocation. Given that the reporting the events is not atomic and which may lead the hook user to a false case that the double-use pattern happens. In general, this can be resolved on the hook side. To alleviate the task of handling it, we add two new hooks to mark the range and reorder the reporting.

---
Full diff: https://github.com/llvm/llvm-project/pull/74353.diff


3 Files Affected:

- (modified) compiler-rt/lib/scudo/standalone/include/scudo/interface.h (+7) 
- (modified) compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp (+22) 
- (modified) compiler-rt/lib/scudo/standalone/wrappers_c.inc (+39-19) 


``````````diff
diff --git a/compiler-rt/lib/scudo/standalone/include/scudo/interface.h b/compiler-rt/lib/scudo/standalone/include/scudo/interface.h
index 260f1a7bd84bb..9ac900b96533e 100644
--- a/compiler-rt/lib/scudo/standalone/include/scudo/interface.h
+++ b/compiler-rt/lib/scudo/standalone/include/scudo/interface.h
@@ -20,6 +20,13 @@ __attribute__((weak)) const char *__scudo_default_options(void);
 __attribute__((weak)) void __scudo_allocate_hook(void *ptr, size_t size);
 __attribute__((weak)) void __scudo_deallocate_hook(void *ptr);
 
+// These hooks are used to mark the scope of doing realloc(). Note that the
+// allocation/deallocation are still reported through the hooks above, this is
+// only used when a hook user wants to know that the allocation/deallocation
+// operations are a single realloc operation.
+__attribute__((weak)) void __scudo_realloc_begin_hook(void *old_ptr);
+__attribute__((weak)) void __scudo_realloc_end_hook(void *old_ptr);
+
 void __scudo_print_stats(void);
 
 typedef void (*iterate_callback)(uintptr_t base, size_t size, void *arg);
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 150688b5b70a5..c6ad40c8cc2a3 100644
--- a/compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp
@@ -61,8 +61,13 @@ struct AllocContext {
 struct DeallocContext {
   void *Ptr;
 };
+struct ReallocContext {
+  void *Begin;
+  void *End;
+};
 static AllocContext AC;
 static DeallocContext DC;
+static ReallocContext RC;
 
 #if (SCUDO_ENABLE_HOOKS_TESTS == 1)
 __attribute__((visibility("default"))) void __scudo_allocate_hook(void *Ptr,
@@ -73,6 +78,14 @@ __attribute__((visibility("default"))) void __scudo_allocate_hook(void *Ptr,
 __attribute__((visibility("default"))) void __scudo_deallocate_hook(void *Ptr) {
   DC.Ptr = Ptr;
 }
+__attribute__((visibility("default"))) void
+__scudo_realloc_begin_hook(void *Ptr) {
+  RC.Begin = Ptr;
+}
+__attribute__((visibility("default"))) void
+__scudo_realloc_end_hook(void *Ptr) {
+  RC.End = Ptr;
+}
 #endif // (SCUDO_ENABLE_HOOKS_TESTS == 1)
 }
 
@@ -88,6 +101,7 @@ class ScudoWrappersCTest : public Test {
       void *InvalidPtr = reinterpret_cast<void *>(0xdeadbeef);
       AC.Ptr = InvalidPtr;
       DC.Ptr = InvalidPtr;
+      RC.Begin = RC.End = InvalidPtr;
     }
   }
   void verifyAllocHookPtr(UNUSED void *Ptr) {
@@ -102,6 +116,12 @@ class ScudoWrappersCTest : public Test {
     if (SCUDO_ENABLE_HOOKS_TESTS)
       EXPECT_EQ(Ptr, DC.Ptr);
   }
+  void verifyReallocHooksScope(UNUSED void *Ptr) {
+    if (SCUDO_ENABLE_HOOKS_TESTS) {
+      EXPECT_EQ(Ptr, RC.Begin);
+      EXPECT_EQ(Ptr, RC.End);
+    }
+  }
 };
 using ScudoWrappersCDeathTest = ScudoWrappersCTest;
 
@@ -297,6 +317,7 @@ TEST_F(ScudoWrappersCDeathTest, Realloc) {
     verifyAllocHookSize(Size * 2U);
     verifyDeallocHookPtr(OldP);
   }
+  verifyReallocHooksScope(OldP);
 
   invalidateHookPtrs();
   OldP = P;
@@ -312,6 +333,7 @@ TEST_F(ScudoWrappersCDeathTest, Realloc) {
     verifyAllocHookPtr(P);
     verifyAllocHookSize(Size / 2U);
   }
+  verifyReallocHooksScope(OldP);
   free(P);
 
   EXPECT_DEATH(P = realloc(P, Size), "");
diff --git a/compiler-rt/lib/scudo/standalone/wrappers_c.inc b/compiler-rt/lib/scudo/standalone/wrappers_c.inc
index 0413ea49eac08..c6d0fb1132aa9 100644
--- a/compiler-rt/lib/scudo/standalone/wrappers_c.inc
+++ b/compiler-rt/lib/scudo/standalone/wrappers_c.inc
@@ -27,6 +27,40 @@ static void reportDeallocation(void *ptr) {
     if (__scudo_deallocate_hook)
       __scudo_deallocate_hook(ptr);
 }
+static void reportReallocBegin(void *old_ptr) {
+  if (SCUDO_ENABLE_HOOKS)
+    if (__scudo_realloc_begin_hook)
+      __scudo_realloc_begin_hook(old_ptr);
+}
+static void reportReallocEnd(void *old_ptr) {
+  if (SCUDO_ENABLE_HOOKS)
+    if (__scudo_realloc_end_hook)
+      __scudo_realloc_end_hook(old_ptr);
+}
+
+static void *reallocImpl(void *ptr, size_t size) {
+  // 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 != nullptr) {
+    // Note that even if NewPtr == ptr, the size has changed. We still need to
+    // report the new size.
+    reportAllocation(NewPtr, size);
+  } else {
+    // If `realloc` fails, the old pointer is not released. Report the old
+    // pointer as allocated back.
+    reportAllocation(ptr, SCUDO_ALLOCATOR.getAllocSize(ptr));
+  }
+
+  return NewPtr;
+}
 
 extern "C" {
 
@@ -175,25 +209,11 @@ 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 != nullptr) {
-    // Note that even if NewPtr == ptr, the size has changed. We still need to
-    // report the new size.
-    reportAllocation(NewPtr, size);
-  } else {
-    // If `realloc` fails, the old pointer is not released. Report the old
-    // pointer as allocated back.
-    reportAllocation(ptr, SCUDO_ALLOCATOR.getAllocSize(ptr));
-  }
+  reportReallocBegin(ptr);
+
+  void *NewPtr = reallocImpl(ptr, size);
+
+  reportReallocEnd(ptr);
 
   return scudo::setErrnoOnNull(NewPtr);
 }

``````````

</details>


https://github.com/llvm/llvm-project/pull/74353