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

[Christopher Ferris via llvm-commits]

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

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.

>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] [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);