[compiler-rt] [scudo] Mitigate commitbase exploit (PR #75295)

via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 12 23:55:17 PST 2023


github-actions[bot] wrote:

<!--LLVM CODE FORMAT COMMENT: {clang-format}-->


:warning: C/C++ code formatter, clang-format found issues in your code. :warning:

<details>
<summary>
You can test this locally with the following command:
</summary>

``````````bash
git-clang-format --diff 912506b75d8a508353a701c230e73ca45a253161 70f5ca3fb598137b4300f277c9c0bbda5929ba1e -- compiler-rt/lib/scudo/standalone/allocator_config.h compiler-rt/lib/scudo/standalone/secondary.h
``````````

</details>

<details>
<summary>
View the diff from clang-format here.
</summary>

``````````diff
diff --git a/compiler-rt/lib/scudo/standalone/allocator_config.h b/compiler-rt/lib/scudo/standalone/allocator_config.h
index b796b07b26..8a9df63912 100644
--- a/compiler-rt/lib/scudo/standalone/allocator_config.h
+++ b/compiler-rt/lib/scudo/standalone/allocator_config.h
@@ -153,8 +153,8 @@ struct DefaultConfig {
       static const s32 MinReleaseToOsIntervalMs = INT32_MIN;
       static const s32 MaxReleaseToOsIntervalMs = INT32_MAX;
     };
-      static const bool VerifyInUseAddresses = true;
-      static const u32 InUseBlocksSize = 1000U;
+    static const bool VerifyInUseAddresses = true;
+    static const u32 InUseBlocksSize = 1000U;
     template <typename Config> using CacheT = MapAllocatorCache<Config>;
   };
 
diff --git a/compiler-rt/lib/scudo/standalone/secondary.h b/compiler-rt/lib/scudo/standalone/secondary.h
index 202dfd5897..d7173aebbf 100644
--- a/compiler-rt/lib/scudo/standalone/secondary.h
+++ b/compiler-rt/lib/scudo/standalone/secondary.h
@@ -479,15 +479,21 @@ public:
     Stats.init();
 
     if (Config::Secondary::VerifyInUseAddresses) {
-        ReservedMemoryT InUseReserved;
-        InUseReserved.create(0U, sizeof(void *)*(Config::Secondary::InUseBlocksSize+1), "scudo:secondary_integrity");
-        DCHECK_NE(InUseReserved.getBase(), 0U);
-        
-        InUseAddresses = InUseReserved.dispatch(InUseReserved.getBase(), sizeof(void *)*(Config::Secondary::InUseBlocksSize+1));
-        CHECK(InUseAddresses.isAllocated());
-        InUseAddresses.setMemoryPermission(InUseAddresses.getBase(), sizeof(void *)*(Config::Secondary::InUseBlocksSize+1), 0);
+      ReservedMemoryT InUseReserved;
+      InUseReserved.create(
+          0U, sizeof(void *) * (Config::Secondary::InUseBlocksSize + 1),
+          "scudo:secondary_integrity");
+      DCHECK_NE(InUseReserved.getBase(), 0U);
+
+      InUseAddresses = InUseReserved.dispatch(
+          InUseReserved.getBase(),
+          sizeof(void *) * (Config::Secondary::InUseBlocksSize + 1));
+      CHECK(InUseAddresses.isAllocated());
+      InUseAddresses.setMemoryPermission(
+          InUseAddresses.getBase(),
+          sizeof(void *) * (Config::Secondary::InUseBlocksSize + 1), 0);
     }
-    
+
     if (LIKELY(S))
       S->link(&Stats);
   }
@@ -558,7 +564,6 @@ private:
   LocalStats Stats GUARDED_BY(Mutex);
 
   MemMapT InUseAddresses GUARDED_BY(Mutex) = {};
-
 };
 
 // As with the Primary, the size passed to this function includes any desired
@@ -605,41 +610,52 @@ void *MapAllocator<Config>::allocate(const Options &Options, uptr Size,
         ScopedLock L(Mutex);
 
         if (Config::Secondary::VerifyInUseAddresses) {
-            void **IntegrityList = reinterpret_cast<void **>(InUseAddresses.getBase());
-            bool isFull = true;
-            bool reachedEnd = false;
-            
-            while (!reachedEnd) {
-                for (u32 I = 0; I < Config::Secondary::InUseBlocksSize; I++) {
-                    if (IntegrityList[I] == nullptr) {
-                        isFull = false;
-                        IntegrityList[I] = Ptr;
-                        break;
-                    }
-                }
-                if (isFull && IntegrityList[Config::Secondary::InUseBlocksSize] != nullptr) {
-                    IntegrityList = static_cast<void **>(IntegrityList[Config::Secondary::InUseBlocksSize]);
-                } else {
-                    reachedEnd = true;
-                }
+          void **IntegrityList =
+              reinterpret_cast<void **>(InUseAddresses.getBase());
+          bool isFull = true;
+          bool reachedEnd = false;
+
+          while (!reachedEnd) {
+            for (u32 I = 0; I < Config::Secondary::InUseBlocksSize; I++) {
+              if (IntegrityList[I] == nullptr) {
+                isFull = false;
+                IntegrityList[I] = Ptr;
+                break;
+              }
             }
-            
-            if (isFull) {
-                ReservedMemoryT InUseReserved;
-                InUseReserved.create(0U, sizeof(void *)*(Config::Secondary::InUseBlocksSize+1), "scudo:secondary_integrity");
-                DCHECK_NE(InUseReserved.getBase(), 0U);
-                
-                MemMapT NewAddresses = InUseReserved.dispatch(InUseReserved.getBase(), sizeof(void *)*(Config::Secondary::InUseBlocksSize+1));
-                CHECK(NewAddresses.isAllocated());
-                NewAddresses.setMemoryPermission(NewAddresses.getBase(), sizeof(void *)*(Config::Secondary::InUseBlocksSize+1), 0);
-                
-                IntegrityList[Config::Secondary::InUseBlocksSize] = reinterpret_cast<void *>(NewAddresses.getBase());
-                
-                IntegrityList = static_cast<void **>(IntegrityList[Config::Secondary::InUseBlocksSize]);
-                IntegrityList[0] = Ptr;
+            if (isFull &&
+                IntegrityList[Config::Secondary::InUseBlocksSize] != nullptr) {
+              IntegrityList = static_cast<void **>(
+                  IntegrityList[Config::Secondary::InUseBlocksSize]);
+            } else {
+              reachedEnd = true;
             }
+          }
+
+          if (isFull) {
+            ReservedMemoryT InUseReserved;
+            InUseReserved.create(
+                0U, sizeof(void *) * (Config::Secondary::InUseBlocksSize + 1),
+                "scudo:secondary_integrity");
+            DCHECK_NE(InUseReserved.getBase(), 0U);
+
+            MemMapT NewAddresses = InUseReserved.dispatch(
+                InUseReserved.getBase(),
+                sizeof(void *) * (Config::Secondary::InUseBlocksSize + 1));
+            CHECK(NewAddresses.isAllocated());
+            NewAddresses.setMemoryPermission(
+                NewAddresses.getBase(),
+                sizeof(void *) * (Config::Secondary::InUseBlocksSize + 1), 0);
+
+            IntegrityList[Config::Secondary::InUseBlocksSize] =
+                reinterpret_cast<void *>(NewAddresses.getBase());
+
+            IntegrityList = static_cast<void **>(
+                IntegrityList[Config::Secondary::InUseBlocksSize]);
+            IntegrityList[0] = Ptr;
+          }
         }
-      
+
         InUseBlocks.push_back(H);
         AllocatedBytes += H->CommitSize;
         FragmentedBytes += H->MemMap.getCapacity() - H->CommitSize;
@@ -716,39 +732,52 @@ void *MapAllocator<Config>::allocate(const Options &Options, uptr Size,
     ScopedLock L(Mutex);
 
     if (Config::Secondary::VerifyInUseAddresses) {
-        void **IntegrityList = reinterpret_cast<void **>(InUseAddresses.getBase());
-        bool isFull = true;
-        bool reachedEnd = false;
-        
-        while (!reachedEnd) {
-            for (u32 I = 0; I < Config::Secondary::InUseBlocksSize; I++) {
-                if (IntegrityList[I] == nullptr) {
-                    isFull = false;
-                    IntegrityList[I] = reinterpret_cast<void *>(HeaderPos + LargeBlock::getHeaderSize());
-                    break;
-                }
-            }
-            if (isFull && IntegrityList[Config::Secondary::InUseBlocksSize] != nullptr) {
-                IntegrityList = static_cast<void **>(IntegrityList[Config::Secondary::InUseBlocksSize]);
-            } else {
-                reachedEnd = true;
-            }
+      void **IntegrityList =
+          reinterpret_cast<void **>(InUseAddresses.getBase());
+      bool isFull = true;
+      bool reachedEnd = false;
+
+      while (!reachedEnd) {
+        for (u32 I = 0; I < Config::Secondary::InUseBlocksSize; I++) {
+          if (IntegrityList[I] == nullptr) {
+            isFull = false;
+            IntegrityList[I] = reinterpret_cast<void *>(
+                HeaderPos + LargeBlock::getHeaderSize());
+            break;
+          }
         }
-        
-        if (isFull) {
-            ReservedMemoryT InUseReserved;
-            InUseReserved.create(0U, sizeof(void *)*(Config::Secondary::InUseBlocksSize+1), "scudo:secondary_integrity");
-            DCHECK_NE(InUseReserved.getBase(), 0U);
-            
-            MemMapT NewAddresses = InUseReserved.dispatch(InUseReserved.getBase(), sizeof(void *)*(Config::Secondary::InUseBlocksSize+1));
-            CHECK(NewAddresses.isAllocated());
-            NewAddresses.setMemoryPermission(NewAddresses.getBase(), sizeof(void *)*(Config::Secondary::InUseBlocksSize+1), 0);
-            
-            IntegrityList[Config::Secondary::InUseBlocksSize] = reinterpret_cast<void *>(NewAddresses.getBase());
-            
-            IntegrityList = static_cast<void **>(IntegrityList[Config::Secondary::InUseBlocksSize]);
-            IntegrityList[0] = reinterpret_cast<void *>(HeaderPos + LargeBlock::getHeaderSize());
+        if (isFull &&
+            IntegrityList[Config::Secondary::InUseBlocksSize] != nullptr) {
+          IntegrityList = static_cast<void **>(
+              IntegrityList[Config::Secondary::InUseBlocksSize]);
+        } else {
+          reachedEnd = true;
         }
+      }
+
+      if (isFull) {
+        ReservedMemoryT InUseReserved;
+        InUseReserved.create(
+            0U, sizeof(void *) * (Config::Secondary::InUseBlocksSize + 1),
+            "scudo:secondary_integrity");
+        DCHECK_NE(InUseReserved.getBase(), 0U);
+
+        MemMapT NewAddresses = InUseReserved.dispatch(
+            InUseReserved.getBase(),
+            sizeof(void *) * (Config::Secondary::InUseBlocksSize + 1));
+        CHECK(NewAddresses.isAllocated());
+        NewAddresses.setMemoryPermission(
+            NewAddresses.getBase(),
+            sizeof(void *) * (Config::Secondary::InUseBlocksSize + 1), 0);
+
+        IntegrityList[Config::Secondary::InUseBlocksSize] =
+            reinterpret_cast<void *>(NewAddresses.getBase());
+
+        IntegrityList = static_cast<void **>(
+            IntegrityList[Config::Secondary::InUseBlocksSize]);
+        IntegrityList[0] =
+            reinterpret_cast<void *>(HeaderPos + LargeBlock::getHeaderSize());
+      }
     }
 
     InUseBlocks.push_back(H);
@@ -771,32 +800,34 @@ void MapAllocator<Config>::deallocate(const Options &Options, void *Ptr)
   {
     ScopedLock L(Mutex);
 
-
     if (Config::Secondary::VerifyInUseAddresses) {
-        void **IntegrityList = reinterpret_cast<void **>(InUseAddresses.getBase());
-        bool isValid = false;
-        bool reachedEnd = false;
-        
-        while (!reachedEnd) {
-            for (u32 I = 0; I < Config::Secondary::InUseBlocksSize; I++) {
-                if (IntegrityList[I] == Ptr) {
-                    isValid = true;
-                    IntegrityList[I] = nullptr;
-                    break;
-                }
-            }
-            if (!isValid && IntegrityList[Config::Secondary::InUseBlocksSize] != nullptr) {
-                IntegrityList = static_cast<void **>(IntegrityList[Config::Secondary::InUseBlocksSize]);
-            } else {
-                reachedEnd = true;
-            }
+      void **IntegrityList =
+          reinterpret_cast<void **>(InUseAddresses.getBase());
+      bool isValid = false;
+      bool reachedEnd = false;
+
+      while (!reachedEnd) {
+        for (u32 I = 0; I < Config::Secondary::InUseBlocksSize; I++) {
+          if (IntegrityList[I] == Ptr) {
+            isValid = true;
+            IntegrityList[I] = nullptr;
+            break;
+          }
         }
-        
-        if (!isValid) {
-            return;
+        if (!isValid &&
+            IntegrityList[Config::Secondary::InUseBlocksSize] != nullptr) {
+          IntegrityList = static_cast<void **>(
+              IntegrityList[Config::Secondary::InUseBlocksSize]);
+        } else {
+          reachedEnd = true;
         }
+      }
+
+      if (!isValid) {
+        return;
+      }
     }
-    
+
     InUseBlocks.remove(H);
     FreedBytes += CommitSize;
     FragmentedBytes -= H->MemMap.getCapacity() - CommitSize;

``````````

</details>


https://github.com/llvm/llvm-project/pull/75295


More information about the llvm-commits mailing list