-
Notifications
You must be signed in to change notification settings - Fork 14.5k
scudo: Support free_sized and free_aligned_sized from C23 #146556
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -170,6 +170,12 @@ class Allocator { | |
Primary.Options.set(OptionBit::DeallocTypeMismatch); | ||
if (getFlags()->delete_size_mismatch) | ||
Primary.Options.set(OptionBit::DeleteSizeMismatch); | ||
if (getFlags()->free_size_mismatch) | ||
Primary.Options.set(OptionBit::FreeSizeMismatch); | ||
if (getFlags()->free_alignment_mismatch) | ||
Primary.Options.set(OptionBit::FreeAlignmentMismatch); | ||
if (getFlags()->delete_alignment_mismatch) | ||
Primary.Options.set(OptionBit::DeleteAlignmentMismatch); | ||
if (allocatorSupportsMemoryTagging<AllocatorConfig>() && | ||
systemSupportsMemoryTagging()) | ||
Primary.Options.set(OptionBit::UseMemoryTagging); | ||
|
@@ -433,7 +439,8 @@ class Allocator { | |
} | ||
|
||
NOINLINE void deallocate(void *Ptr, Chunk::Origin Origin, uptr DeleteSize = 0, | ||
UNUSED uptr Alignment = MinAlignment) { | ||
bool HasDeleteSize = false, uptr DeleteAlignment = 0, | ||
bool HasDeleteAlignment = false) { | ||
Comment on lines
+442
to
+443
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we use |
||
if (UNLIKELY(!Ptr)) | ||
return; | ||
|
||
|
@@ -456,6 +463,9 @@ class Allocator { | |
} | ||
#endif // GWP_ASAN_HOOKS | ||
|
||
if (UNLIKELY(HasDeleteAlignment && !isPowerOfTwo(DeleteAlignment))) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why it's not be fore GuardedAlloc? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mostly because the isAligned check is also not above. If I move this one, arguably that one should also be moved. Thoughts? |
||
reportAlignmentNotPowerOfTwo(DeleteAlignment); | ||
|
||
if (UNLIKELY(!isAligned(reinterpret_cast<uptr>(Ptr), MinAlignment))) | ||
reportMisalignedPointer(AllocatorAction::Deallocating, Ptr); | ||
|
||
|
@@ -470,19 +480,41 @@ class Allocator { | |
|
||
const Options Options = Primary.Options.load(); | ||
if (Options.get(OptionBit::DeallocTypeMismatch)) { | ||
if (UNLIKELY(Header.OriginOrWasZeroed != Origin)) { | ||
// 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); | ||
} | ||
if (UNLIKELY(isOriginMismatch( | ||
static_cast<Chunk::Origin>(Header.OriginOrWasZeroed), Origin, | ||
HasDeleteSize))) | ||
reportDeallocTypeMismatch(AllocatorAction::Deallocating, Ptr, | ||
Header.OriginOrWasZeroed, Origin, | ||
HasDeleteSize); | ||
} | ||
|
||
const uptr Size = getSize(Ptr, &Header); | ||
if (DeleteSize && Options.get(OptionBit::DeleteSizeMismatch)) { | ||
if (UNLIKELY(DeleteSize != Size)) | ||
reportDeleteSizeMismatch(Ptr, DeleteSize, Size); | ||
switch (Origin) { | ||
case Chunk::Origin::New: | ||
FALLTHROUGH; | ||
case Chunk::Origin::NewArray: | ||
if (Options.get(OptionBit::DeleteSizeMismatch) && HasDeleteSize) { | ||
if (UNLIKELY(DeleteSize != Size)) | ||
reportDeleteSizeMismatch(Ptr, DeleteSize, Size); | ||
} | ||
if (Options.get(OptionBit::DeleteAlignmentMismatch) && | ||
HasDeleteAlignment) { | ||
if (UNLIKELY(!isAligned(reinterpret_cast<uptr>(Ptr), DeleteAlignment))) | ||
reportDeleteAlignmentMismatch(Ptr, DeleteAlignment); | ||
} | ||
break; | ||
case Chunk::Origin::Memalign: | ||
if (Options.get(OptionBit::FreeAlignmentMismatch) && HasDeleteAlignment) { | ||
if (UNLIKELY(!isAligned(reinterpret_cast<uptr>(Ptr), DeleteAlignment))) | ||
reportFreeAlignmentMismatch(Ptr, DeleteAlignment); | ||
} | ||
FALLTHROUGH; | ||
case Chunk::Origin::Malloc: | ||
if (Options.get(OptionBit::FreeSizeMismatch) && HasDeleteSize) { | ||
if (UNLIKELY(DeleteSize != Size)) | ||
reportFreeSizeMismatch(Ptr, DeleteSize, Size); | ||
} | ||
break; | ||
} | ||
|
||
quarantineOrDeallocateChunk(Options, TaggedPtr, &Header, Size); | ||
|
@@ -529,14 +561,20 @@ class Allocator { | |
if (UNLIKELY(Header.State != Chunk::State::Allocated)) | ||
reportInvalidChunkState(AllocatorAction::Reallocating, OldPtr); | ||
|
||
// Pointer has to be allocated with a malloc-type function. Some | ||
// applications think that it is OK to realloc a memalign'ed pointer, which | ||
// will trigger this check. It really isn't. | ||
if (Options.get(OptionBit::DeallocTypeMismatch)) { | ||
if (UNLIKELY(Header.OriginOrWasZeroed != Chunk::Origin::Malloc)) | ||
reportDeallocTypeMismatch(AllocatorAction::Reallocating, OldPtr, | ||
Header.OriginOrWasZeroed, | ||
Chunk::Origin::Malloc); | ||
// There is no language prohibiting the use of realloc with | ||
// aligned_alloc/posix_memalign/memalign and etc. The outcome in | ||
// practice is that the newly allocated memory will typically not | ||
// have the same alignment but will have minimum alignment. With | ||
// regards to operator new, there is no guarantee that the allocator | ||
// being used with malloc is the same as operator new. There is also | ||
// no guarantee that they share the same minimum alignment guarantees. | ||
// So we reject these. | ||
if (UNLIKELY(Header.OriginOrWasZeroed == Chunk::Origin::New || | ||
Header.OriginOrWasZeroed == Chunk::Origin::NewArray)) | ||
reportDeallocTypeMismatch( | ||
AllocatorAction::Reallocating, OldPtr, Header.OriginOrWasZeroed, | ||
Chunk::Origin::Malloc, /*HasDeleteSize=*/false); | ||
} | ||
|
||
void *BlockBegin = getBlockBegin(OldTaggedPtr, &Header); | ||
|
@@ -1746,6 +1784,19 @@ class Allocator { | |
return (Bytes - sizeof(AllocationRingBuffer)) / | ||
sizeof(typename AllocationRingBuffer::Entry); | ||
} | ||
|
||
static bool isOriginMismatch(Chunk::Origin Alloc, Chunk::Origin Dealloc, | ||
bool HasDeleteSize) { | ||
if (Alloc == Dealloc) { | ||
return false; | ||
} | ||
if (Alloc == Chunk::Origin::Memalign && Dealloc == Chunk::Origin::Malloc && | ||
!HasDeleteSize) { | ||
// aligned_alloc with free is allowed, but not free_sized. | ||
return false; | ||
} | ||
return true; | ||
} | ||
}; | ||
|
||
} // namespace scudo | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,3 +49,18 @@ SCUDO_FLAG(int, release_to_os_interval_ms, 5000, | |
SCUDO_FLAG(int, allocation_ring_buffer_size, 32768, | ||
"Entries to keep in the allocation ring buffer for scudo. " | ||
"Values less or equal to zero disable the buffer.") | ||
|
||
SCUDO_FLAG(bool, delete_alignment_mismatch, true, | ||
"Terminate on an alignment mismatch between a aligned-delete and " | ||
"the actual " | ||
"alignment of a chunk (as provided to new/new[]).") | ||
|
||
SCUDO_FLAG(bool, free_size_mismatch, true, | ||
"Terminate on a size mismatch between a free_sized and the actual " | ||
"size of a chunk (as provided to malloc/calloc/realloc).") | ||
|
||
SCUDO_FLAG(bool, free_alignment_mismatch, true, | ||
"Terminate on an alignment mismatch between a free_aligned_sized " | ||
"and the actual " | ||
"alignment of a chunk (as provided to " | ||
"aligned_alloc/posix_memalign/memalign).") | ||
Comment on lines
+53
to
+66
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we merge the two cases delete_alignment_mismatch and free_alignment_mismatch? Or is there a reason we need to distinguish them? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a separate patch
could you please
Move exiting deallocate into private as
and in public section replace
deallocate -> with inline "delete{,Sized}{,Aligned}"
and update existing calls as is?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#147735