[compiler-rt] [sanitizer_common][lsan] Keep the original allocation alive when realloc fails (PR #222849)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 10 23:07:03 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-compiler-rt-sanitizer
Author: Bojun Seo (Bojun-Seo)
<details>
<summary>Changes</summary>
CombinedAllocator::Reallocate() deallocated the original chunk even when
the replacement allocation failed, and __lsan::Reallocate() unregistered
the pointer before asking for that replacement. A failing realloc()
therefore released memory the caller still owned: a free hook fired for a
live pointer, and a later malloc() handed the same chunk out again.
Return early in the allocator, and in LSan allocate the replacement
before releasing the original, the way ASan and HWASan already do it.
Going through __lsan::Allocate() also makes realloc() honor
allocator_may_return_null, and tracks realloc(NULL, 0) as the malloc(0)
it is rather than as a zero-sized allocation.
Assisted-by: Claude Opus 5
---
Full diff: https://github.com/llvm/llvm-project/pull/222849.diff
5 Files Affected:
- (modified) compiler-rt/lib/lsan/lsan_allocator.cpp (+16-7)
- (modified) compiler-rt/lib/sanitizer_common/sanitizer_allocator_combined.h (+4-2)
- (modified) compiler-rt/lib/sanitizer_common/tests/sanitizer_allocator_test.cpp (+52)
- (added) compiler-rt/test/lsan/TestCases/Linux/realloc_failure_keeps_original.cpp (+75)
- (modified) compiler-rt/test/lsan/TestCases/realloc_zero.c (+14)
``````````diff
diff --git a/compiler-rt/lib/lsan/lsan_allocator.cpp b/compiler-rt/lib/lsan/lsan_allocator.cpp
index 110c1cfc3bf92..8433626f60816 100644
--- a/compiler-rt/lib/lsan/lsan_allocator.cpp
+++ b/compiler-rt/lib/lsan/lsan_allocator.cpp
@@ -133,13 +133,22 @@ void *Reallocate(const StackTrace &stack, void *p, uptr new_size,
ReportAllocationSizeTooBig(new_size, stack);
return nullptr;
}
- RegisterDeallocation(p);
- void *new_p =
- allocator.Reallocate(GetAllocatorCache(), p, new_size, alignment);
- if (new_p)
- RegisterAllocation(stack, new_p, new_size);
- else if (new_size != 0)
- RegisterAllocation(stack, p, new_size);
+ if (!p)
+ return Allocate(stack, new_size, alignment, false);
+ if (!new_size) {
+ Deallocate(p);
+ return nullptr;
+ }
+ CHECK(allocator.PointerIsMine(p));
+ ChunkMetadata *m = Metadata(p);
+ CHECK(m);
+ const uptr old_size = m->requested_size;
+ // Allocate first: on failure p must be left untouched.
+ void *new_p = Allocate(stack, new_size, alignment, false);
+ if (!new_p)
+ return nullptr;
+ internal_memcpy(new_p, p, Min(new_size, old_size));
+ Deallocate(p);
return new_p;
}
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_combined.h b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_combined.h
index 49940d9b5d505..938bb4c173764 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_combined.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_combined.h
@@ -106,8 +106,10 @@ class CombinedAllocator {
uptr old_size = GetActuallyAllocatedSize(p);
uptr memcpy_size = Min(new_size, old_size);
void *new_p = Allocate(cache, new_size, alignment);
- if (new_p)
- internal_memcpy(new_p, p, memcpy_size);
+ // On failure the caller still owns p, as realloc() requires.
+ if (!new_p)
+ return nullptr;
+ internal_memcpy(new_p, p, memcpy_size);
Deallocate(cache, p);
return new_p;
}
diff --git a/compiler-rt/lib/sanitizer_common/tests/sanitizer_allocator_test.cpp b/compiler-rt/lib/sanitizer_common/tests/sanitizer_allocator_test.cpp
index 601897a64f051..21d9e4180cd63 100644
--- a/compiler-rt/lib/sanitizer_common/tests/sanitizer_allocator_test.cpp
+++ b/compiler-rt/lib/sanitizer_common/tests/sanitizer_allocator_test.cpp
@@ -750,6 +750,58 @@ void TestCombinedAllocator(uptr premapped_heap = 0) {
allocated.clear();
a->SwallowCache(&cache);
}
+
+ // A failing Reallocate() must keep p allocated. (uptr)-1 overflows the
+ // size-plus-alignment check inside Allocate(), which logs a warning.
+ {
+ const uptr kSize = 128;
+ char *p = reinterpret_cast<char *>(a->Allocate(&cache, kSize, 1));
+ ASSERT_NE(p, nullptr);
+ uptr *meta = reinterpret_cast<uptr *>(a->GetMetaData(p));
+ *meta = kSize;
+ internal_memset(p, 'x', kSize);
+
+ EXPECT_EQ(a->Reallocate(&cache, p, (uptr)-1, 1), nullptr);
+
+ // These hold even for a released chunk; the loop below is what detects it.
+ EXPECT_EQ(*reinterpret_cast<uptr *>(a->GetMetaData(p)), kSize);
+ EXPECT_EQ(p[0], 'x');
+ EXPECT_EQ(p[kSize - 1], 'x');
+
+ void *others[8];
+ for (uptr i = 0; i < ARRAY_SIZE(others); i++) {
+ others[i] = a->Allocate(&cache, kSize, 1);
+ EXPECT_NE(others[i], p);
+ }
+ for (uptr i = 0; i < ARRAY_SIZE(others); i++)
+ a->Deallocate(&cache, others[i]);
+
+ *meta = 0;
+ a->Deallocate(&cache, p);
+ a->SwallowCache(&cache);
+ }
+
+ // Same for the secondary, where a release unmaps the chunk. A regression
+ // leaves p unmapped, so assert ownership before reading through it.
+ {
+ const uptr kLarge = 1 << 20;
+ void *p = a->Allocate(&cache, kLarge, 1);
+ ASSERT_NE(p, nullptr);
+ if (!a->FromPrimary(p)) {
+ uptr *meta = reinterpret_cast<uptr *>(a->GetMetaData(p));
+ *meta = kLarge;
+
+ EXPECT_EQ(a->Reallocate(&cache, p, (uptr)-1, 1), nullptr);
+
+ ASSERT_TRUE(a->PointerIsMine(p));
+ EXPECT_EQ(a->GetBlockBegin(p), p);
+ EXPECT_EQ(*reinterpret_cast<uptr *>(a->GetMetaData(p)), kLarge);
+ *meta = 0;
+ }
+ a->Deallocate(&cache, p);
+ a->SwallowCache(&cache);
+ }
+
a->DestroyCache(&cache);
a->TestOnlyUnmap();
}
diff --git a/compiler-rt/test/lsan/TestCases/Linux/realloc_failure_keeps_original.cpp b/compiler-rt/test/lsan/TestCases/Linux/realloc_failure_keeps_original.cpp
new file mode 100644
index 0000000000000..35482a6fbaf9a
--- /dev/null
+++ b/compiler-rt/test/lsan/TestCases/Linux/realloc_failure_keeps_original.cpp
@@ -0,0 +1,75 @@
+// Verifies that a failing realloc() leaves the original allocation untouched
+// and reports no free hook for it.
+//
+// RUN: %clangxx_lsan %s -o %t
+// RUN: %env_lsan_opts=detect_leaks=0:allocator_may_return_null=1 %run %t 2>&1 | FileCheck %s
+// REQUIRES: lsan-standalone
+
+#include <sanitizer/allocator_interface.h>
+
+#include <cassert>
+#include <cstdio>
+#include <cstdlib>
+#include <cstring>
+#include <sys/resource.h>
+#include <unistd.h>
+
+static void *g_freed;
+static void *g_alloced;
+
+static void OnMalloc(const volatile void *ptr, size_t) {
+ g_alloced = (void *)ptr;
+}
+
+static void OnFree(const volatile void *ptr) { g_freed = (void *)ptr; }
+
+// Caps the address space just above the current usage, so that the large
+// mmap below fails inside the allocator rather than being rejected up front
+// by max_allocation_size_mb.
+static void CapAddressSpace() {
+ FILE *f = fopen("/proc/self/statm", "r");
+ assert(f);
+ unsigned long vsz_pages = 0;
+ int scanned = fscanf(f, "%lu", &vsz_pages);
+ fclose(f);
+ assert(scanned == 1);
+
+ rlimit rl;
+ int res = getrlimit(RLIMIT_AS, &rl);
+ assert(res == 0);
+ rl.rlim_cur = (rlim_t)vsz_pages * getpagesize() + (64UL << 20);
+ res = setrlimit(RLIMIT_AS, &rl);
+ assert(res == 0);
+}
+
+int main() {
+ const size_t kSize = 100;
+ char *p = (char *)malloc(kSize);
+ assert(p);
+ memset(p, 'a', kSize);
+
+ // Install the hooks only after CapAddressSpace(), which itself allocates.
+ CapAddressSpace();
+ int installed = __sanitizer_install_malloc_and_free_hooks(OnMalloc, OnFree);
+ assert(installed);
+
+ void *q = realloc(p, 512UL << 20);
+ assert(q == NULL);
+
+ assert(g_freed == nullptr);
+ assert(g_alloced == nullptr);
+
+ assert(__sanitizer_get_ownership(p));
+ assert(__sanitizer_get_allocated_size(p) == kSize);
+ for (size_t i = 0; i < kSize; ++i)
+ assert(p[i] == 'a');
+
+ fprintf(stderr, "original allocation survived realloc failure\n");
+ free(p);
+ assert(g_freed == p);
+ fprintf(stderr, "freed once\n");
+ return 0;
+}
+
+// CHECK: original allocation survived realloc failure
+// CHECK: freed once
diff --git a/compiler-rt/test/lsan/TestCases/realloc_zero.c b/compiler-rt/test/lsan/TestCases/realloc_zero.c
index d4ce4754d9bdf..84ec830455065 100644
--- a/compiler-rt/test/lsan/TestCases/realloc_zero.c
+++ b/compiler-rt/test/lsan/TestCases/realloc_zero.c
@@ -4,10 +4,24 @@
#include <assert.h>
#include <stdlib.h>
+#if __has_feature(leak_sanitizer)
+# include <sanitizer/allocator_interface.h>
+#endif
+
int main() {
char *p = malloc(1);
// The behavior of realloc(p, 0) is implementation-defined.
// We free the allocation.
assert(realloc(p, 0) == NULL);
+
+ // realloc(NULL, 0) allocates instead, and must be tracked like malloc(0).
+ void *q = realloc(NULL, 0);
+ assert(q != NULL);
+#if __has_feature(leak_sanitizer)
+ assert(__sanitizer_get_ownership(q));
+ assert(__sanitizer_get_allocated_size(q) == 1);
+#endif
+ free(q);
+
p = 0;
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/222849
More information about the llvm-commits
mailing list