diff --git a/browser/app/winlauncher/freestanding/DllBlocklist.cpp b/browser/app/winlauncher/freestanding/DllBlocklist.cpp index 2806cec87e15..a77418e3b158 100644 --- a/browser/app/winlauncher/freestanding/DllBlocklist.cpp +++ b/browser/app/winlauncher/freestanding/DllBlocklist.cpp @@ -467,7 +467,7 @@ NTSTATUS NTAPI patched_NtMapViewOfSection( if (nt::RtlGetProcessHeap()) { ModuleLoadFrame::NotifySectionMap( nt::AllocatedUnicodeString(sectionFileName), *aBaseAddress, stubStatus, - loadStatus); + loadStatus, isDependent); } if (loadStatus == ModuleLoadInfo::Status::Loaded || diff --git a/browser/app/winlauncher/freestanding/ModuleLoadFrame.cpp b/browser/app/winlauncher/freestanding/ModuleLoadFrame.cpp index c734e70ed5ed..3aa043b8c7aa 100644 --- a/browser/app/winlauncher/freestanding/ModuleLoadFrame.cpp +++ b/browser/app/winlauncher/freestanding/ModuleLoadFrame.cpp @@ -25,12 +25,14 @@ ModuleLoadFrame::ModuleLoadFrame(PCUNICODE_STRING aRequestedDllName) ModuleLoadFrame::ModuleLoadFrame(nt::AllocatedUnicodeString&& aSectionName, const void* aMapBaseAddr, NTSTATUS aNtStatus, - ModuleLoadInfo::Status aLoadStatus) + ModuleLoadInfo::Status aLoadStatus, + bool aIsDependent) : mPrev(sTopFrame.get()), mContext(nullptr), mLSPSubstitutionRequired(false), mLoadNtStatus(aNtStatus), - mLoadInfo(std::move(aSectionName), aMapBaseAddr, aLoadStatus) { + mLoadInfo(std::move(aSectionName), aMapBaseAddr, aLoadStatus, + aIsDependent) { sTopFrame.set(this); gLoaderPrivateAPI.NotifyBeginDllLoad(&mContext, mLoadInfo.mSectionName); @@ -70,7 +72,8 @@ void ModuleLoadFrame::SetLSPSubstitutionRequired(PCUNICODE_STRING aLeafName) { /* static */ void ModuleLoadFrame::NotifySectionMap( nt::AllocatedUnicodeString&& aSectionName, const void* aMapBaseAddr, - NTSTATUS aMapNtStatus, ModuleLoadInfo::Status aLoadStatus) { + NTSTATUS aMapNtStatus, ModuleLoadInfo::Status aLoadStatus, + bool aIsDependent) { ModuleLoadFrame* topFrame = sTopFrame.get(); if (!topFrame) { // The only time that this data is useful is during initial mapping of @@ -79,13 +82,13 @@ void ModuleLoadFrame::NotifySectionMap( // initial process startup. if (gLoaderPrivateAPI.IsDefaultObserver()) { OnBareSectionMap(std::move(aSectionName), aMapBaseAddr, aMapNtStatus, - aLoadStatus); + aLoadStatus, aIsDependent); } return; } topFrame->OnSectionMap(std::move(aSectionName), aMapBaseAddr, aMapNtStatus, - aLoadStatus); + aLoadStatus, aIsDependent); } /* static */ @@ -94,12 +97,13 @@ bool ModuleLoadFrame::ExistsTopFrame() { return !!sTopFrame.get(); } void ModuleLoadFrame::OnSectionMap(nt::AllocatedUnicodeString&& aSectionName, const void* aMapBaseAddr, NTSTATUS aMapNtStatus, - ModuleLoadInfo::Status aLoadStatus) { + ModuleLoadInfo::Status aLoadStatus, + bool aIsDependent) { if (mLoadInfo.mBaseAddr) { // If mBaseAddr is not null then |this| has already seen a module load. This // means that we are witnessing a bare section map. OnBareSectionMap(std::move(aSectionName), aMapBaseAddr, aMapNtStatus, - aLoadStatus); + aLoadStatus, aIsDependent); return; } @@ -111,10 +115,11 @@ void ModuleLoadFrame::OnSectionMap(nt::AllocatedUnicodeString&& aSectionName, /* static */ void ModuleLoadFrame::OnBareSectionMap( nt::AllocatedUnicodeString&& aSectionName, const void* aMapBaseAddr, - NTSTATUS aMapNtStatus, ModuleLoadInfo::Status aLoadStatus) { + NTSTATUS aMapNtStatus, ModuleLoadInfo::Status aLoadStatus, + bool aIsDependent) { // We call the special constructor variant that is used for bare mappings. ModuleLoadFrame frame(std::move(aSectionName), aMapBaseAddr, aMapNtStatus, - aLoadStatus); + aLoadStatus, aIsDependent); } NTSTATUS ModuleLoadFrame::SetLoadStatus(NTSTATUS aNtStatus, diff --git a/browser/app/winlauncher/freestanding/ModuleLoadFrame.h b/browser/app/winlauncher/freestanding/ModuleLoadFrame.h index a0dfc01b5712..51a179db991a 100644 --- a/browser/app/winlauncher/freestanding/ModuleLoadFrame.h +++ b/browser/app/winlauncher/freestanding/ModuleLoadFrame.h @@ -37,7 +37,8 @@ class MOZ_RAII ModuleLoadFrame final { */ static void NotifySectionMap(nt::AllocatedUnicodeString&& aSectionName, const void* aMapBaseAddr, NTSTATUS aMapNtStatus, - ModuleLoadInfo::Status aLoadStatus); + ModuleLoadInfo::Status aLoadStatus, + bool aIsDependent); static bool ExistsTopFrame(); /** @@ -57,12 +58,12 @@ class MOZ_RAII ModuleLoadFrame final { */ ModuleLoadFrame(nt::AllocatedUnicodeString&& aSectionName, const void* aMapBaseAddr, NTSTATUS aNtStatus, - ModuleLoadInfo::Status aLoadStatus); + ModuleLoadInfo::Status aLoadStatus, bool aIsDependent); void SetLSPSubstitutionRequired(PCUNICODE_STRING aLeafName); void OnSectionMap(nt::AllocatedUnicodeString&& aSectionName, const void* aMapBaseAddr, NTSTATUS aMapNtStatus, - ModuleLoadInfo::Status aLoadStatus); + ModuleLoadInfo::Status aLoadStatus, bool aIsDependent); /** * A "bare" section mapping is one that was mapped without the code passing @@ -71,7 +72,8 @@ class MOZ_RAII ModuleLoadFrame final { */ static void OnBareSectionMap(nt::AllocatedUnicodeString&& aSectionName, const void* aMapBaseAddr, NTSTATUS aMapNtStatus, - ModuleLoadInfo::Status aLoadStatus); + ModuleLoadInfo::Status aLoadStatus, + bool aIsDependent); private: // Link to the previous frame diff --git a/mozglue/dllservices/ModuleLoadInfo.h b/mozglue/dllservices/ModuleLoadInfo.h index 2d9bdc0cc729..15710430610c 100644 --- a/mozglue/dllservices/ModuleLoadInfo.h +++ b/mozglue/dllservices/ModuleLoadInfo.h @@ -34,7 +34,8 @@ struct ModuleLoadInfo final { mThreadId(nt::RtlGetCurrentThreadId()), mRequestedDllName(aRequestedDllName), mBaseAddr(nullptr), - mStatus(Status::Loaded) { + mStatus(Status::Loaded), + mIsDependent(false) { # if defined(IMPL_MFBT) ::QueryPerformanceCounter(&mBeginTimestamp); # else @@ -49,12 +50,13 @@ struct ModuleLoadInfo final { * of another library. */ ModuleLoadInfo(nt::AllocatedUnicodeString&& aSectionName, - const void* aBaseAddr, Status aLoadStatus) + const void* aBaseAddr, Status aLoadStatus, bool aIsDependent) : mLoadTimeInfo(), mThreadId(nt::RtlGetCurrentThreadId()), mSectionName(std::move(aSectionName)), mBaseAddr(aBaseAddr), - mStatus(aLoadStatus) { + mStatus(aLoadStatus), + mIsDependent(aIsDependent) { # if defined(IMPL_MFBT) ::QueryPerformanceCounter(&mBeginTimestamp); # else @@ -164,6 +166,8 @@ struct ModuleLoadInfo final { Vector mBacktrace; // The status of DLL load Status mStatus; + // Whether the module is one of the executables's dependent modules or not + bool mIsDependent; }; using ModuleLoadInfoVec = Vector; diff --git a/mozglue/misc/NativeNt.h b/mozglue/misc/NativeNt.h index ca37fc7bec47..79b90bcea92c 100644 --- a/mozglue/misc/NativeNt.h +++ b/mozglue/misc/NativeNt.h @@ -26,9 +26,7 @@ #include "mozilla/interceptor/TargetFunction.h" #if defined(MOZILLA_INTERNAL_API) -# include "nsHashKeys.h" # include "nsString.h" -# include "nsTHashtable.h" #endif // defined(MOZILLA_INTERNAL_API) // The declarations within this #if block are intended to be used for initial @@ -743,17 +741,6 @@ class MOZ_RAII PEHeaders final { } } -#if defined(MOZILLA_INTERNAL_API) - nsTHashtable GenerateDependentModuleSet() - const { - nsTHashtable dependentModuleSet; - EnumImportChunks([&dependentModuleSet](const char* aModule) { - dependentModuleSet.PutEntry(GetLeafName(NS_ConvertASCIItoUTF16(aModule))); - }); - return dependentModuleSet; - } -#endif // defined(MOZILLA_INTERNAL_API) - /** * If |aBoundaries| is given, this method checks whether each IAT entry is * within the given range, and if any entry is out of the range, we return diff --git a/mozglue/tests/gtest/TestNativeNtGTest.cpp b/mozglue/tests/gtest/TestNativeNtGTest.cpp index e0f0a343a74b..7619c744ca73 100644 --- a/mozglue/tests/gtest/TestNativeNtGTest.cpp +++ b/mozglue/tests/gtest/TestNativeNtGTest.cpp @@ -9,12 +9,19 @@ #include "gtest/gtest.h" #include "mozilla/NativeNt.h" +#include "nsHashKeys.h" +#include "nsTHashSet.h" TEST(TestNativeNtGTest, GenerateDependentModuleSet) { mozilla::nt::PEHeaders executable(::GetModuleHandleW(nullptr)); - auto dependentModules = executable.GenerateDependentModuleSet(); - EXPECT_NE(dependentModules.GetEntry(u"mozglue.dll"_ns), nullptr); - EXPECT_NE(dependentModules.GetEntry(u"MOZGLUE.dll"_ns), nullptr); - EXPECT_EQ(dependentModules.GetEntry(u"xxx.dll"_ns), nullptr); + nsTHashSet dependentModules; + executable.EnumImportChunks([&](const char* aModule) { + dependentModules.Insert( + mozilla::nt::GetLeafName(NS_ConvertASCIItoUTF16(aModule))); + }); + + EXPECT_TRUE(dependentModules.Contains(u"mozglue.dll"_ns)); + EXPECT_TRUE(dependentModules.Contains(u"MOZGLUE.dll"_ns)); + EXPECT_FALSE(dependentModules.Contains(u"xxx.dll"_ns)); } diff --git a/toolkit/xre/UntrustedModulesData.cpp b/toolkit/xre/UntrustedModulesData.cpp index a13a88ef1680..f830c910dfde 100644 --- a/toolkit/xre/UntrustedModulesData.cpp +++ b/toolkit/xre/UntrustedModulesData.cpp @@ -205,7 +205,7 @@ ProcessedModuleLoadEvent::ProcessedModuleLoadEvent() ProcessedModuleLoadEvent::ProcessedModuleLoadEvent( glue::EnhancedModuleLoadInfo&& aModLoadInfo, - RefPtr&& aModuleRecord, bool aIsDependent) + RefPtr&& aModuleRecord) : mProcessUptimeMS(QPCTimeStampToProcessUptimeMilliseconds( aModLoadInfo.mNtLoadInfo.mBeginTimestamp)), mLoadDurationMS(QPCLoadDurationToMilliseconds(aModLoadInfo.mNtLoadInfo)), @@ -214,7 +214,7 @@ ProcessedModuleLoadEvent::ProcessedModuleLoadEvent( mBaseAddress( reinterpret_cast(aModLoadInfo.mNtLoadInfo.mBaseAddr)), mModule(std::move(aModuleRecord)), - mIsDependent(aIsDependent), + mIsDependent(aModLoadInfo.mNtLoadInfo.mIsDependent), mLoadStatus(static_cast(aModLoadInfo.mNtLoadInfo.mStatus)) { if (!mModule || !(*mModule)) { return; diff --git a/toolkit/xre/UntrustedModulesData.h b/toolkit/xre/UntrustedModulesData.h index 3a28f22c6729..6cd095b91335 100644 --- a/toolkit/xre/UntrustedModulesData.h +++ b/toolkit/xre/UntrustedModulesData.h @@ -127,8 +127,7 @@ class ProcessedModuleLoadEvent final { public: ProcessedModuleLoadEvent(); ProcessedModuleLoadEvent(glue::EnhancedModuleLoadInfo&& aModLoadInfo, - RefPtr&& aModuleRecord, - bool aIsDependent); + RefPtr&& aModuleRecord); explicit operator bool() const { return mModule && *mModule; } bool IsXULLoad() const; diff --git a/toolkit/xre/UntrustedModulesProcessor.cpp b/toolkit/xre/UntrustedModulesProcessor.cpp index 466fbf777ac2..a8d964e44766 100644 --- a/toolkit/xre/UntrustedModulesProcessor.cpp +++ b/toolkit/xre/UntrustedModulesProcessor.cpp @@ -87,34 +87,6 @@ class MOZ_RAII BackgroundPriorityRegion final { const BOOL mIsBackground; }; -// This class wraps a set of the executables's dependent modules -// that is delay-initialized the first time Lookup() is called. -class DependentModules final { - Maybe> mDependentModules; - - public: - bool Lookup(const nsString& aModulePath) { - if (aModulePath.IsEmpty()) { - return false; - } - - if (mDependentModules.isNothing()) { - nt::PEHeaders executable(::GetModuleHandleW(nullptr)); - - // We generate a hash table only when the executable's import table is - // tampered. If the import table is intact, all dependent modules are - // legit and we're not interested in any of them. In such a case, we - // set an empty table so that this function returns false. - mDependentModules = - Some(executable.IsImportDirectoryTampered() - ? executable.GenerateDependentModuleSet() - : nsTHashtable()); - } - - return !!mDependentModules.ref().GetEntry(nt::GetLeafName(aModulePath)); - } -}; - /* static */ bool UntrustedModulesProcessor::IsSupportedProcessType() { switch (XRE_GetProcessType()) { @@ -150,8 +122,7 @@ UntrustedModulesProcessor::UntrustedModulesProcessor() LazyIdleThread::ManualShutdown)), mUnprocessedMutex( "mozilla::UntrustedModulesProcessor::mUnprocessedMutex"), - mAllowProcessing(true), - mIsFirstBatchProcessed(false) { + mAllowProcessing(true) { AddObservers(); } @@ -595,11 +566,8 @@ void UntrustedModulesProcessor::ProcessModuleLoadQueue() { return; } - auto cleanup = MakeScopeExit([&]() { mIsFirstBatchProcessed = true; }); - Telemetry::BatchProcessedStackGenerator stackProcessor; ModulesMap modules; - DependentModules dependentModules; Maybe maybeXulLoadDuration; Vector processedStacks; @@ -624,18 +592,9 @@ void UntrustedModulesProcessor::ProcessModuleLoadQueue() { return; } - bool isDependent = mIsFirstBatchProcessed - ? false - : dependentModules.Lookup(module->mSanitizedDllName); - - if (!mAllowProcessing) { - return; - } - glue::EnhancedModuleLoadInfo::BacktraceType backtrace = std::move(entry.mNtLoadInfo.mBacktrace); - ProcessedModuleLoadEvent event(std::move(entry), std::move(module), - isDependent); + ProcessedModuleLoadEvent event(std::move(entry), std::move(module)); if (!event) { // We don't have a sanitized DLL path, so we cannot include this event @@ -857,10 +816,7 @@ void UntrustedModulesProcessor::CompleteProcessing( return; } - auto cleanup = MakeScopeExit([&]() { mIsFirstBatchProcessed = true; }); - Telemetry::BatchProcessedStackGenerator stackProcessor; - DependentModules dependentModules; Maybe maybeXulLoadDuration; Vector processedStacks; @@ -883,19 +839,9 @@ void UntrustedModulesProcessor::CompleteProcessing( return; } - bool isDependent = - mIsFirstBatchProcessed - ? false - : dependentModules.Lookup(module->mSanitizedDllName); - - if (!mAllowProcessing) { - return; - } - glue::EnhancedModuleLoadInfo::BacktraceType backtrace = std::move(item.mNtLoadInfo.mBacktrace); - ProcessedModuleLoadEvent event(std::move(item), std::move(module), - isDependent); + ProcessedModuleLoadEvent event(std::move(item), std::move(module)); if (!mAllowProcessing) { return; diff --git a/toolkit/xre/UntrustedModulesProcessor.h b/toolkit/xre/UntrustedModulesProcessor.h index 546c05119afc..2bd3329ff280 100644 --- a/toolkit/xre/UntrustedModulesProcessor.h +++ b/toolkit/xre/UntrustedModulesProcessor.h @@ -141,15 +141,6 @@ class UntrustedModulesProcessor final : public nsIObserver { // This member may be touched by any thread Atomic mAllowProcessing; - - // This member must only be touched on mThread, making sure a hash table of - // dependent modules is initialized once in a process when the first batch - // of queued events is processed. We don't need the hash table to process - // subsequent queues because we're interested in modules imported via the - // executable's Import Table which are all expected to be in the first queue. - // A boolean flag is enough to control initialization because tasks of - // processing queues are serialized. - bool mIsFirstBatchProcessed; }; } // namespace mozilla