Page MenuHomePhabricator

[scudo] Detect double free when running with MTE.
ClosedPublic

Authored by eugenis on Nov 3 2022, 1:24 PM.

Details

Summary

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

Diff Detail

Event Timeline

eugenis created this revision.Nov 3 2022, 1:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 3 2022, 1:24 PM
eugenis requested review of this revision.Nov 3 2022, 1:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 3 2022, 1:24 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript
pcc added inline comments.Nov 4 2022, 3:41 PM
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 added inline comments.Nov 7 2022, 11:02 AM
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.

pcc added inline comments.Nov 10 2022, 11:07 AM
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 updated this revision to Diff 475942.Nov 16 2022, 3:54 PM

expanded comments

eugenis marked an inline comment as done.Nov 16 2022, 3:54 PM
fmayer added a subscriber: fmayer.Nov 18 2022, 11:04 AM
fmayer added inline comments.
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 updated this revision to Diff 476573.Nov 18 2022, 12:46 PM

addressed Florian's comment

eugenis marked an inline comment as done.Nov 18 2022, 12:46 PM
fmayer accepted this revision.Nov 18 2022, 12:54 PM
This revision is now accepted and ready to land.Nov 18 2022, 12:54 PM
This revision was landed with ongoing or failed builds.Nov 18 2022, 1:18 PM
This revision was automatically updated to reflect the committed changes.