[compiler-rt] [rtsan] Fix sanitizer_common/TestCases/dlsym_alloc.c (PR #216579)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

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

Author: Chris Apple (cjappl)

<details>
<summary>Changes</summary>

This test crashes on `free(NULL)` when DlsymAlloc is being used. This is because `DlysmAlloc::Free` seems to crash on linux when it's called with `nullptr`.

The vast majority of other sanitizers guard against it on the sanitizer side in this way:

https://github.com/llvm/llvm-project/blob/65e0fe689bb7c1a6516644c27c25c3fe3c8b1ac7/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp#L728

https://github.com/llvm/llvm-project/blob/65e0fe689bb7c1a6516644c27c25c3fe3c8b1ac7/compiler-rt/lib/lsan/lsan_interceptors.cpp#L78

For example.


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


1 Files Affected:

- (modified) compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp (+19-15) 


``````````diff
diff --git a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
index 0ec691b54768b..93bddf9e5f6bc 100644
--- a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
@@ -869,28 +869,32 @@ INTERCEPTOR(void *, calloc, SIZE_T num, SIZE_T size) {
 }
 
 INTERCEPTOR(void, free, void *ptr) {
-  if (DlsymAlloc::PointerIsMine(ptr))
-    return DlsymAlloc::Free(ptr);
-
   // According to the C and C++ standard, freeing a nullptr is guaranteed to be
   // a no-op (and thus real-time safe). This can be confirmed for looking at
   // __libc_free in the glibc source.
-  if (ptr != nullptr)
-    __rtsan_notify_intercepted_call("free");
+  // Crucially must be done before DlsymAlloc::Free, as it can crash on
+  // nullptr input on linux.
+  if (UNLIKELY(!ptr))
+    return;
+
+  if (DlsymAlloc::PointerIsMine(ptr))
+    return DlsymAlloc::Free(ptr);
+
+  __rtsan_notify_intercepted_call("free");
 
   return REAL(free)(ptr);
 }
 
 #if SANITIZER_INTERCEPT_FREE_SIZED
 INTERCEPTOR(void, free_sized, void *ptr, SIZE_T size) {
+  // see above comment in `free` interceptor
+  if (UNLIKELY(!ptr))
+    return;
+
   if (DlsymAlloc::PointerIsMine(ptr))
     return DlsymAlloc::Free(ptr);
 
-  // According to the C and C++ standard, freeing a nullptr is guaranteed to be
-  // a no-op (and thus real-time safe). This can be confirmed for looking at
-  // __libc_free in the glibc source.
-  if (ptr != nullptr)
-    __rtsan_notify_intercepted_call("free_sized");
+  __rtsan_notify_intercepted_call("free_sized");
 
   if (REAL(free_sized))
     return REAL(free_sized)(ptr, size);
@@ -904,14 +908,14 @@ INTERCEPTOR(void, free_sized, void *ptr, SIZE_T size) {
 #if SANITIZER_INTERCEPT_FREE_ALIGNED_SIZED
 INTERCEPTOR(void, free_aligned_sized, void *ptr, SIZE_T alignment,
             SIZE_T size) {
+  // see above comment in `free` interceptor
+  if (UNLIKELY(!ptr))
+    return;
+
   if (DlsymAlloc::PointerIsMine(ptr))
     return DlsymAlloc::Free(ptr);
 
-  // According to the C and C++ standard, freeing a nullptr is guaranteed to be
-  // a no-op (and thus real-time safe). This can be confirmed for looking at
-  // __libc_free in the glibc source.
-  if (ptr != nullptr)
-    __rtsan_notify_intercepted_call("free_aligned_sized");
+  __rtsan_notify_intercepted_call("free_aligned_sized");
 
   if (REAL(free_aligned_sized))
     return REAL(free_aligned_sized)(ptr, alignment, size);

``````````

</details>


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