[PATCH] D100911: scudo: Make prepareTaggedChunk() and resizeTaggedChunk() generic.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: compiler-rt/lib/scudo/standalone/combined.h:1146
+    uptr UntaggedNewPtr = untagPointer(RoundNewPtr);
+    if (UntaggedNewPtr != BlockEnd)
+      storeTag(UntaggedNewPtr);
----------------
vitalybuka wrote:
> vitalybuka wrote:
> > pcc wrote:
> > > eugenis wrote:
> > > > vitalybuka wrote:
> > > > > pcc wrote:
> > > > > > vitalybuka wrote:
> > > > > > > Why we don't tag entire unused tail? It may detect UseAfterFree.
> > > > > > We detect UAF by retagging on free. So from that perspective there's no advantage to retagging the tail here.
> > > > > then why do we need to remove a tag on a single granule here?
> > > > > To distinguish UAF and out of bounds?
> > > > For guaranteed linear overflow detection.
> > > > 
> > > > In fact, by not retagging the tail, we have guaranteed false negative for overflow by 16 bytes, right? Should we change this? It should be pretty cheap in the primary allocator.
> > > Hmm, I would expect the cost increase to correspond to the average ratio between size classes (i.e. around sqrt(2)).
> > > 
> > > Let's investigate doing that as part of a separate change. It should be easier to implement once we have the generic resizeTaggedChunk() implementation in.
> > So UAF is not guaranteed, but we want linear overflow guaranteed and without spending cycles to retag entire [RoundNewPtr, BlockEnd)?
> > Hmm, I would expect the cost increase to correspond to the average ratio between size classes (i.e. around sqrt(2)).
> > 
> > Let's investigate doing that as part of a separate change. It should be easier to implement once we have the generic resizeTaggedChunk() implementation in.
> 
> Agree, this is unchanged in the patch and better addressed (if needed) separately.
Immediate UAF is guaranteed due to retag on free -- that retags the entire previous allocation (of course, we don't know how large the next allocation to use that chunk will be).

Yes, the reason why we didn't retag that entire region with 0 was to save cycles.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100911/new/

https://reviews.llvm.org/D100911