[compiler-rt] [sanitizer_common][lsan] Keep the original allocation alive when realloc fails (PR #222849)
Bojun Seo via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 10 23:06:27 PDT 2026
https://github.com/Bojun-Seo created https://github.com/llvm/llvm-project/pull/222849
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
>From 78905b440265c7e9ab43a8bd176d234e45a9ad5c Mon Sep 17 00:00:00 2001
From: "bojun.seo" <bojun.seo at lge.com>
Date: Wed, 26 Aug 2026 11:15:39 +0900
Subject: [PATCH 1/2] [sanitizer_common] Keep the original allocation alive
when Reallocate fails
CombinedAllocator::Reallocate() deallocated the original chunk even when the
replacement allocation could not be satisfied, so a failing realloc() released
memory that the caller still owned. Callers that correctly retry or fall back
on failure were left with a dangling pointer.
Return early instead, which matches realloc() semantics: on failure the
original allocation is untouched and remains owned by the caller.
Stand-alone LeakSanitizer is the in-tree caller that can observe this, and it
does so in the default configuration: its realloc() reaches this function
directly, so the allocator_may_return_null check in __lsan::Allocate() is
bypassed and null is returned to the program after the chunk has already gone
back on the free list. __sanitizer_get_ownership() still reports that address
as live while a later malloc() hands the very same chunk out again. This commit
stops the release; the LSan side is fixed in the next one.
AddressSanitizer and MemProfiler are unaffected: both implement their own
Reallocate(), which releases the original only once the replacement has been
allocated. InternalRealloc() is layered on this function, but it turns a null
result into a fatal ReportInternalAllocatorOutOfMemory(), so the dangling
pointer never reaches its caller there.
Assisted-by: Claude Opus 5
---
.../sanitizer_allocator_combined.h | 6 ++-
.../tests/sanitizer_allocator_test.cpp | 52 +++++++++++++++++++
2 files changed, 56 insertions(+), 2 deletions(-)
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();
}
>From 146b7b6a08714f63b857d6b73b96135db4597923 Mon Sep 17 00:00:00 2001
From: "bojun.seo" <bojun.seo at lge.com>
Date: Wed, 9 Sep 2026 01:20:49 +0000
Subject: [PATCH 2/2] [lsan] Do not release the original allocation when
realloc fails
Reallocate() unregistered p before asking the allocator for a replacement, so a
failing realloc() reported a free hook for a pointer the caller still owned and
then re-registered that pointer with the *new* size and the realloc stack.
Hook consumers saw a free with no matching malloc, and
__sanitizer_get_allocated_size() reported a size that was never allocated.
Reorder the operation the way AddressSanitizer already does it: allocate the
replacement first and release the original only once that succeeded. On failure
p is now left completely untouched, so no hook fires and its metadata still
describes the original allocation.
The replacement now comes from __lsan::Allocate() rather than from
CombinedAllocator::Reallocate(), so realloc() also honors
allocator_may_return_null like every other allocation entry point: an
unsatisfiable request previously returned null whatever that flag said, and now
reports out-of-memory unless the flag allows null.
The same routing changes realloc(NULL, 0), which is malloc(0) by definition but
was registered with a requested size of zero: __sanitizer_get_ownership()
reported the returned pointer as not owned and __sanitizer_get_allocated_size()
reported zero for it, while malloc(0) reported one byte for the same thing. It is
now tracked exactly like malloc(0), and realloc_zero.c is extended to cover that
so it cannot silently regress.
LSan was the only sanitizer front-end still calling
CombinedAllocator::Reallocate(); the function keeps its other in-tree caller,
InternalRealloc(). Its CHECK(PointerIsMine(p)) moves here along with the rest:
without it the copy length below would come from whatever GetMetaData() reads
next to a pointer the allocator does not own.
Assisted-by: Claude Opus 5
---
compiler-rt/lib/lsan/lsan_allocator.cpp | 23 ++++--
.../Linux/realloc_failure_keeps_original.cpp | 75 +++++++++++++++++++
.../test/lsan/TestCases/realloc_zero.c | 14 ++++
3 files changed, 105 insertions(+), 7 deletions(-)
create mode 100644 compiler-rt/test/lsan/TestCases/Linux/realloc_failure_keeps_original.cpp
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/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;
}
More information about the llvm-commits
mailing list