From 706117a7d7cd3022102d579466dc2a17dbe94373 Mon Sep 17 00:00:00 2001 From: Brad Werth Date: Thu, 9 Mar 2023 23:12:20 +0000 Subject: [PATCH] Bug 1792146: Make replace_malloc_usable_size succeed with pointers within allocation pages. r=glandium This changes replace_malloc_usable_size to allow measuring the size of ptrs that appear anywhere within an allocation page, not just ptrs to the start of the allocation pages. This also modifies a gtest to explicitly check the usable size of some modified pointers, not just the usable size of the pages where those pointers are located. Differential Revision: https://phabricator.services.mozilla.com/D170963 --- memory/replace/phc/PHC.cpp | 17 ++++++++++++++--- memory/replace/phc/test/gtest/TestPHC.cpp | 4 +++- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/memory/replace/phc/PHC.cpp b/memory/replace/phc/PHC.cpp index 3d7c944ca578..d86e9d37e662 100644 --- a/memory/replace/phc/PHC.cpp +++ b/memory/replace/phc/PHC.cpp @@ -699,6 +699,12 @@ class GMut { return page.mState != AllocPageState::InUse && aNow >= page.mReuseTime; } + // Get the address of the allocation page referred to via an index. Used + // when checking pointers against page boundaries. + uint8_t* AllocPageBaseAddr(GMutLock, uintptr_t aIndex) { + return mAllocPages[aIndex].mBaseAddr; + } + Maybe PageArena(GMutLock aLock, uintptr_t aIndex) { const AllocPageInfo& page = mAllocPages[aIndex]; AssertAllocPageInUse(aLock, page); @@ -1367,13 +1373,18 @@ static size_t replace_malloc_usable_size(usable_ptr_t aPtr) { GMut::CrashOnGuardPage(const_cast(aPtr)); } - // At this point we know we have an allocation page. + // At this point we know aPtr lands within an allocation page, due to the + // math done in the PtrKind constructor. But if aPtr points to memory + // before the base address of the allocation, we return 0. uintptr_t index = pk.AllocPageIndex(); MutexAutoLock lock(GMut::sMutex); - // Check for malloc_usable_size() of a freed block. - gMut->EnsureValidAndInUse(lock, const_cast(aPtr), index); + void* pageBaseAddr = gMut->AllocPageBaseAddr(lock, index); + + if (MOZ_UNLIKELY(aPtr < pageBaseAddr)) { + return 0; + } return gMut->PageUsableSize(lock, index); } diff --git a/memory/replace/phc/test/gtest/TestPHC.cpp b/memory/replace/phc/test/gtest/TestPHC.cpp index b83db800f2b9..738a50eee27e 100644 --- a/memory/replace/phc/test/gtest/TestPHC.cpp +++ b/memory/replace/phc/test/gtest/TestPHC.cpp @@ -147,12 +147,13 @@ TEST(PHC, TestPHCInfo) PHCInfoEq(phcInfo, phc::AddrInfo::Kind::InUsePage, p, 32ul, true, false)); ASSERT_EQ(moz_malloc_usable_size(p), 32ul); jemalloc_ptr_info(p, &jeInfo); + ASSERT_TRUE(JeInfoEq(jeInfo, TagLiveAlloc, p, 32, 0)); // Test an in-use PHC allocation: last byte within it. ASSERT_TRUE(ReplaceMalloc::IsPHCAllocation(p + 31, &phcInfo)); ASSERT_TRUE( PHCInfoEq(phcInfo, phc::AddrInfo::Kind::InUsePage, p, 32ul, true, false)); - ASSERT_TRUE(JeInfoEq(jeInfo, TagLiveAlloc, p, 32, 0)); + ASSERT_EQ(moz_malloc_usable_size(p + 31), 32ul); jemalloc_ptr_info(p + 31, &jeInfo); ASSERT_TRUE(JeInfoEq(jeInfo, TagLiveAlloc, p, 32, 0)); @@ -160,6 +161,7 @@ TEST(PHC, TestPHCInfo) ASSERT_TRUE(ReplaceMalloc::IsPHCAllocation(p - 1, &phcInfo)); ASSERT_TRUE( PHCInfoEq(phcInfo, phc::AddrInfo::Kind::InUsePage, p, 32ul, true, false)); + ASSERT_EQ(moz_malloc_usable_size(p - 1), 0ul); jemalloc_ptr_info(p - 1, &jeInfo); ASSERT_TRUE(JeInfoEq(jeInfo, TagUnknown, nullptr, 0, 0)); -- 2.11.4.GIT