From 98f340fd14b5f7ee9a8c44977dfd194bdefc6886 Mon Sep 17 00:00:00 2001 From: Justin Lebar Date: Tue, 10 Apr 2012 15:52:56 -0400 Subject: [PATCH] Bug 742491 - Use a thread-safe DLL patcher on Windows, when possible. r=glandium --- dom/plugins/base/nsPluginNativeWindowWin.cpp | 2 + toolkit/xre/nsWindowsDllBlocklist.cpp | 2 + toolkit/xre/nsWindowsDllInterceptor.h | 282 +++++++++++++++++-- toolkit/xre/test/win/TestDllInterceptor.cpp | 2 + 4 files changed, 267 insertions(+), 21 deletions(-) diff --git a/dom/plugins/base/nsPluginNativeWindowWin.cpp b/dom/plugins/base/nsPluginNativeWindowWin.cpp index e013a6e725b..fd60d48fc61 100644 --- a/dom/plugins/base/nsPluginNativeWindowWin.cpp +++ b/dom/plugins/base/nsPluginNativeWindowWin.cpp @@ -60,6 +60,8 @@ #include "nsTWeakRef.h" #include "nsCrashOnException.h" +using namespace mozilla; + #define NP_POPUP_API_VERSION 16 #define nsMajorVersion(v) (((PRInt32)(v) >> 16) & 0xffff) diff --git a/toolkit/xre/nsWindowsDllBlocklist.cpp b/toolkit/xre/nsWindowsDllBlocklist.cpp index 5fc42e4ce01..c6040218af3 100644 --- a/toolkit/xre/nsWindowsDllBlocklist.cpp +++ b/toolkit/xre/nsWindowsDllBlocklist.cpp @@ -56,6 +56,8 @@ #include "nsWindowsDllInterceptor.h" +using namespace mozilla; + #if defined(MOZ_CRASHREPORTER) && !defined(NO_BLOCKLIST_CRASHREPORTER) #include "nsExceptionHandler.h" #endif diff --git a/toolkit/xre/nsWindowsDllInterceptor.h b/toolkit/xre/nsWindowsDllInterceptor.h index bc6d693c54c..325316410bc 100644 --- a/toolkit/xre/nsWindowsDllInterceptor.h +++ b/toolkit/xre/nsWindowsDllInterceptor.h @@ -41,7 +41,34 @@ #include /* - * Simple trampoline interception + * Simple function interception. + * + * We have two separate mechanisms for intercepting a function: We can use the + * built-in nop space, if it exists, or we can create a detour. + * + * Using the built-in nop space works as follows: On x86-32, DLL functions + * begin with a two-byte nop (mov edi, edi) and are preceeded by five bytes of + * NOP instructions. + * + * When we detect a function with this prelude, we do the following: + * + * 1. Write a long jump to our interceptor function into the five bytes of NOPs + * before the function. + * + * 2. Write a short jump -5 into the two-byte nop at the beginning of the function. + * + * This mechanism is nice because it's thread-safe. It's even safe to do if + * another thread is currently running the function we're modifying! + * + * When the WindowsDllNopSpacePatcher is destroyed, we overwrite the short jump + * but not the long jump, so re-intercepting the same function won't work, + * because its prelude won't match. + * + * + * Unfortunately nop space patching doesn't work on functions which don't have + * this magic prelude (and in particular, x86-64 never has the prelude). So + * when we can't use the built-in nop space, we fall back to using a detour, + * which works as follows: * * 1. Save first N bytes of OrigFunction to trampoline, where N is a * number of bytes >= 5 that are instruction aligned. @@ -55,27 +82,175 @@ * 4. Hook function needs to call the trampoline during its execution, * to invoke the original function (so address of trampoline is * returned). - * - * When the WindowsDllInterceptor class is destructed, OrigFunction is - * patched again to jump directly to the trampoline instead of going - * through the hook function. As such, re-intercepting the same function - * won't work, as jump instructions are not supported. + * + * When the WindowsDllDetourPatcher object is destructed, OrigFunction is + * patched again to jump directly to the trampoline instead of going through + * the hook function. As such, re-intercepting the same function won't work, as + * jump instructions are not supported. + * + * Note that this is not thread-safe. Sad day. + * */ -class WindowsDllInterceptor +#include + +namespace mozilla { +namespace internal { + +class WindowsDllNopSpacePatcher +{ + typedef unsigned char *byteptr_t; + HMODULE mModule; + + // Dumb array for remembering the addresses of functions we've patched. + // (This should be nsTArray, but non-XPCOM code uses this class.) + static const size_t maxPatchedFns = 128; + byteptr_t mPatchedFns[maxPatchedFns]; + int mPatchedFnsLen; + +public: + WindowsDllNopSpacePatcher() + : mModule(0) + , mPatchedFnsLen(0) + {} + + ~WindowsDllNopSpacePatcher() + { + // Restore the mov edi, edi to the beginning of each function we patched. + + for (int i = 0; i < mPatchedFnsLen; i++) { + byteptr_t fn = mPatchedFns[i]; + + // Ensure we can write to the code. + DWORD op; + if (!VirtualProtectEx(GetCurrentProcess(), fn, 2, PAGE_EXECUTE_READWRITE, &op)) { + // printf("VirtualProtectEx failed! %d\n", GetLastError()); + continue; + } + + // mov edi, edi + *((uint16_t*)fn) = 0xff8b; + + // Restore the old protection. + VirtualProtectEx(GetCurrentProcess(), fn, 2, op, &op); + + // I don't think this is actually necessary, but it can't hurt. + FlushInstructionCache(GetCurrentProcess(), + /* ignored */ NULL, + /* ignored */ 0); + } + } + + void Init(const char *modulename) + { + mModule = LoadLibraryExA(modulename, NULL, 0); + if (!mModule) { + //printf("LoadLibraryEx for '%s' failed\n", modulename); + return; + } + } + +#if defined(_M_IX86) + bool AddHook(const char *pname, intptr_t hookDest, void **origFunc) + { + if (!mModule) + return false; + + if (mPatchedFnsLen == maxPatchedFns) { + // printf ("No space for hook in mPatchedFns.\n"); + return false; + } + + byteptr_t fn = reinterpret_cast(GetProcAddress(mModule, pname)); + if (!fn) { + //printf ("GetProcAddress failed\n"); + return false; + } + + // Ensure we can read and write starting at fn - 5 (for the long jmp we're + // going to write) and ending at fn + 2 (for the short jmp up to the long + // jmp). + DWORD op; + if (!VirtualProtectEx(GetCurrentProcess(), fn - 5, 7, PAGE_EXECUTE_READWRITE, &op)) { + //printf ("VirtualProtectEx failed! %d\n", GetLastError()); + return false; + } + + bool rv = WriteHook(fn, hookDest, origFunc); + + // Re-protect, and we're done. + VirtualProtectEx(GetCurrentProcess(), fn - 5, 7, op, &op); + + if (rv) { + mPatchedFns[mPatchedFnsLen] = fn; + mPatchedFnsLen++; + } + + return rv; + } + + bool WriteHook(byteptr_t fn, intptr_t hookDest, void **origFunc) + { + // Check that the 5 bytes before fn are NOP's, and that the 2 bytes after + // fn are mov(edi, edi). + // + // It's safe to read fn[-5] because we set it to PAGE_EXECUTE_READWRITE + // before calling WriteHook. + + for (int i = -5; i <= -1; i++) { + if (fn[i] != 0x90) // nop + return false; + } + + // mov edi, edi. Yes, there are two ways to encode the same thing: + // + // 0x89ff == mov r/m, r + // 0x8bff == mov r, r/m + // + // where "r" is register and "r/m" is register or memory. Windows seems to + // use 8bff; I include 89ff out of paranoia. + if ((fn[0] != 0x8b && fn[0] != 0x89) || fn[1] != 0xff) { + return false; + } + + // Write a long jump into the space above the function. + fn[-5] = 0xe9; // jmp + *((intptr_t*)(fn - 4)) = hookDest - (uintptr_t)(fn); // target displacement + + // Set origFunc here, because after this point, hookDest might be called, + // and hookDest might use the origFunc pointer. + *origFunc = fn + 2; + + // Short jump up into our long jump. + *((uint16_t*)(fn)) = 0xf9eb; // jmp $-5 + + // I think this routine is safe without this, but it can't hurt. + FlushInstructionCache(GetCurrentProcess(), + /* ignored */ NULL, + /* ignored */ 0); + + return true; + } +#else + bool AddHook(const char *pname, intptr_t hookDest, void **origFunc) + { + // Not implemented except on x86-32. + return false; + } +#endif +}; + +class WindowsDllDetourPatcher { typedef unsigned char *byteptr_t; public: - WindowsDllInterceptor() - : mModule(0) + WindowsDllDetourPatcher() + : mModule(0), mHookPage(0), mMaxHooks(0), mCurHooks(0) { } - WindowsDllInterceptor(const char *modulename, int nhooks = 0) { - Init(modulename, nhooks); - } - - ~WindowsDllInterceptor() { + ~WindowsDllDetourPatcher() + { int i; byteptr_t p; for (i = 0, p = mHookPage; i < mCurHooks; i++, p += kHookSize) { @@ -108,7 +283,8 @@ public: } } - void Init(const char *modulename, int nhooks = 0) { + void Init(const char *modulename, int nhooks = 0) + { if (mModule) return; @@ -123,7 +299,6 @@ public: nhooks = hooksPerPage; mMaxHooks = nhooks + (hooksPerPage % nhooks); - mCurHooks = 0; mHookPage = (byteptr_t) VirtualAllocEx(GetCurrentProcess(), NULL, mMaxHooks * kHookSize, MEM_COMMIT | MEM_RESERVE, @@ -135,7 +310,13 @@ public: } } - void LockHooks() { + bool Initialized() + { + return !!mModule; + } + + void LockHooks() + { if (!mModule) return; @@ -145,9 +326,7 @@ public: mModule = 0; } - bool AddHook(const char *pname, - intptr_t hookDest, - void **origFunc) + bool AddHook(const char *pname, intptr_t hookDest, void **origFunc) { if (!mModule) return false; @@ -423,7 +602,8 @@ protected: VirtualProtectEx(GetCurrentProcess(), origFunction, nBytes, op, &op); } - byteptr_t FindTrampolineSpace() { + byteptr_t FindTrampolineSpace() + { if (mCurHooks >= mMaxHooks) return 0; @@ -435,5 +615,65 @@ protected: } }; +} // namespace internal + +class WindowsDllInterceptor +{ + internal::WindowsDllNopSpacePatcher mNopSpacePatcher; + internal::WindowsDllDetourPatcher mDetourPatcher; + + const char *mModuleName; + int mNHooks; + +public: + WindowsDllInterceptor() + : mModuleName(NULL) + , mNHooks(0) + {} + + void Init(const char *moduleName, int nhooks = 0) + { + if (mModuleName) { + return; + } + + mModuleName = moduleName; + mNHooks = nhooks; + mNopSpacePatcher.Init(moduleName); + + // Lazily initialize mDetourPatcher, since it allocates memory and we might + // not need it. + } + + void LockHooks() + { + if (mDetourPatcher.Initialized()) + mDetourPatcher.LockHooks(); + } + + bool AddHook(const char *pname, intptr_t hookDest, void **origFunc) + { + if (!mModuleName) { + // printf("AddHook before initialized?\n"); + return false; + } + + if (mNopSpacePatcher.AddHook(pname, hookDest, origFunc)) { + // printf("nopSpacePatcher succeeded.\n"); + return true; + } + + if (!mDetourPatcher.Initialized()) { + // printf("Initializing detour patcher.\n"); + mDetourPatcher.Init(mModuleName, mNHooks); + } + + bool rv = mDetourPatcher.AddHook(pname, hookDest, origFunc); + // printf("detourPatcher returned %d\n", rv); + return rv; + } +}; + +} // namespace mozilla #endif /* NS_WINDOWS_DLL_INTERCEPTOR_H_ */ diff --git a/toolkit/xre/test/win/TestDllInterceptor.cpp b/toolkit/xre/test/win/TestDllInterceptor.cpp index 948210e97f6..3df06efbe57 100644 --- a/toolkit/xre/test/win/TestDllInterceptor.cpp +++ b/toolkit/xre/test/win/TestDllInterceptor.cpp @@ -38,6 +38,8 @@ #include #include "nsWindowsDllInterceptor.h" +using namespace mozilla; + struct payload { UINT64 a; UINT64 b;