diff --git a/toolkit/xre/UntrustedModulesData.cpp b/toolkit/xre/UntrustedModulesData.cpp index f830c910dfde..d2f62a607a01 100644 --- a/toolkit/xre/UntrustedModulesData.cpp +++ b/toolkit/xre/UntrustedModulesData.cpp @@ -321,21 +321,8 @@ void UntrustedModulesData::AddNewLoads( Unused << mModules.LookupOrInsert(entry.GetKey(), entry.GetData()); } - // This constant matches the maximum in Telemetry::CombinedStacks - const size_t kMaxEvents = 50; MOZ_ASSERT(mEvents.length() <= kMaxEvents); - if (mEvents.length() + aEvents.length() > kMaxEvents) { - // Ensure that we will never retain more tha kMaxEvents events - size_t newLength = kMaxEvents - mEvents.length(); - if (!newLength) { - return; - } - - aEvents.shrinkTo(newLength); - aStacks.shrinkTo(newLength); - } - if (mEvents.empty()) { mEvents = std::move(aEvents); } else { diff --git a/toolkit/xre/UntrustedModulesData.h b/toolkit/xre/UntrustedModulesData.h index 6cd095b91335..9e274838d8b5 100644 --- a/toolkit/xre/UntrustedModulesData.h +++ b/toolkit/xre/UntrustedModulesData.h @@ -169,6 +169,10 @@ class ModulesMap final class UntrustedModulesData final { public: + // Ensure mEvents will never retain more than kMaxEvents events. + // This constant matches the maximum in Telemetry::CombinedStacks. + static constexpr size_t kMaxEvents = 50; + UntrustedModulesData() : mProcessType(XRE_GetProcessType()), mPid(::GetCurrentProcessId()), diff --git a/toolkit/xre/UntrustedModulesProcessor.cpp b/toolkit/xre/UntrustedModulesProcessor.cpp index a8d964e44766..3e02134f931f 100644 --- a/toolkit/xre/UntrustedModulesProcessor.cpp +++ b/toolkit/xre/UntrustedModulesProcessor.cpp @@ -553,7 +553,27 @@ void UntrustedModulesProcessor::ProcessModuleLoadQueue() { { // Scope for lock MutexAutoLock lock(mUnprocessedMutex); CancelScheduledProcessing(lock); - loadsToProcess.swap(mUnprocessedModuleLoads); + + // The potential size of mProcessedModuleLoads if all of the unprocessed + // events are from third-party modules. + const size_t newDataLength = mProcessedModuleLoads.mEvents.length() + + mUnprocessedModuleLoads.length(); + if (newDataLength <= UntrustedModulesData::kMaxEvents) { + loadsToProcess.swap(mUnprocessedModuleLoads); + } else { + // To prevent mProcessedModuleLoads from exceeding |kMaxEvents|, + // we process the first items in the mUnprocessedModuleLoads, + // leaving the the remaining events for the next time. + const size_t capacity = UntrustedModulesData::kMaxEvents > + mProcessedModuleLoads.mEvents.length() + ? (UntrustedModulesData::kMaxEvents - + mProcessedModuleLoads.mEvents.length()) + : 0; + auto moveRangeBegin = mUnprocessedModuleLoads.begin(); + auto moveRangeEnd = moveRangeBegin + capacity; + Unused << loadsToProcess.moveAppend(moveRangeBegin, moveRangeEnd); + mUnprocessedModuleLoads.erase(moveRangeBegin, moveRangeEnd); + } } if (!mAllowProcessing || loadsToProcess.empty()) { diff --git a/toolkit/xre/test/gtest/TestUntrustedModules.cpp b/toolkit/xre/test/gtest/TestUntrustedModules.cpp index 3a345f729e03..02c6aea57da8 100644 --- a/toolkit/xre/test/gtest/TestUntrustedModules.cpp +++ b/toolkit/xre/test/gtest/TestUntrustedModules.cpp @@ -135,64 +135,6 @@ class UntrustedModulesCollector { } }; -static void ValidateUntrustedModules(const UntrustedModulesData& aData) { - // This defines a list of modules which are listed on our blocklist and - // thus its loading status is not expected to be Status::Loaded. - // Although the UntrustedModulesFixture test does not touch any of them, - // the current process might have run a test like TestDllBlocklist where - // we try to load and block them. - const struct { - const wchar_t* mName; - ModuleLoadInfo::Status mStatus; - } kKnownModules[] = { - // Sorted by mName for binary-search - {L"TestDllBlocklist_MatchByName.dll", ModuleLoadInfo::Status::Blocked}, - {L"TestDllBlocklist_MatchByVersion.dll", ModuleLoadInfo::Status::Blocked}, - {L"TestDllBlocklist_NoOpEntryPoint.dll", - ModuleLoadInfo::Status::Redirected}, - }; - - EXPECT_EQ(aData.mProcessType, GeckoProcessType_Default); - EXPECT_EQ(aData.mPid, ::GetCurrentProcessId()); - - nsTHashtable> moduleSet; - for (const RefPtr& module : aData.mModules.Values()) { - moduleSet.PutEntry(module); - } - - for (const auto& evt : aData.mEvents) { - const nsDependentSubstring leafName = - nt::GetLeafName(evt.mModule->mResolvedNtName); - const nsAutoString leafNameStr(leafName.Data(), leafName.Length()); - size_t match; - if (BinarySearchIf( - kKnownModules, 0, ArrayLength(kKnownModules), - [&leafNameStr](const auto& aVal) { - return _wcsicmp(leafNameStr.get(), aVal.mName); - }, - &match)) { - // No check for mThreadId because a known module may be loaded - // in a different thread. - EXPECT_EQ(evt.mLoadStatus, - static_cast(kKnownModules[match].mStatus)); - } else { - EXPECT_EQ(evt.mThreadId, ::GetCurrentThreadId()); - EXPECT_EQ(evt.mLoadStatus, 0); - } - - // Make sure mModule is pointing to an entry of mModules. - EXPECT_TRUE(moduleSet.Contains(evt.mModule)); - EXPECT_FALSE(evt.mIsDependent); - } - - // No check for the mXULLoadDurationMS field because the field has a value - // in CCov build GTest, but it is empty in non-CCov build (bug 1681936). - EXPECT_GT(aData.mEvents.length(), 0); - EXPECT_GT(aData.mStacks.GetModuleCount(), 0); - EXPECT_EQ(aData.mSanitizationFailures, 0); - EXPECT_EQ(aData.mTrustTestFailures, 0); -} - class UntrustedModulesFixture : public TelemetryTestFixture { static constexpr int kLoadCountBeforeDllServices = 5; static constexpr int kLoadCountAfterDllServices = 5; @@ -229,6 +171,8 @@ class UntrustedModulesFixture : public TelemetryTestFixture { kLoadCountBeforeDllServices + kLoadCountAfterDllServices; static const nsString kTestModules[]; + static void ValidateUntrustedModules(const UntrustedModulesData& aData); + static void LoadAndFree(const nsAString& aLeaf) { nsModuleHandle dll(::LoadLibraryW(PrependWorkingDir(aLeaf).get())); EXPECT_TRUE(!!dll); @@ -301,10 +245,81 @@ class UntrustedModulesFixture : public TelemetryTestFixture { }; const nsString UntrustedModulesFixture::kTestModules[] = { - u"TestUntrustedModules_Dll1.dll"_ns, u"TestUntrustedModules_Dll2.dll"_ns}; + // Sorted for binary-search + u"TestUntrustedModules_Dll1.dll"_ns, + u"TestUntrustedModules_Dll2.dll"_ns, +}; + INIT_ONCE UntrustedModulesFixture::sInitLoadOnce = INIT_ONCE_STATIC_INIT; UntrustedModulesCollector UntrustedModulesFixture::sInitLoadDataCollector; +void UntrustedModulesFixture::ValidateUntrustedModules( + const UntrustedModulesData& aData) { + // This defines a list of modules which are listed on our blocklist and + // thus its loading status is not expected to be Status::Loaded. + // Although the UntrustedModulesFixture test does not touch any of them, + // the current process might have run a test like TestDllBlocklist where + // we try to load and block them. + const struct { + const wchar_t* mName; + ModuleLoadInfo::Status mStatus; + } kKnownModules[] = { + // Sorted by mName for binary-search + {L"TestDllBlocklist_MatchByName.dll", ModuleLoadInfo::Status::Blocked}, + {L"TestDllBlocklist_MatchByVersion.dll", ModuleLoadInfo::Status::Blocked}, + {L"TestDllBlocklist_NoOpEntryPoint.dll", + ModuleLoadInfo::Status::Redirected}, + }; + + EXPECT_EQ(aData.mProcessType, GeckoProcessType_Default); + EXPECT_EQ(aData.mPid, ::GetCurrentProcessId()); + + nsTHashtable> moduleSet; + for (const RefPtr& module : aData.mModules.Values()) { + moduleSet.PutEntry(module); + } + + for (const auto& evt : aData.mEvents) { + const nsDependentSubstring leafName = + nt::GetLeafName(evt.mModule->mResolvedNtName); + const nsAutoString leafNameStr(leafName.Data(), leafName.Length()); + size_t match; + if (BinarySearchIf( + kKnownModules, 0, ArrayLength(kKnownModules), + [&leafNameStr](const auto& aVal) { + return _wcsicmp(leafNameStr.get(), aVal.mName); + }, + &match)) { + EXPECT_EQ(evt.mLoadStatus, + static_cast(kKnownModules[match].mStatus)); + } else { + EXPECT_EQ(evt.mLoadStatus, 0); + } + + if (BinarySearchIf( + kTestModules, 0, ArrayLength(kTestModules), + [&leafNameStr](const auto& aVal) { + return _wcsicmp(leafNameStr.get(), aVal.get()); + }, + &match)) { + // We know the test modules are loaded in the main thread, + // but we don't know about other modules. + EXPECT_EQ(evt.mThreadId, ::GetCurrentThreadId()); + } + + // Make sure mModule is pointing to an entry of mModules. + EXPECT_TRUE(moduleSet.Contains(evt.mModule)); + EXPECT_FALSE(evt.mIsDependent); + } + + // No check for the mXULLoadDurationMS field because the field has a value + // in CCov build GTest, but it is empty in non-CCov build (bug 1681936). + EXPECT_GT(aData.mEvents.length(), 0); + EXPECT_GT(aData.mStacks.GetModuleCount(), 0); + EXPECT_EQ(aData.mSanitizationFailures, 0); + EXPECT_EQ(aData.mTrustTestFailures, 0); +} + BOOL CALLBACK UntrustedModulesFixture::InitialModuleLoadOnce(PINIT_ONCE, void*, void**) { for (int i = 0; i < kLoadCountBeforeDllServices; ++i) {