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

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka added inline comments.


================
Comment at: compiler-rt/lib/scudo/standalone/combined.h:1146
+    uptr UntaggedNewPtr = untagPointer(RoundNewPtr);
+    if (UntaggedNewPtr != BlockEnd)
+      storeTag(UntaggedNewPtr);
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100911