[PATCH] D87739: [WIP] scudo: Add an API for disabling memory initialization per-thread.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: compiler-rt/lib/scudo/standalone/combined.h:417
+            // therefore it needs to be zeroed now.
+            memset(TaggedPtr, 0, Min(Size, PrevEnd - TaggedUserPtr));
+          } else if (Size) {
----------------
eugenis wrote:
> eugenis wrote:
> > resizeTaggedChunk does not zero memory. I think this  memset needs to cover the entire allocation?
> Ah no, I misread. Ignore this.
I think the problem is related to this line actually. `resizeTaggedChunk` rounds up to 16 before it starts zeroing, so the memset needs to stop at the rounded-up original bound, otherwise it can leave the end of the original last granule uninitialized. I am testing this diff:
```
@@ -414,12 +424,18 @@ public:
             // UAF tag. But if tagging was disabled per-thread when the memory
             // was freed, it would not have been retagged and thus zeroed, and
             // therefore it needs to be zeroed now.
-            memset(TaggedPtr, 0, Min(Size, PrevEnd - TaggedUserPtr));
+            memset(TaggedPtr, 0,
+                   Min(Size, roundUpTo(PrevEnd - TaggedUserPtr,
+                                       archMemoryTagGranuleSize())));
           } else if (Size) {
             // Clear any stack metadata that may have previously been stored in
             // the chunk data.
```


================
Comment at: compiler-rt/lib/scudo/standalone/combined.h:426
               computeOddEvenMaskForPointerMaybe(BlockUptr, BlockSize);
           TaggedPtr = prepareTaggedChunk(Ptr, Size, OddEvenMask, BlockEnd);
         }
----------------
eugenis wrote:
> Rename setRandomTag to setRandomTagAndZeroData ?
Does that really need to be in the name of the function or would a comment be enough? I think most of the functions in memtag.h end up zeroing parts of the memory that they touch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87739