scudo: Check for UAF in ring buffer before OOB in more distant blocks.

[pcc]

Summary

It's more likely that we have a UAF than an OOB in blocks that are
more than 1 block away from the fault address, so the UAF should
appear first in the error report.

[eugenis]

compiler-rt/lib/scudo/standalone/combined.h

1325

Did you mean

if (CheckOOB(Info.BlockBegin) || MinDistance == 0)

?

[pcc]

compiler-rt/lib/scudo/standalone/combined.h

1325

I think that would result in us not checking the surrounding blocks if MinDistance == 0, even if MaxDistance was some larger number.

[eugenis]

LGTM

compiler-rt/lib/scudo/standalone/combined.h

static void getErrorInfo(struct scudo_error_info *ErrorInfo,
                            uintptr_t MemoryAddr, size_t MemorySize) {
     *ErrorInfo = {};
     if (!allocatorSupportsMemoryTagging<Params>() ||
         MemoryAddr + MemorySize < MemoryAddr)
       return;
 
     auto *Depot = reinterpret_cast<const StackDepot *>(DepotPtr);
     size_t NextErrorReport = 0;
+
+    // Check for OOB in the current block and the two surrounding blocks. Beyond
+    // that, UAF is more likely.
     if (extractTag(FaultAddr) != 0)
       getInlineErrorInfo(ErrorInfo, NextErrorReport, FaultAddr, Depot,
                          RegionInfoPtr, Memory, MemoryTags, MemoryAddr,
-                         MemorySize);
+                         MemorySize, 0, 2);
+
+    // 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);
+
+    // Check for OOB in the 28 blocks surrounding the 3 we checked earlier.
+    // Beyond that we are likely to hit false positives.
+    if (extractTag(FaultAddr) != 0)
+      getInlineErrorInfo(ErrorInfo, NextErrorReport, FaultAddr, Depot,
+                         RegionInfoPtr, Memory, MemoryTags, MemoryAddr,
+                         MemorySize, 2, 16);
   }
 
 private:
   using SecondaryT = MapAllocator<Params>;
   typedef typename PrimaryT::SizeClassMap SizeClassMap;
 
   static const uptr MinAlignmentLog = SCUDO_MIN_ALIGNMENT_LOG;
   static const uptr MaxAlignmentLog = 24U; // 16 MB seems reasonable.
static const size_t NumErrorReports =
       sizeof(((scudo_error_info *)0)->reports) /
       sizeof(((scudo_error_info *)0)->reports[0]);
 
   static void getInlineErrorInfo(struct scudo_error_info *ErrorInfo,
                                  size_t &NextErrorReport, uintptr_t FaultAddr,
                                  const StackDepot *Depot,
                                  const char *RegionInfoPtr, const char *Memory,
                                  const char *MemoryTags, uintptr_t MemoryAddr,
-                                 size_t MemorySize) {
+                                 size_t MemorySize, size_t MinDistance,
+                                 size_t MaxDistance) {
     uptr UntaggedFaultAddr = untagPointer(FaultAddr);
     u8 FaultAddrTag = extractTag(FaultAddr);
     BlockInfo Info =
         PrimaryT::findNearestBlock(RegionInfoPtr, UntaggedFaultAddr);
 
     auto GetGranule = [&](uptr Addr, const char **Data, uint8_t *Tag) -> bool {
       if (Addr < MemoryAddr || Addr + archMemoryTagGranuleSize() < Addr ||
           Addr + archMemoryTagGranuleSize() > MemoryAddr + MemorySize)
auto CheckOOB = [&](uptr BlockAddr) {
       R->allocation_address = ChunkAddr;
       R->allocation_size = Header.SizeOrUnusedBytes;
       collectTraceMaybe(Depot, R->allocation_trace,
                         Data[MemTagAllocationTraceIndex]);
       R->allocation_tid = Data[MemTagAllocationTidIndex];
       return NextErrorReport == NumErrorReports;
     };
 
-    if (CheckOOB(Info.BlockBegin))
+    if (MinDistance == 0 && CheckOOB(Info.BlockBegin))
       return;
 
-    // Check for OOB in the 30 surrounding blocks. Beyond that we are likely to
-    // hit false positives.
-    for (int I = 1; I != 16; ++I)
+    for (size_t I = Max<size_t>(MinDistance, 1); I != MaxDistance; ++I)
       if (CheckOOB(Info.BlockBegin + I * Info.BlockSize) ||
           CheckOOB(Info.BlockBegin - I * Info.BlockSize))
         return;
   }
 
   static void getRingBufferErrorInfo(struct scudo_error_info *ErrorInfo,
                                      size_t &NextErrorReport,
                                      uintptr_t FaultAddr,