[PATCH] D102376: scudo: Require fault address to be in bounds for UAF.
Peter Collingbourne via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed May 12 17:03:50 PDT 2021
pcc updated this revision to Diff 344984.
pcc added a comment.
- Add comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D102376/new/
https://reviews.llvm.org/D102376
Files:
compiler-rt/lib/scudo/standalone/combined.h
Index: compiler-rt/lib/scudo/standalone/combined.h
===================================================================
--- compiler-rt/lib/scudo/standalone/combined.h
+++ compiler-rt/lib/scudo/standalone/combined.h
@@ -1333,22 +1333,35 @@
--I) {
auto *Entry = &RingBuffer->Entries[I % AllocationRingBuffer::NumEntries];
uptr EntryPtr = atomic_load_relaxed(&Entry->Ptr);
- uptr UntaggedEntryPtr = untagPointer(EntryPtr);
- uptr EntrySize = atomic_load_relaxed(&Entry->AllocationSize);
- if (!EntryPtr || FaultAddr < EntryPtr - getPageSizeCached() ||
- FaultAddr >= EntryPtr + EntrySize + getPageSizeCached())
+ if (!EntryPtr)
continue;
+ uptr UntaggedEntryPtr = untagPointer(EntryPtr);
+ uptr EntrySize = atomic_load_relaxed(&Entry->AllocationSize);
u32 AllocationTrace = atomic_load_relaxed(&Entry->AllocationTrace);
u32 AllocationTid = atomic_load_relaxed(&Entry->AllocationTid);
u32 DeallocationTrace = atomic_load_relaxed(&Entry->DeallocationTrace);
u32 DeallocationTid = atomic_load_relaxed(&Entry->DeallocationTid);
- // For UAF the ring buffer will contain two entries, one for the
- // allocation and another for the deallocation. Don't report buffer
- // overflow/underflow using the allocation entry if we have already
- // collected a report from the deallocation entry.
- if (!DeallocationTrace) {
+ if (DeallocationTid) {
+ // For UAF we only consider in-bounds fault addresses because
+ // out-of-bounds UAF is rare and attempting to detect it is very likely
+ // to result in false positives.
+ if (FaultAddr < EntryPtr || FaultAddr >= EntryPtr + EntrySize)
+ continue;
+ } else {
+ // Ring buffer OOB is only possible with secondary allocations. In this
+ // case we are guaranteed a guard region of at least a page on either
+ // side of the allocation (guard page on the right, guard page + tagged
+ // region on the left), so ignore any faults outside of that range.
+ if (FaultAddr < EntryPtr - getPageSizeCached() ||
+ FaultAddr >= EntryPtr + EntrySize + getPageSizeCached())
+ continue;
+
+ // For UAF the ring buffer will contain two entries, one for the
+ // allocation and another for the deallocation. Don't report buffer
+ // overflow/underflow using the allocation entry if we have already
+ // collected a report from the deallocation entry.
bool Found = false;
for (uptr J = 0; J != NextErrorReport; ++J) {
if (ErrorInfo->reports[J].allocation_address == UntaggedEntryPtr) {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D102376.344984.patch
Type: text/x-patch
Size: 2692 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210513/10c95c13/attachment.bin>
More information about the llvm-commits
mailing list