[compiler-rt] a5bdc4a - [scudo] do not store size inside ring buffer (#74541)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 8 17:17:01 PST 2023
Author: Florian Mayer
Date: 2023-12-08T17:16:56-08:00
New Revision: a5bdc4a4604272fa2b2c60fa9dde7aee05456162
URL: https://github.com/llvm/llvm-project/commit/a5bdc4a4604272fa2b2c60fa9dde7aee05456162
DIFF: https://github.com/llvm/llvm-project/commit/a5bdc4a4604272fa2b2c60fa9dde7aee05456162.diff
LOG: [scudo] do not store size inside ring buffer (#74541)
Added:
Modified:
compiler-rt/lib/scudo/standalone/combined.h
compiler-rt/lib/scudo/standalone/fuzz/get_error_info_fuzzer.cpp
compiler-rt/lib/scudo/standalone/wrappers_c_bionic.cpp
Removed:
################################################################################
diff --git a/compiler-rt/lib/scudo/standalone/combined.h b/compiler-rt/lib/scudo/standalone/combined.h
index 25ad11dbf7ee54..65ddc488370a7d 100644
--- a/compiler-rt/lib/scudo/standalone/combined.h
+++ b/compiler-rt/lib/scudo/standalone/combined.h
@@ -14,6 +14,7 @@
#include "flags.h"
#include "flags_parser.h"
#include "local_cache.h"
+#include "mem_map.h"
#include "memtag.h"
#include "options.h"
#include "quarantine.h"
@@ -935,8 +936,7 @@ class Allocator {
uptr getRingBufferSize() {
initThreadMaybe();
- auto *RingBuffer = getRingBuffer();
- return RingBuffer ? ringBufferSizeInBytes(RingBuffer->Size) : 0;
+ return RingBufferElements ? ringBufferSizeInBytes(RingBufferElements) : 0;
}
static bool setRingBufferSizeForBuffer(char *Buffer, size_t Size) {
@@ -966,8 +966,9 @@ class Allocator {
static void getErrorInfo(struct scudo_error_info *ErrorInfo,
uintptr_t FaultAddr, const char *DepotPtr,
const char *RegionInfoPtr, const char *RingBufferPtr,
- const char *Memory, const char *MemoryTags,
- uintptr_t MemoryAddr, size_t MemorySize) {
+ size_t RingBufferSize, const char *Memory,
+ const char *MemoryTags, uintptr_t MemoryAddr,
+ size_t MemorySize) {
*ErrorInfo = {};
if (!allocatorSupportsMemoryTagging<Config>() ||
MemoryAddr + MemorySize < MemoryAddr)
@@ -986,7 +987,7 @@ class Allocator {
// Check the ring buffer. For primary allocations this will only find UAF;
// for secondary allocations we can find either UAF or OOB.
getRingBufferErrorInfo(ErrorInfo, NextErrorReport, FaultAddr, Depot,
- RingBufferPtr);
+ RingBufferPtr, RingBufferSize);
// Check for OOB in the 28 blocks surrounding the 3 we checked earlier.
// Beyond that we are likely to hit false positives.
@@ -1051,15 +1052,15 @@ class Allocator {
atomic_u32 DeallocationTid;
};
- MemMapT MemMap;
atomic_uptr Pos;
- u32 Size;
// An array of Size (at least one) elements of type Entry is immediately
// following to this struct.
};
// Pointer to memory mapped area starting with AllocationRingBuffer struct,
// and immediately followed by Size elements of type Entry.
char *RawRingBuffer = {};
+ u32 RingBufferElements = 0;
+ MemMapT RawRingBufferMap;
// The following might get optimized out by the compiler.
NOINLINE void performSanityChecks() {
@@ -1267,7 +1268,7 @@ class Allocator {
u32 DeallocationTid) {
uptr Pos = atomic_fetch_add(&getRingBuffer()->Pos, 1, memory_order_relaxed);
typename AllocationRingBuffer::Entry *Entry =
- getRingBufferEntry(RawRingBuffer, Pos % getRingBuffer()->Size);
+ getRingBufferEntry(RawRingBuffer, Pos % RingBufferElements);
// First invalidate our entry so that we don't attempt to interpret a
// partially written state in getSecondaryErrorInfo(). The fences below
@@ -1408,17 +1409,19 @@ class Allocator {
size_t &NextErrorReport,
uintptr_t FaultAddr,
const StackDepot *Depot,
- const char *RingBufferPtr) {
+ const char *RingBufferPtr,
+ size_t RingBufferSize) {
auto *RingBuffer =
reinterpret_cast<const AllocationRingBuffer *>(RingBufferPtr);
- if (!RingBuffer || RingBuffer->Size == 0)
+ size_t RingBufferElements = ringBufferElementsFromBytes(RingBufferSize);
+ if (!RingBuffer || RingBufferElements == 0)
return;
uptr Pos = atomic_load_relaxed(&RingBuffer->Pos);
- for (uptr I = Pos - 1;
- I != Pos - 1 - RingBuffer->Size && NextErrorReport != NumErrorReports;
+ for (uptr I = Pos - 1; I != Pos - 1 - RingBufferElements &&
+ NextErrorReport != NumErrorReports;
--I) {
- auto *Entry = getRingBufferEntry(RingBufferPtr, I % RingBuffer->Size);
+ auto *Entry = getRingBufferEntry(RingBufferPtr, I % RingBufferElements);
uptr EntryPtr = atomic_load_relaxed(&Entry->Ptr);
if (!EntryPtr)
continue;
@@ -1508,9 +1511,8 @@ class Allocator {
getPageSizeCached()),
"scudo:ring_buffer");
RawRingBuffer = reinterpret_cast<char *>(MemMap.getBase());
- auto *RingBuffer = reinterpret_cast<AllocationRingBuffer *>(RawRingBuffer);
- RingBuffer->MemMap = MemMap;
- RingBuffer->Size = AllocationRingBufferSize;
+ RawRingBufferMap = MemMap;
+ RingBufferElements = AllocationRingBufferSize;
static_assert(sizeof(AllocationRingBuffer) %
alignof(typename AllocationRingBuffer::Entry) ==
0,
@@ -1520,16 +1522,23 @@ class Allocator {
void unmapRingBuffer() {
auto *RingBuffer = getRingBuffer();
if (RingBuffer != nullptr) {
- MemMapT MemMap = RingBuffer->MemMap;
- MemMap.unmap(MemMap.getBase(), MemMap.getCapacity());
+ RawRingBufferMap.unmap(RawRingBufferMap.getBase(),
+ RawRingBufferMap.getCapacity());
}
RawRingBuffer = nullptr;
}
- static constexpr size_t ringBufferSizeInBytes(u32 AllocationRingBufferSize) {
+ static constexpr size_t ringBufferSizeInBytes(u32 RingBufferElements) {
return sizeof(AllocationRingBuffer) +
- AllocationRingBufferSize *
- sizeof(typename AllocationRingBuffer::Entry);
+ RingBufferElements * sizeof(typename AllocationRingBuffer::Entry);
+ }
+
+ static constexpr size_t ringBufferElementsFromBytes(size_t Bytes) {
+ if (Bytes < sizeof(AllocationRingBuffer)) {
+ return 0;
+ }
+ return (Bytes - sizeof(AllocationRingBuffer)) /
+ sizeof(typename AllocationRingBuffer::Entry);
}
inline AllocationRingBuffer *getRingBuffer() {
diff --git a/compiler-rt/lib/scudo/standalone/fuzz/get_error_info_fuzzer.cpp b/compiler-rt/lib/scudo/standalone/fuzz/get_error_info_fuzzer.cpp
index 74456450a47612..5b01ebe11c0952 100644
--- a/compiler-rt/lib/scudo/standalone/fuzz/get_error_info_fuzzer.cpp
+++ b/compiler-rt/lib/scudo/standalone/fuzz/get_error_info_fuzzer.cpp
@@ -46,14 +46,11 @@ extern "C" int LLVMFuzzerTestOneInput(uint8_t *Data, size_t Size) {
}
std::string RingBufferBytes = FDP.ConsumeRemainingBytesAsString();
- // RingBuffer is too short.
- if (!AllocatorT::setRingBufferSizeForBuffer(RingBufferBytes.data(),
- RingBufferBytes.size()))
- return 0;
scudo_error_info ErrorInfo;
AllocatorT::getErrorInfo(&ErrorInfo, FaultAddr, StackDepot.data(),
- RegionInfo.data(), RingBufferBytes.data(), Memory,
- MemoryTags, MemoryAddr, MemorySize);
+ RegionInfo.data(), RingBufferBytes.data(),
+ RingBufferBytes.size(), Memory, MemoryTags,
+ MemoryAddr, MemorySize);
return 0;
}
diff --git a/compiler-rt/lib/scudo/standalone/wrappers_c_bionic.cpp b/compiler-rt/lib/scudo/standalone/wrappers_c_bionic.cpp
index f203615ab3602d..21694c3f17fe57 100644
--- a/compiler-rt/lib/scudo/standalone/wrappers_c_bionic.cpp
+++ b/compiler-rt/lib/scudo/standalone/wrappers_c_bionic.cpp
@@ -44,10 +44,9 @@ INTERFACE void __scudo_get_error_info(
const char *ring_buffer, size_t ring_buffer_size, const char *memory,
const char *memory_tags, uintptr_t memory_addr, size_t memory_size) {
(void)(stack_depot_size);
- (void)(ring_buffer_size);
Allocator.getErrorInfo(error_info, fault_addr, stack_depot, region_info,
- ring_buffer, memory, memory_tags, memory_addr,
- memory_size);
+ ring_buffer, ring_buffer_size, memory, memory_tags,
+ memory_addr, memory_size);
}
INTERFACE const char *__scudo_get_stack_depot_addr() {
More information about the llvm-commits
mailing list