[compiler-rt] [scudo] Add flags to secondary cache entries. (PR #177506)

[Christopher Ferris via llvm-commits]

https://github.com/cferris1000 updated https://github.com/llvm/llvm-project/pull/177506

>From e8e4be0fe7085558ba482f5a46c4ab07cc558574 Mon Sep 17 00:00:00 2001
From: Christopher Ferris <cferris at google.com>
Date: Thu, 22 Jan 2026 16:28:54 -0800
Subject: [PATCH 1/2] [scudo] Add flags to secondary cache entries.

Instead of setting a cache entry back to read-write based on whether
MTE is enabled, add a flag that indicates if this entry was
made inaccessible. On Android, we have seen strange crashes were
MTE is disabled for a process, but the cache entry is
inaccessible, causing an access error. This indicates MTE
was enabled at some point, then disabled again. There is
likely some race, or method to disable without making
inaccessible entries read-writable again. Adding the flags
should avoid this happening in the future.

Remove the resetting of the secondary cache entries when
disabling MTE since it can be done as necessary.

This change will also allow us to always make the secondary
cache entries inaccessible in the future to increase security
measures even when MTE is not enabled.

Current unit tests already cover this change.
---
 compiler-rt/lib/scudo/standalone/secondary.h | 35 ++++++++++++--------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/compiler-rt/lib/scudo/standalone/secondary.h b/compiler-rt/lib/scudo/standalone/secondary.h
index 04e33c04baa34..c2c6ebc337608 100644
--- a/compiler-rt/lib/scudo/standalone/secondary.h
+++ b/compiler-rt/lib/scudo/standalone/secondary.h
@@ -100,6 +100,12 @@ struct CachedBlock {
   u16 Next = 0;
   u16 Prev = 0;
 
+  enum CacheFlags : u16 {
+    None = 0,
+    NoAccess = 0x1,
+  };
+  CacheFlags Flags = CachedBlock::None;
+
   bool isValid() { return CommitBase != 0; }
 
   void invalidate() { CommitBase = 0; }
@@ -299,7 +305,9 @@ class MapAllocatorCache {
         Entry.MemMap.setMemoryPermission(Entry.CommitBase, Entry.CommitSize,
                                          MAP_NOACCESS);
       }
-    }
+      Entry.Flags = CachedBlock::NoAccess;
+    } else
+      Entry.Flags = CachedBlock::None;
 
     // Usually only one entry will be evicted from the cache.
     // Only in the rare event that the cache shrinks in real-time
@@ -522,20 +530,18 @@ class MapAllocatorCache {
   }
 
   void disableMemoryTagging() EXCLUDES(Mutex) {
+    if (Config::getQuarantineDisabled())
+      return;
+
     ScopedLock L(Mutex);
-    if (!Config::getQuarantineDisabled()) {
-      for (u32 I = 0; I != Config::getQuarantineSize(); ++I) {
-        if (Quarantine[I].isValid()) {
-          MemMapT &MemMap = Quarantine[I].MemMap;
-          unmapCallBack(MemMap);
-          Quarantine[I].invalidate();
-        }
+    for (u32 I = 0; I != Config::getQuarantineSize(); ++I) {
+      if (Quarantine[I].isValid()) {
+        MemMapT &MemMap = Quarantine[I].MemMap;
+        unmapCallBack(MemMap);
+        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(); }
@@ -754,9 +760,12 @@ MapAllocator<Config>::tryAllocateFromCache(const Options &Options, uptr Size,
   LargeBlock::Header *H = reinterpret_cast<LargeBlock::Header *>(
       LargeBlock::addHeaderTag<Config>(EntryHeaderPos));
   bool Zeroed = Entry.Time == 0;
+
+  if (UNLIKELY(Entry.Flags & CachedBlock::NoAccess))
+    Entry.MemMap.setMemoryPermission(Entry.CommitBase, Entry.CommitSize, 0);
+
   if (useMemoryTagging<Config>(Options)) {
     uptr NewBlockBegin = reinterpret_cast<uptr>(H + 1);
-    Entry.MemMap.setMemoryPermission(Entry.CommitBase, Entry.CommitSize, 0);
     if (Zeroed) {
       storeTags(LargeBlock::addHeaderTag<Config>(Entry.CommitBase),
                 NewBlockBegin);

>From 6f9a0f3dff11e47a4057b9b71f606b47c659277c Mon Sep 17 00:00:00 2001
From: Christopher Ferris <cferris at google.com>
Date: Fri, 23 Jan 2026 11:11:50 -0800
Subject: [PATCH 2/2] Always init the Flags field.

Add a comment about setting the permissions.

Change from feedback.
---
 compiler-rt/lib/scudo/standalone/secondary.h | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/compiler-rt/lib/scudo/standalone/secondary.h b/compiler-rt/lib/scudo/standalone/secondary.h
index c2c6ebc337608..bb321104ee6df 100644
--- a/compiler-rt/lib/scudo/standalone/secondary.h
+++ b/compiler-rt/lib/scudo/standalone/secondary.h
@@ -290,6 +290,7 @@ class MapAllocatorCache {
     Entry.BlockBegin = BlockBegin;
     Entry.MemMap = MemMap;
     Entry.Time = UINT64_MAX;
+    Entry.Flags = CachedBlock::None;
 
     bool MemoryTaggingEnabled = useMemoryTagging<Config>(Options);
     if (MemoryTaggingEnabled) {
@@ -306,8 +307,7 @@ class MapAllocatorCache {
                                          MAP_NOACCESS);
       }
       Entry.Flags = CachedBlock::NoAccess;
-    } else
-      Entry.Flags = CachedBlock::None;
+    }
 
     // Usually only one entry will be evicted from the cache.
     // Only in the rare event that the cache shrinks in real-time
@@ -761,8 +761,11 @@ MapAllocator<Config>::tryAllocateFromCache(const Options &Options, uptr Size,
       LargeBlock::addHeaderTag<Config>(EntryHeaderPos));
   bool Zeroed = Entry.Time == 0;
 
-  if (UNLIKELY(Entry.Flags & CachedBlock::NoAccess))
-    Entry.MemMap.setMemoryPermission(Entry.CommitBase, Entry.CommitSize, 0);
+  if (UNLIKELY(Entry.Flags & CachedBlock::NoAccess)) {
+    // NOTE: Flags set to 0 actually restores read-write.
+    Entry.MemMap.setMemoryPermission(Entry.CommitBase, Entry.CommitSize,
+                                     /*Flags=*/0);
+  }
 
   if (useMemoryTagging<Config>(Options)) {
     uptr NewBlockBegin = reinterpret_cast<uptr>(H + 1);