Skip to content

Commit ed0fd13

Browse files
authored
[scudo] Double frees result in chunk state error (llvm#110345)
Fixes bug where a device that supports tagged pointers doesn't use the tagged pointer when computing the checksum. Add tests to verify that double frees result in chunk state error not corrupted header errors.
1 parent 1b6a46a commit ed0fd13

File tree

2 files changed

+35
-8
lines changed

2 files changed

+35
-8
lines changed

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

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1255,22 +1255,26 @@ class Allocator {
12551255
else
12561256
Header->State = Chunk::State::Quarantined;
12571257

1258-
void *BlockBegin;
1259-
if (LIKELY(!useMemoryTagging<AllocatorConfig>(Options))) {
1258+
if (LIKELY(!useMemoryTagging<AllocatorConfig>(Options)))
12601259
Header->OriginOrWasZeroed = 0U;
1261-
if (BypassQuarantine && allocatorSupportsMemoryTagging<AllocatorConfig>())
1262-
Ptr = untagPointer(Ptr);
1263-
BlockBegin = getBlockBegin(Ptr, Header);
1264-
} else {
1260+
else {
12651261
Header->OriginOrWasZeroed =
12661262
Header->ClassId && !TSDRegistry.getDisableMemInit();
1267-
BlockBegin =
1268-
retagBlock(Options, TaggedPtr, Ptr, Header, Size, BypassQuarantine);
12691263
}
12701264

12711265
Chunk::storeHeader(Cookie, Ptr, Header);
12721266

12731267
if (BypassQuarantine) {
1268+
void *BlockBegin;
1269+
if (LIKELY(!useMemoryTagging<AllocatorConfig>(Options))) {
1270+
// Must do this after storeHeader because loadHeader uses a tagged ptr.
1271+
if (allocatorSupportsMemoryTagging<AllocatorConfig>())
1272+
Ptr = untagPointer(Ptr);
1273+
BlockBegin = getBlockBegin(Ptr, Header);
1274+
} else {
1275+
BlockBegin = retagBlock(Options, TaggedPtr, Ptr, Header, Size, true);
1276+
}
1277+
12741278
const uptr ClassId = Header->ClassId;
12751279
if (LIKELY(ClassId)) {
12761280
bool CacheDrained;
@@ -1288,6 +1292,8 @@ class Allocator {
12881292
Secondary.deallocate(Options, BlockBegin);
12891293
}
12901294
} else {
1295+
if (UNLIKELY(useMemoryTagging<AllocatorConfig>(Options)))
1296+
retagBlock(Options, TaggedPtr, Ptr, Header, Size, false);
12911297
typename TSDRegistryT::ScopedTSD TSD(TSDRegistry);
12921298
Quarantine.put(&TSD->getQuarantineCache(),
12931299
QuarantineCallback(*this, TSD->getCache()), Ptr, Size);

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,27 @@ SCUDO_TYPED_TEST(ScudoCombinedDeathTest, UseAfterFree) {
534534
}
535535
}
536536

537+
SCUDO_TYPED_TEST(ScudoCombinedDeathTest, DoubleFreeFromPrimary) {
538+
auto *Allocator = this->Allocator.get();
539+
540+
for (scudo::uptr SizeLog = 0U; SizeLog <= 20U; SizeLog++) {
541+
const scudo::uptr Size = 1U << SizeLog;
542+
if (!isPrimaryAllocation<TestAllocator<TypeParam>>(Size, 0))
543+
break;
544+
545+
// Verify that a double free results in a chunk state error.
546+
EXPECT_DEATH(
547+
{
548+
// Allocate from primary
549+
void *P = Allocator->allocate(Size, Origin);
550+
ASSERT_TRUE(P != nullptr);
551+
Allocator->deallocate(P, Origin);
552+
Allocator->deallocate(P, Origin);
553+
},
554+
"invalid chunk state");
555+
}
556+
}
557+
537558
SCUDO_TYPED_TEST(ScudoCombinedDeathTest, DisableMemoryTagging) {
538559
auto *Allocator = this->Allocator.get();
539560

0 commit comments

Comments
 (0)