From 46d4610039aa7cd14c221d1b874a9dfe03f48649 Mon Sep 17 00:00:00 2001 From: Ciure Andrei Date: Fri, 6 Jul 2018 22:22:05 +0300 Subject: [PATCH] Backed out 1 changesets (bug 1473371)for windows-specific changes and fails on windows CLOSED TREE Backed out changeset f86b10b13521 (bug 1473371) --- browser/app/winlauncher/DllBlocklistWin.cpp | 14 +- mozglue/misc/nsWindowsDllInterceptor.h | 136 ++++-------------- .../TestDllInterceptorCrossProcess.cpp | 12 +- 3 files changed, 48 insertions(+), 114 deletions(-) diff --git a/browser/app/winlauncher/DllBlocklistWin.cpp b/browser/app/winlauncher/DllBlocklistWin.cpp index a7c55882e7a4..a88a0747348f 100644 --- a/browser/app/winlauncher/DllBlocklistWin.cpp +++ b/browser/app/winlauncher/DllBlocklistWin.cpp @@ -357,13 +357,21 @@ InitializeDllBlocklistOOP(HANDLE aChildProcess) { mozilla::CrossProcessDllInterceptor intcpt(aChildProcess); intcpt.Init(L"ntdll.dll"); - bool ok = stub_NtMapViewOfSection.SetDetour(aChildProcess, intcpt, - "NtMapViewOfSection", + bool ok = stub_NtMapViewOfSection.SetDetour(intcpt, "NtMapViewOfSection", &patched_NtMapViewOfSection); if (!ok) { return false; } + // Set the child process's copy of stub_NtMapViewOfSection + SIZE_T bytesWritten; + ok = !!::WriteProcessMemory(aChildProcess, &stub_NtMapViewOfSection, + &stub_NtMapViewOfSection, + sizeof(stub_NtMapViewOfSection), &bytesWritten); + if (!ok) { + return false; + } + // Because aChildProcess has just been created in a suspended state, its // dynamic linker has not yet been initialized, thus its executable has // not yet been linked with ntdll.dll. If the blocklist hook intercepts a @@ -399,8 +407,6 @@ InitializeDllBlocklistOOP(HANDLE aChildProcess) ptrdiff_t iatLength = (curIatThunk - firstIatThunk) * sizeof(IMAGE_THUNK_DATA); - SIZE_T bytesWritten; - { // Scope for prot AutoVirtualProtect prot(firstIatThunk, iatLength, PAGE_READWRITE, aChildProcess); diff --git a/mozglue/misc/nsWindowsDllInterceptor.h b/mozglue/misc/nsWindowsDllInterceptor.h index f28172a9c82d..4ae504257b56 100644 --- a/mozglue/misc/nsWindowsDllInterceptor.h +++ b/mozglue/misc/nsWindowsDllInterceptor.h @@ -84,32 +84,32 @@ namespace mozilla { namespace interceptor { -template -struct OriginalFunctionPtrTraits; - -template -struct OriginalFunctionPtrTraits -{ - using ReturnType = R; -}; - -#if defined(_M_IX86) -template -struct OriginalFunctionPtrTraits -{ - using ReturnType = R; -}; - -template -struct OriginalFunctionPtrTraits -{ - using ReturnType = R; -}; -#endif // defined(_M_IX86) - template class FuncHook final { + template + struct OriginalFunctionPtrTraits; + + template + struct OriginalFunctionPtrTraits + { + using ReturnType = R; + }; + +#if defined(_M_IX86) + template + struct OriginalFunctionPtrTraits + { + using ReturnType = R; + }; + + template + struct OriginalFunctionPtrTraits + { + using ReturnType = R; + }; +#endif // defined(_M_IX86) + public: using ThisType = FuncHook; using ReturnType = typename OriginalFunctionPtrTraits::ReturnType; @@ -221,96 +221,15 @@ private: INIT_ONCE mInitOnce; }; -template -class MOZ_ONLY_USED_TO_AVOID_STATIC_CONSTRUCTORS FuncHookCrossProcess final -{ -public: - using ThisType = FuncHookCrossProcess; - using ReturnType = typename OriginalFunctionPtrTraits::ReturnType; - - FuncHookCrossProcess() = default; - ~FuncHookCrossProcess() = default; - - bool Set(HANDLE aProcess, InterceptorT& aInterceptor, const char* aName, - FuncPtrT aHookDest) - { - if (!aInterceptor.AddHook(aName, reinterpret_cast(aHookDest), - reinterpret_cast(&mOrigFunc))) { - return false; - } - - return CopyStubToChildProcess(aProcess); - } - - bool SetDetour(HANDLE aProcess, InterceptorT& aInterceptor, const char* aName, - FuncPtrT aHookDest) - { - if (!aInterceptor.AddDetour(aName, reinterpret_cast(aHookDest), - reinterpret_cast(&mOrigFunc))) { - return false; - } - - return CopyStubToChildProcess(aProcess); - } - - explicit operator bool() const - { - return !!mOrigFunc; - } - - /** - * NB: This operator is only meaningful when invoked in the target process! - */ - template - ReturnType operator()(ArgsType... aArgs) const - { - return mOrigFunc(std::forward(aArgs)...); - } - - FuncHookCrossProcess(const FuncHookCrossProcess&) = delete; - FuncHookCrossProcess(FuncHookCrossProcess&&) = delete; - FuncHookCrossProcess& operator=(const FuncHookCrossProcess&) = delete; - FuncHookCrossProcess& operator=(FuncHookCrossProcess&& aOther) = delete; - -private: - bool CopyStubToChildProcess(HANDLE aProcess) - { - SIZE_T bytesWritten; - return !!::WriteProcessMemory(aProcess, &mOrigFunc, &mOrigFunc, - sizeof(mOrigFunc), &bytesWritten); - } - -private: - FuncPtrT mOrigFunc; -}; - enum { kDefaultTrampolineSize = 128 }; -template -struct TypeResolver; - -template -struct TypeResolver -{ - template - using FuncHookType = FuncHook; -}; - -template -struct TypeResolver -{ - template - using FuncHookType = FuncHookCrossProcess; -}; - template > -class WindowsDllInterceptor final : public TypeResolver> +class WindowsDllInterceptor final { typedef WindowsDllInterceptor ThisType; @@ -453,12 +372,13 @@ private: return mDetourPatcher.AddHook(aProc, aHookDest, aOrigFunc); } +public: + template + using FuncHookType = FuncHook; + private: template friend class FuncHook; - - template - friend class FuncHookCrossProcess; }; } // namespace interceptor diff --git a/mozglue/tests/interceptor/TestDllInterceptorCrossProcess.cpp b/mozglue/tests/interceptor/TestDllInterceptorCrossProcess.cpp index 7a2ee3737504..c29d132ba9b0 100644 --- a/mozglue/tests/interceptor/TestDllInterceptorCrossProcess.cpp +++ b/mozglue/tests/interceptor/TestDllInterceptorCrossProcess.cpp @@ -73,14 +73,22 @@ int ParentMain() mozilla::CrossProcessDllInterceptor intcpt(childProcess.get()); intcpt.Init("TestDllInterceptorCrossProcess.exe"); - if (!gOrigReturnResult.Set(childProcess.get(), intcpt, "ReturnResult", - &ReturnResultHook)) { + if (!gOrigReturnResult.Set(intcpt, "ReturnResult", &ReturnResultHook)) { printf("TEST-UNEXPECTED-FAIL | DllInterceptorCrossProcess | Failed to add hook\n"); return 1; } printf("TEST-PASS | DllInterceptorCrossProcess | Hook added\n"); + // Let's save the original hook + SIZE_T bytesWritten; + if (!::WriteProcessMemory(childProcess.get(), &gOrigReturnResult, + &gOrigReturnResult, sizeof(gOrigReturnResult), + &bytesWritten)) { + printf("TEST-UNEXPECTED-FAIL | DllInterceptorCrossProcess | Failed to write original function pointer\n"); + return 1; + } + if (::ResumeThread(childMainThread.get()) == static_cast(-1)) { printf("TEST-UNEXPECTED-FAIL | DllInterceptorCrossProcess | Failed to resume child thread\n"); return 1;