[scudo] Fix secondary caching for mte

[cferris1000]

The current code always unmaps a secondary allocation when MTE is enabled. Fix this to match the comment, namely only unmap if MTE was enabled and is no longer enabled after acquiring the lock.

In addition, allow quaratine to work in the secondary even if MTE is not enabled.

[llvmbot]

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Christopher Ferris (cferris1000)

Changes

The current code always unmaps a secondary allocation when MTE is enabled. Fix this to match the comment, namely only unmap if MTE was enabled and is no longer enabled after acquiring the lock.

In addition, allow quaratine to work in the secondary even if MTE is not enabled.

---

Full diff: https://github.com/llvm/llvm-project/pull/150156.diff

1 Files Affected:

• (modified) compiler-rt/lib/scudo/standalone/secondary.h (+7-4)

diff --git a/compiler-rt/lib/scudo/standalone/secondary.h b/compiler-rt/lib/scudo/standalone/secondary.h
index 286e5d332f57c..f04c5b7cfc2ea 100644
--- a/compiler-rt/lib/scudo/standalone/secondary.h
+++ b/compiler-rt/lib/scudo/standalone/secondary.h
@@ -269,7 +269,8 @@ class MapAllocatorCache {
     Entry.MemMap = MemMap;
     Entry.Time = UINT64_MAX;
 
-    if (useMemoryTagging<Config>(Options)) {
+    bool MemoryTaggingEnabled = useMemoryTagging<Config>(Options);
+    if (MemoryTaggingEnabled) {
       if (Interval == 0 && !SCUDO_FUCHSIA) {
         // Release the memory and make it inaccessible at the same time by
         // creating a new MAP_NOACCESS mapping on top of the existing mapping.
@@ -302,7 +303,7 @@ class MapAllocatorCache {
       if (Entry.Time != 0)
         Entry.Time = Time;
 
-      if (useMemoryTagging<Config>(Options) && QuarantinePos == -1U) {
+      if (MemoryTaggingEnabled && !useMemoryTagging<Config>(Options)) {
         // If we get here then memory tagging was disabled in between when we
         // read Options and when we locked Mutex. We can't insert our entry into
         // the quarantine or the cache because the permissions would be wrong so
@@ -310,7 +311,8 @@ class MapAllocatorCache {
         unmapCallBack(Entry.MemMap);
         break;
       }
-      if (Config::getQuarantineSize() && useMemoryTagging<Config>(Options)) {
+
+      if (Config::getQuarantineSize()) {
         QuarantinePos =
             (QuarantinePos + 1) % Max(Config::getQuarantineSize(), 1u);
         if (!Quarantine[QuarantinePos].isValid()) {
@@ -513,9 +515,10 @@ class MapAllocatorCache {
         Quarantine[I].invalidate();
       }
     }
+    QuarantinePos = -1U;
+
     for (CachedBlock &Entry : LRUEntries)
       Entry.MemMap.setMemoryPermission(Entry.CommitBase, Entry.CommitSize, 0);
-    QuarantinePos = -1U;
   }
 
   void disable() NO_THREAD_SAFETY_ANALYSIS { Mutex.lock(); }

[cferris1000]

This fixes the issue from the other change.

[cferris1000]

I ran this through tests on Android and no issues. Both with MTE and without.

I also ran the tests on 32 bit.