From 6d3a4adc47d0e44e556c27d1b836cbd27e140cf2 Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Fri, 29 Mar 2024 09:44:17 -0700 Subject: [scudo] Only init RingBuffer when needed. (#85994) Only attempt to initialize the ring buffer when tracking is enabled. Updated unit tests, and added a few new unit tests to verify the RingBuffer is not initialized by default. Verified that the two maps associated with the RingBuffer are not created in processes by default. GitOrigin-RevId: 0dbd804a690720688d8234d8bdaee8f8f4fdcddc Change-Id: I52058304a671079f549a9e02d34ba97baa3fa92a --- standalone/combined.h | 31 ++++++++++----- standalone/tests/combined_test.cpp | 78 ++++++++++++++++++++++++++++++++------ 2 files changed, 87 insertions(+), 22 deletions(-) diff --git a/standalone/combined.h b/standalone/combined.h index f4dd90aac66..e7bc90cd096 100644 --- a/standalone/combined.h +++ b/standalone/combined.h @@ -18,6 +18,7 @@ #include "local_cache.h" #include "mem_map.h" #include "memtag.h" +#include "mutex.h" #include "options.h" #include "quarantine.h" #include "report.h" @@ -178,17 +179,17 @@ public: Quarantine.init( static_cast(getFlags()->quarantine_size_kb << 10), static_cast(getFlags()->thread_local_quarantine_size_kb << 10)); - - mapAndInitializeRingBuffer(); } - void enableRingBuffer() { + void enableRingBuffer() NO_THREAD_SAFETY_ANALYSIS { AllocationRingBuffer *RB = getRingBuffer(); if (RB) RB->Depot->enable(); + RingBufferInitLock.unlock(); } - void disableRingBuffer() { + void disableRingBuffer() NO_THREAD_SAFETY_ANALYSIS { + RingBufferInitLock.lock(); AllocationRingBuffer *RB = getRingBuffer(); if (RB) RB->Depot->disable(); @@ -915,9 +916,11 @@ public: DCHECK(!Primary.Options.load().get(OptionBit::TrackAllocationStacks)); return; } - if (Track) + + if (Track) { + initRingBufferMaybe(); Primary.Options.set(OptionBit::TrackAllocationStacks); - else + } else Primary.Options.clear(OptionBit::TrackAllocationStacks); } @@ -1092,6 +1095,9 @@ private: 0, "invalid alignment"); + // Lock to initialize the RingBuffer + HybridMutex RingBufferInitLock; + // Pointer to memory mapped area starting with AllocationRingBuffer struct, // and immediately followed by Size elements of type Entry. atomic_uptr RingBufferAddress = {}; @@ -1546,11 +1552,16 @@ private: RBEntryStart)[N]; } - void mapAndInitializeRingBuffer() { - if (getFlags()->allocation_ring_buffer_size <= 0) + void initRingBufferMaybe() { + ScopedLock L(RingBufferInitLock); + if (getRingBuffer() != nullptr) return; - u32 AllocationRingBufferSize = - static_cast(getFlags()->allocation_ring_buffer_size); + + int ring_buffer_size = getFlags()->allocation_ring_buffer_size; + if (ring_buffer_size <= 0) + return; + + u32 AllocationRingBufferSize = static_cast(ring_buffer_size); // We store alloc and free stacks for each entry. constexpr u32 kStacksPerRingBufferEntry = 2; diff --git a/standalone/tests/combined_test.cpp b/standalone/tests/combined_test.cpp index 6a311adc55e..1a36155bcd4 100644 --- a/standalone/tests/combined_test.cpp +++ b/standalone/tests/combined_test.cpp @@ -867,32 +867,86 @@ SCUDO_TYPED_TEST(ScudoCombinedTest, ReallocateInPlaceStress) { } } +SCUDO_TYPED_TEST(ScudoCombinedTest, RingBufferDefaultDisabled) { + // The RingBuffer is not initialized until tracking is enabled for the + // first time. + auto *Allocator = this->Allocator.get(); + EXPECT_EQ(0u, Allocator->getRingBufferSize()); + EXPECT_EQ(nullptr, Allocator->getRingBufferAddress()); +} + +SCUDO_TYPED_TEST(ScudoCombinedTest, RingBufferInitOnce) { + auto *Allocator = this->Allocator.get(); + Allocator->setTrackAllocationStacks(true); + + auto RingBufferSize = Allocator->getRingBufferSize(); + ASSERT_GT(RingBufferSize, 0u); + auto *RingBufferAddress = Allocator->getRingBufferAddress(); + EXPECT_NE(nullptr, RingBufferAddress); + + // Enable tracking again to verify that the initialization only happens once. + Allocator->setTrackAllocationStacks(true); + ASSERT_EQ(RingBufferSize, Allocator->getRingBufferSize()); + EXPECT_EQ(RingBufferAddress, Allocator->getRingBufferAddress()); +} + SCUDO_TYPED_TEST(ScudoCombinedTest, RingBufferSize) { auto *Allocator = this->Allocator.get(); - auto Size = Allocator->getRingBufferSize(); - ASSERT_GT(Size, 0u); - EXPECT_EQ(Allocator->getRingBufferAddress()[Size - 1], '\0'); + Allocator->setTrackAllocationStacks(true); + + auto RingBufferSize = Allocator->getRingBufferSize(); + ASSERT_GT(RingBufferSize, 0u); + EXPECT_EQ(Allocator->getRingBufferAddress()[RingBufferSize - 1], '\0'); } SCUDO_TYPED_TEST(ScudoCombinedTest, RingBufferAddress) { auto *Allocator = this->Allocator.get(); - auto *Addr = Allocator->getRingBufferAddress(); - EXPECT_NE(Addr, nullptr); - EXPECT_EQ(Addr, Allocator->getRingBufferAddress()); + Allocator->setTrackAllocationStacks(true); + + auto *RingBufferAddress = Allocator->getRingBufferAddress(); + EXPECT_NE(RingBufferAddress, nullptr); + EXPECT_EQ(RingBufferAddress, Allocator->getRingBufferAddress()); +} + +SCUDO_TYPED_TEST(ScudoCombinedTest, StackDepotDefaultDisabled) { + // The StackDepot is not initialized until tracking is enabled for the + // first time. + auto *Allocator = this->Allocator.get(); + EXPECT_EQ(0u, Allocator->getStackDepotSize()); + EXPECT_EQ(nullptr, Allocator->getStackDepotAddress()); +} + +SCUDO_TYPED_TEST(ScudoCombinedTest, StackDepotInitOnce) { + auto *Allocator = this->Allocator.get(); + Allocator->setTrackAllocationStacks(true); + + auto StackDepotSize = Allocator->getStackDepotSize(); + EXPECT_GT(StackDepotSize, 0u); + auto *StackDepotAddress = Allocator->getStackDepotAddress(); + EXPECT_NE(nullptr, StackDepotAddress); + + // Enable tracking again to verify that the initialization only happens once. + Allocator->setTrackAllocationStacks(true); + EXPECT_EQ(StackDepotSize, Allocator->getStackDepotSize()); + EXPECT_EQ(StackDepotAddress, Allocator->getStackDepotAddress()); } SCUDO_TYPED_TEST(ScudoCombinedTest, StackDepotSize) { auto *Allocator = this->Allocator.get(); - auto Size = Allocator->getStackDepotSize(); - ASSERT_GT(Size, 0u); - EXPECT_EQ(Allocator->getStackDepotAddress()[Size - 1], '\0'); + Allocator->setTrackAllocationStacks(true); + + auto StackDepotSize = Allocator->getStackDepotSize(); + EXPECT_GT(StackDepotSize, 0u); + EXPECT_EQ(Allocator->getStackDepotAddress()[StackDepotSize - 1], '\0'); } SCUDO_TYPED_TEST(ScudoCombinedTest, StackDepotAddress) { auto *Allocator = this->Allocator.get(); - auto *Addr = Allocator->getStackDepotAddress(); - EXPECT_NE(Addr, nullptr); - EXPECT_EQ(Addr, Allocator->getStackDepotAddress()); + Allocator->setTrackAllocationStacks(true); + + auto *StackDepotAddress = Allocator->getStackDepotAddress(); + EXPECT_NE(StackDepotAddress, nullptr); + EXPECT_EQ(StackDepotAddress, Allocator->getStackDepotAddress()); } SCUDO_TYPED_TEST(ScudoCombinedTest, StackDepot) { -- cgit v1.2.3