[compiler-rt] [scudo] Mitigate commitbase exploit (PR #75295)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 15 00:38:22 PST 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-compiler-rt-sanitizer
Author: None (SIELA1915)
<details>
<summary>Changes</summary>
This is a proposed mitigation for an attack which overwrites the commit base of a fake secondary chunk. This fake secondary chunk is forged by an attacker who has successfully leaked the scudo cookie.
When this fake secondary chunk is freed, the chunk is put into the secondary cache, and when it is retrieved again from the cache a chunk will be allocated at the CommitBase location, which the attacker is able to freely choose.
This mitigation tracks in a separate memory region the addresses of all currently allocated secondary chunks and only adds free'd secondary chunks to the cache if they were recorded in this dedicated memory region.
The tracking in this mitigation happens through a simple list of addresses, which is looped over at allocation/deallocation. Each block of contiguous memory has a at compile time configurable number of addresses stored, as well as one additional address pointing to the next block in case there are more addresses.
Two options where added in the Allocator Config, `InUseBlocksSize` which specifies the number of header addresses which are saved in a contiguous memory region and `VerifyInUseAddresses` to completely disable this mitigation.
Performance impact was tested using some custom benchmarks, which are available at [https://github.com/SIELA1915/house-of-scudo/tree/master/benchmark](https://github.com/SIELA1915/house-of-scudo/tree/master/benchmark).
| benchmark name | runtime (in s) | with mitigation (in s) |
|--------------------------|----------------|------------------------|
| single-block-used | 5.8379 | 5.8170 |
| many-blocks-used | 7.0976 | 9.8458 |
| random-blocks-order | 3.8206 | 4.38057 |
| random-half-blocks-order | 3.7602 | 4.5997 |
| many-many-blocks-used | 6.4697 | 9.487 |
While in the worst case (many secondary allocations with many secondary chunks being allocated at the same time) the performance impact is quite high, in usual usage it should not matter match. This seems to also be confirmed by the following benchmark tests of [mimalloc-bench](https://github.com/daanx/mimalloc-bench), where scudo is the normal version and scudo_fixed the one with the mitigation (values are averages of five runs for each):
| test | alloc | time | rss | user | sys | page-faults | page-reclaims|
|----------------|-------------|---------|--------|-------|------|-------------|--------------|
| cfrac | sys | 8.49 | 3171 | 8.49 | 0.0 | 0 | 434 |
| cfrac | scudo_fixed | 10.03 | 4645 | 10.03 | 0.0 | 0 | 612 |
| cfrac | scudo| 9.96 | 4672 | 9.96 | 0.0 | 0 | 611 |
| espresso | sys | 6.36 | 2540 | 6.35 | 0.01 | 1 | 473 |
| espresso | scudo_fixed | 7.05 | 4852 | 7.03 | 0.01 | 0 | 659 |
| espresso | scudo | 7.02 | 4860 | 7.0 | 0.02 | 0 | 654 |
| barnes | sys | 3.57 | 58405 | 3.54 | 0.03 | 0 | 16570 |
| barnes | scudo_fixed | 3.42 | 59704 | 3.39 | 0.03 | 0 | 16645 |
| barnes | scudo | 3.77 | 59739 | 3.75 | 0.02 | 0 | 16645 |
| redis | sys | 9.26 | 7929 | 0.42 | 0.04 | 3 | 1240 |
| redis | scudo_fixed | 9.81 | 9587 | 0.44 | 0.05 | 0 | 1482 |
| redis | scudo | 8.85 | 9648 | 0.4 | 0.06 | 0 | 1480 |
| larsonN-sized | sys | 3.0 | 374957 | 257.39 | 4.62 | 0 | 106747 |
| larsonN-sized | scudo_fixed | 128.05 | 168812 | 94.15 | 152.25 | 15 | 82785 |
| larsonN-sized | scudo | 129.12 | 168765 | 94.66 | 150.81 | 20 | 82673 |
| mstressN | sys | 5.05 | 1526414 | 37.09 | 20.58 | 0 | 3288327 |
| mstressN | scudo_fixed | 13.81 | 803629 | 119.19 | 108.42 | 0 | 6181076 |
| mstressN | scudo | 13.7 | 800176 | 115.88 | 105.06 | 0 | 6171572 |
| rptestN | sys | 4.64 | 162499 | 28.06 | 10.43 | 2 | 87482 |
| rptestN | scudo_fixed | 22.79 | 143832 | 60.92 | 40.67 | 3 | 528002 |
| rptestN | scudo | 21.94 | 145304 | 58.56 | 39.31 | 0 | 526599 |
| lua | sys | 8.45 | 61516 | 7.81 | 0.57 | 179 | 144312 |
| lua | scudo_fixed | 8.44 | 71195 | 7.76 | 0.68 | 2 | 179348 |
| lua | scudo | 8.52 | 71162 | 7.8 | 0.71 | 0 | 179306 |
| alloc-test1 | sys | 5.44 | 13859 | 5.43 | 0.01 | 0 | 9342 |
| alloc-test1 | scudo_fixed | 5.87 | 15688 | 5.85 | 0.01 | 0 | 11476 |
| alloc-test1 | scudo | 5.73 | 15740 | 5.72 | 0.01 | 0 | 10898 |
| alloc-testN | sys | 6.12 | 17233 | 87.82 | 0.04 | 7 | 11993 |
| alloc-testN | scudo_fixed | 10.77 | 17896 | 168.24 | 0.06 | 7 | 16705 |
| alloc-testN | scudo | 12.23 | 17858 | 192.0 | 0.06 | 6 | 19608 |
| sh6benchN | sys | 0.76 | 335016 | 14.53 | 1.21 | 0 | 83493 |
| sh6benchN | scudo_fixed | 39.32 | 371601 | 448.45 | 175.71 | 17 | 137682 |
| sh6benchN | scudo | 38.37 | 371660 | 450.64 | 179.58 | 15 | 137678 |
| sh8benchN | sys | 1.47 | 424444 | 47.6 | 0.45 | 0 | 110235 |
| sh8benchN | scudo_fixed | 94.2 | 277936 | 749.79 | 4333.51 | 17 | 220871 |
| sh8benchN | scudo | 95.0 | 278531 | 768.26 | 4356.15 | 25 | 223608 |
| xmalloc-testN | sys | 1.02 | 126016 | 256.71 | 1.76 | 11 | 109729 |
| xmalloc-testN | scudo_fixed | 85.24 | 88415 | 23.92 | 191.14 | 0 | 45575 |
| xmalloc-testN | scudo | 82.89 | 88232 | 23.99 | 196.19 | 0 | 50801 |
| cache-scratch1 | sys | 1.82 | 3692 | 1.82 | 0.0 | 0 | 239 |
| cache-scratch1 | scudo_fixed | 1.94 | 3909 | 1.94 | 0.0 | 0 | 276 |
| cache-scratch1 | scudo | 1.88 | 3871 | 1.87 | 0.0 | 0 | 273 |
| cache-scratchN | sys | 0.09 | 4001 | 4.49 | 0.0 | 0 | 392 |
| cache-scratchN | scudo_fixed | 0.1 | 4606 | 4.64 | 0.0 | 0 | 527 |
| cache-scratchN | scudo | 0.1 | 4685 | 4.74 | 0.0 | 0 | 530 |
| glibc-simple | sys | 5.43 | 1977 | 5.43 | 0.0 | 0 | 216 |
| glibc-simple | scudo_fixed | 7.14 | 3623 | 7.13 | 0.0 | 0 | 365 |
| glibc-simple | scudo | 7.5 | 3696 | 7.5 | 0.0 | 0 | 380 |
| glibc-thread | sys | 0.68 | 12809 | 109.56 | 0.03 | 70 | 4955 |
| glibc-thread | scudo_fixed | 2.66 | 15507 | 109.55 | 0.04 | 9 | 4254 |
| glibc-thread | scudo | 2.9 | 15431 | 109.55 | 0.05 | 15 | 4257 |
---
Full diff: https://github.com/llvm/llvm-project/pull/75295.diff
4 Files Affected:
- (modified) compiler-rt/lib/scudo/standalone/allocator_config.h (+8)
- (modified) compiler-rt/lib/scudo/standalone/secondary.h (+147)
- (modified) compiler-rt/lib/scudo/standalone/tests/combined_test.cpp (+4)
- (modified) compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp (+4)
``````````diff
diff --git a/compiler-rt/lib/scudo/standalone/allocator_config.h b/compiler-rt/lib/scudo/standalone/allocator_config.h
index 3c6aa3acb0e45b..02d0b4e0e5d7ba 100644
--- a/compiler-rt/lib/scudo/standalone/allocator_config.h
+++ b/compiler-rt/lib/scudo/standalone/allocator_config.h
@@ -153,6 +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;
template <typename Config> using CacheT = MapAllocatorCache<Config>;
};
@@ -198,6 +200,8 @@ struct AndroidConfig {
static const s32 MinReleaseToOsIntervalMs = 0;
static const s32 MaxReleaseToOsIntervalMs = 1000;
};
+ static const bool VerifyInUseAddresses = true;
+ static const u32 InUseBlocksSize = 1000U;
template <typename Config> using CacheT = MapAllocatorCache<Config>;
};
@@ -230,6 +234,8 @@ struct FuchsiaConfig {
template <typename Config> using PrimaryT = SizeClassAllocator64<Config>;
struct Secondary {
+ static const bool VerifyInUseAddresses = true;
+ static const u32 InUseBlocksSize = 1000U;
template <typename Config> using CacheT = MapAllocatorNoCache<Config>;
};
template <typename Config> using SecondaryT = MapAllocator<Config>;
@@ -254,6 +260,8 @@ struct TrustyConfig {
template <typename Config> using PrimaryT = SizeClassAllocator64<Config>;
struct Secondary {
+ static const bool VerifyInUseAddresses = true;
+ static const u32 InUseBlocksSize = 1000U;
template <typename Config> using CacheT = MapAllocatorNoCache<Config>;
};
diff --git a/compiler-rt/lib/scudo/standalone/secondary.h b/compiler-rt/lib/scudo/standalone/secondary.h
index f52a4188bcf3a6..d7173aebbf0496 100644
--- a/compiler-rt/lib/scudo/standalone/secondary.h
+++ b/compiler-rt/lib/scudo/standalone/secondary.h
@@ -11,6 +11,7 @@
#include "chunk.h"
#include "common.h"
+#include "internal_defs.h"
#include "list.h"
#include "mem_map.h"
#include "memtag.h"
@@ -476,6 +477,23 @@ template <typename Config> class MapAllocator {
DCHECK_EQ(FreedBytes, 0U);
Cache.init(ReleaseToOsInterval);
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);
+ }
+
if (LIKELY(S))
S->link(&Stats);
}
@@ -544,6 +562,8 @@ template <typename Config> class MapAllocator {
u32 NumberOfAllocs GUARDED_BY(Mutex) = 0;
u32 NumberOfFrees GUARDED_BY(Mutex) = 0;
LocalStats Stats GUARDED_BY(Mutex);
+
+ MemMapT InUseAddresses GUARDED_BY(Mutex) = {};
};
// As with the Primary, the size passed to this function includes any desired
@@ -588,6 +608,54 @@ void *MapAllocator<Config>::allocate(const Options &Options, uptr Size,
BlockEnd - PtrInt);
{
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;
+ }
+ }
+
+ 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;
@@ -662,6 +730,56 @@ void *MapAllocator<Config>::allocate(const Options &Options, uptr Size,
*BlockEndPtr = CommitBase + CommitSize;
{
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;
+ }
+ }
+
+ 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);
AllocatedBytes += CommitSize;
FragmentedBytes += H->MemMap.getCapacity() - CommitSize;
@@ -681,6 +799,35 @@ void MapAllocator<Config>::deallocate(const Options &Options, void *Ptr)
const uptr CommitSize = H->CommitSize;
{
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;
+ }
+ }
+
+ if (!isValid) {
+ return;
+ }
+ }
+
InUseBlocks.remove(H);
FreedBytes += CommitSize;
FragmentedBytes -= H->MemMap.getCapacity() - CommitSize;
diff --git a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
index 3dbd93cacefd68..721e524b07b824 100644
--- a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
@@ -205,6 +205,8 @@ struct TestConditionVariableConfig {
#endif
struct Secondary {
+ static const bool VerifyInUseAddresses = true;
+ static const scudo::u32 InUseBlocksSize = 1000U;
template <typename Config>
using CacheT = scudo::MapAllocatorNoCache<Config>;
};
@@ -678,6 +680,8 @@ struct DeathConfig {
using PrimaryT = scudo::SizeClassAllocator64<Config>;
struct Secondary {
+ static const bool VerifyInUseAddresses = true;
+ static const scudo::u32 InUseBlocksSize = 1000U;
template <typename Config>
using CacheT = scudo::MapAllocatorNoCache<Config>;
};
diff --git a/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp b/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp
index 18d2e187fa3ce2..a5485b0e117707 100644
--- a/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp
@@ -85,6 +85,8 @@ template <typename Config> static void testSecondaryBasic(void) {
struct NoCacheConfig {
static const bool MaySupportMemoryTagging = false;
struct Secondary {
+ static const bool VerifyInUseAddresses = true;
+ static const scudo::u32 InUseBlocksSize = 1000U;
template <typename Config>
using CacheT = scudo::MapAllocatorNoCache<Config>;
};
@@ -102,6 +104,8 @@ struct TestConfig {
static const scudo::s32 MaxReleaseToOsIntervalMs = INT32_MAX;
};
+ static const bool VerifyInUseAddresses = true;
+ static const scudo::u32 InUseBlocksSize = 1000U;
template <typename Config> using CacheT = scudo::MapAllocatorCache<Config>;
};
};
``````````
</details>
https://github.com/llvm/llvm-project/pull/75295
More information about the llvm-commits
mailing list