[scudo] Detect double free when running with MTE.

[eugenis]

Summary

Try to trigger an MTE fault on double/invalid free by touching the first
byte of the allocation with the provided pointer.

[pcc]

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

565

For zero-sized allocations I'd expect the correct allocation tag to be stored in the first byte of the granule. It may be simpler to say that we skip this for zero-sized allocations because it is not expected to succeed even for valid allocations.

[eugenis]

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

565

Not always - ex. zero size, 32 byte aligned allocation in a 32-byte size class will end up with header at offset 16 in the block, payload at offset 32, and no space to store the tag. We could bump the allocated size a little to handle this case, but I don't know if it is worth it. It will also make the deallocation logic more costly.

I've benchmarked the current change and could not see any overhead.

[eugenis]

ping

[pcc]

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

565

Oh right, I forgot about that case. So can we add this example to the comment?

[eugenis]

expanded comments

[eugenis]

ping

[fmayer]

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

571

nit: afaict llvm uses const char, so I think this should be volatile char for consistency.

[eugenis]

addressed Florian's comment

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

if (Options.get(OptionBit::DeallocTypeMismatch)) {
         // With the exception of memalign'd chunks, that can be still be free'd.
         if (Header.OriginOrWasZeroed != Chunk::Origin::Memalign ||
             Origin != Chunk::Origin::Malloc)
           reportDeallocTypeMismatch(AllocatorAction::Deallocating, Ptr,
                                     Header.OriginOrWasZeroed, Origin);
       }
     }
 
+    // Try to detect deallocation with a wrong MTE tag by touching the first
+    // byte with a correctly tagged pointer. Skip zero-sized allocations that do
+    // not always store the correct tag value anywhere (for example, a zero
+    // size, 32 byte aligned allocation in a 32-byte size class will end up with
+    // header at offset 16 in the block, payload at offset 32, and no space to
+    // store the tag).
     const uptr Size = getSize(Ptr, &Header);
+    if (useMemoryTagging<Params>(Options) && Size != 0)
+      *(char volatile *)TaggedPtr;
+
     if (DeleteSize && Options.get(OptionBit::DeleteSizeMismatch)) {
       if (UNLIKELY(DeleteSize != Size))
         reportDeleteSizeMismatch(Ptr, DeleteSize, Size);
     }
 
     quarantineOrDeallocateChunk(Options, TaggedPtr, &Header, Size);
   }
 

compiler-rt/lib/scudo/standalone/tests/combined_test.cpp

EXPECT_DEATH(
           void *P = Allocator->allocate(Size, Origin);
           Allocator->deallocate(P, Origin);
           reinterpret_cast<char *>(P)[Size - 1] = 0xaa;
         },
         "");
   }
 }
 
+SCUDO_TYPED_TEST(ScudoCombinedDeathTest, FreeWithTagMismatch) {
+  auto *Allocator = this->Allocator.get();
+
+  if (!Allocator->useMemoryTaggingTestOnly())
+    return;
+
+  // Check that double free is detected.
+  for (scudo::uptr SizeLog = 0U; SizeLog <= 20U; SizeLog++) {
+    const scudo::uptr Size = 1U << SizeLog;
+    EXPECT_DEATH(
+        {
+          disableDebuggerdMaybe();
+          void *P = Allocator->allocate(Size, Origin);
+          scudo::uptr NewTag = (scudo::extractTag(reinterpret_cast<scudo::uptr>(P)) + 1) % 16;
+          void *Q = scudo::addFixedTag(scudo::untagPointer(P), NewTag);
+          Allocator->deallocate(Q, Origin);
+        },
+        "");
+  }
+}
+
 SCUDO_TYPED_TEST(ScudoCombinedDeathTest, DisableMemoryTagging) {
   auto *Allocator = this->Allocator.get();
 
   if (Allocator->useMemoryTaggingTestOnly()) {
     // Check that disabling memory tagging works correctly.
     void *P = Allocator->allocate(2048, Origin);
     EXPECT_DEATH(reinterpret_cast<char *>(P)[2048] = 0xaa, "");
     scudo::ScopedDisableMemoryTagChecks NoTagChecks;