Bug 1445601 - Stop using LoadLibraryA in nsWindowsDllInterceptor. r=aklotz draft
authorMasatoshi Kimura <VYV03354@nifty.ne.jp>
Sat, 24 Mar 2018 22:12:38 +0900
changeset 777873 8debc4b3802000508234ed751b55b9e1ecedc2c7
parent 777562 071ee904485e21e19ca08456d32bce6825b77a26
child 777874 7e2646655411194f4d12f0fc0fdff5bbbe3b5e23
push id105316
push userVYV03354@nifty.ne.jp
push dateThu, 05 Apr 2018 12:57:18 +0000
reviewersaklotz
bugs1445601
milestone61.0a1
Bug 1445601 - Stop using LoadLibraryA in nsWindowsDllInterceptor. r=aklotz MozReview-Commit-ID: 6yZiSFJ2Eot
dom/plugins/ipc/FunctionHook.cpp
toolkit/xre/test/win/TestDllInterceptor.cpp
xpcom/build/nsWindowsDllInterceptor.h
--- a/dom/plugins/ipc/FunctionHook.cpp
+++ b/dom/plugins/ipc/FunctionHook.cpp
@@ -53,28 +53,31 @@ FunctionHook::HookFunctions(int aQuirks)
   }
 }
 
 #if defined(XP_WIN)
 
 // This cache is created when a DLL is registered with a FunctionHook.
 // It is cleared on a call to ClearDllInterceptorCache().  It
 // must be freed before exit to avoid leaks.
-typedef nsClassHashtable<nsCStringHashKey, WindowsDllInterceptor> DllInterceptors;
+typedef nsClassHashtable<nsStringHashKey, WindowsDllInterceptor> DllInterceptors;
 DllInterceptors* sDllInterceptorCache = nullptr;
 
 WindowsDllInterceptor*
 FunctionHook::GetDllInterceptorFor(const char* aModuleName)
 {
   if (!sDllInterceptorCache) {
     sDllInterceptorCache = new DllInterceptors();
   }
 
+  MOZ_ASSERT(NS_IsAscii(aModuleName),
+             "Non-ASCII module names are not supported");
+  NS_ConvertASCIItoUTF16 moduleName(aModuleName);
   WindowsDllInterceptor* ret =
-    sDllInterceptorCache->LookupOrAdd(nsCString(aModuleName), aModuleName);
+    sDllInterceptorCache->LookupOrAdd(moduleName, moduleName.get());
   MOZ_ASSERT(ret);
   return ret;
 }
 
 void
 FunctionHook::ClearDllInterceptorCache()
 {
   delete sDllInterceptorCache;
--- a/toolkit/xre/test/win/TestDllInterceptor.cpp
+++ b/toolkit/xre/test/win/TestDllInterceptor.cpp
@@ -56,17 +56,18 @@ bool CheckHook(HookTestFunc aHookTestFun
            "Executed hooked function %s from %s\n", aFuncName, aDllName);
     return true;
   }
   printf("TEST-FAILED | WindowsDllInterceptor | "
          "Failed to execute hooked function %s from %s\n", aFuncName, aDllName);
   return false;
 }
 
-bool TestHook(HookTestFunc funcTester, const char *dll, const char *func)
+template <size_t N>
+bool TestHook(HookTestFunc funcTester, const char (&dll)[N], const char *func)
 {
   void *orig_func;
   bool successful = false;
   {
     WindowsDllInterceptor TestIntercept;
     TestIntercept.Init(dll);
     successful = TestIntercept.AddHook(func, 0, &orig_func);
   }
@@ -75,17 +76,18 @@ bool TestHook(HookTestFunc funcTester, c
     printf("TEST-PASS | WindowsDllInterceptor | Could hook %s from %s\n", func, dll);
     return CheckHook(funcTester, orig_func, dll, func);
   } else {
     printf("TEST-UNEXPECTED-FAIL | WindowsDllInterceptor | Failed to hook %s from %s\n", func, dll);
     return false;
   }
 }
 
-bool TestDetour(const char *dll, const char *func)
+template <size_t N>
+bool TestDetour(const char (&dll)[N], const char *func)
 {
   void *orig_func;
   bool successful = false;
   {
     WindowsDllInterceptor TestIntercept;
     TestIntercept.Init(dll);
     successful = TestIntercept.AddDetour(func, 0, &orig_func);
   }
@@ -94,17 +96,18 @@ bool TestDetour(const char *dll, const c
     printf("TEST-PASS | WindowsDllInterceptor | Could detour %s from %s\n", func, dll);
     return true;
   } else {
     printf("TEST-UNEXPECTED-FAIL | WindowsDllInterceptor | Failed to detour %s from %s\n", func, dll);
     return false;
   }
 }
 
-bool MaybeTestHook(const bool cond, HookTestFunc funcTester, const char* dll, const char* func)
+template <size_t N>
+bool MaybeTestHook(const bool cond, HookTestFunc funcTester, const char (&dll)[N], const char* func)
 {
   if (!cond) {
     printf("TEST-SKIPPED | WindowsDllInterceptor | Skipped hook test for %s from %s\n", func, dll);
     return true;
   }
 
   return TestHook(funcTester, dll, func);
 }
--- a/xpcom/build/nsWindowsDllInterceptor.h
+++ b/xpcom/build/nsWindowsDllInterceptor.h
@@ -149,30 +149,26 @@ public:
 
       // I don't think this is actually necessary, but it can't hurt.
       FlushInstructionCache(GetCurrentProcess(),
                             /* ignored */ nullptr,
                             /* ignored */ 0);
     }
   }
 
-  void Init(const char* aModuleName)
+  void Init(HMODULE aModule)
   {
     if (!IsCompatible()) {
 #if defined(MOZILLA_INTERNAL_API)
       NS_WARNING("NOP space patching is unavailable for compatibility reasons");
 #endif
       return;
     }
 
-    mModule = LoadLibraryExA(aModuleName, nullptr, 0);
-    if (!mModule) {
-      //printf("LoadLibraryEx for '%s' failed\n", aModuleName);
-      return;
-    }
+    mModule = aModule;
   }
 
   /**
    * NVIDIA Optimus drivers utilize Microsoft Detours 2.x to patch functions
    * in our address space. There is a bug in Detours 2.x that causes it to
    * patch at the wrong address when attempting to detour code that is already
    * NOP space patched. This function is an effort to detect the presence of
    * this NVIDIA code in our address space and disable NOP space patching if it
@@ -357,17 +353,17 @@ private:
     // we resolve redirected address from import table.
     if (aOriginalFunction[0] == 0xff && aOriginalFunction[1] == 0x25) {
       return (byteptr_t)(**((uint32_t**) (aOriginalFunction + 2)));
     }
 
     return aOriginalFunction;
   }
 #else
-  void Init(const char* aModuleName)
+  void Init(HMODULE aModule)
   {
     // Not implemented except on x86-32.
   }
 
   bool AddHook(const char* aName, intptr_t aHookDest, void** aOrigFunc)
   {
     // Not implemented except on x86-32.
     return false;
@@ -419,27 +415,23 @@ public:
         continue;
       *((intptr_t*)(origBytes + 2)) = dest;
 #else
 #error "Unknown processor type"
 #endif
     }
   }
 
-  void Init(const char* aModuleName, int aNumHooks = 0)
+  void Init(HMODULE aModule, int aNumHooks = 0)
   {
-    if (mModule) {
+    if (mModule || !aModule) {
       return;
     }
 
-    mModule = LoadLibraryExA(aModuleName, nullptr, 0);
-    if (!mModule) {
-      //printf("LoadLibraryEx for '%s' failed\n", aModuleName);
-      return;
-    }
+    mModule = aModule;
 
     int hooksPerPage = 4096 / kHookSize;
     if (aNumHooks == 0) {
       aNumHooks = hooksPerPage;
     }
 
     mMaxHooks = aNumHooks + (hooksPerPage % aNumHooks);
 
@@ -1404,39 +1396,68 @@ protected:
 
 } // namespace internal
 
 class WindowsDllInterceptor
 {
   internal::WindowsDllNopSpacePatcher mNopSpacePatcher;
   internal::WindowsDllDetourPatcher mDetourPatcher;
 
-  const char* mModuleName;
+  HMODULE mModule;
   int mNHooks;
 
 public:
-  explicit WindowsDllInterceptor(const char* aModuleName = nullptr,
+  explicit WindowsDllInterceptor()
+    : mModule(nullptr)
+    , mNHooks(0)
+  {
+  }
+
+  template <size_t N>
+  explicit WindowsDllInterceptor(const char (&aModuleName)[N],
                                  int aNumHooks = 0)
-    : mModuleName(nullptr)
-    , mNHooks(0)
+    : WindowsDllInterceptor()
+  {
+    Init(aModuleName, aNumHooks);
+  }
+
+  explicit WindowsDllInterceptor(const wchar_t* aModuleName,
+                                 int aNumHooks = 0)
+    : WindowsDllInterceptor()
   {
     if (aModuleName) {
       Init(aModuleName, aNumHooks);
     }
   }
 
-  void Init(const char* aModuleName, int aNumHooks = 0)
+  template <size_t N>
+  void Init(const char (&aModuleName)[N], int aNumHooks = 0)
   {
-    if (mModuleName) {
+    wchar_t moduleName[N];
+    for (size_t i = 0; i < N; ++i) {
+      MOZ_ASSERT(!(aModuleName[i] & 0x80),
+                 "Use wchar_t* overload for non-ASCII module names");
+      moduleName[i] = aModuleName[i];
+    }
+    Init(moduleName, aNumHooks);
+  }
+
+  void Init(const wchar_t* aModuleName, int aNumHooks = 0)
+  {
+    if (mModule) {
       return;
     }
 
-    mModuleName = aModuleName;
+    mModule = LoadLibraryExW(aModuleName, nullptr, 0);
+    if (!mModule) {
+      //printf("LoadLibraryEx for '%S' failed\n", aModuleName);
+      return;
+    }
     mNHooks = aNumHooks;
-    mNopSpacePatcher.Init(aModuleName);
+    mNopSpacePatcher.Init(mModule);
 
     // Lazily initialize mDetourPatcher, since it allocates memory and we might
     // not need it.
   }
 
   /**
    * Hook/detour the method aName from the DLL we set in Init so that it calls
    * aHookDest instead.  Returns the original method pointer in aOrigFunc
@@ -1447,17 +1468,17 @@ public:
    * succeeds now, updates to the hooked DLL could cause it to fail in
    * the future.
    */
   bool AddHook(const char* aName, intptr_t aHookDest, void** aOrigFunc)
   {
     // Use a nop space patch if possible, otherwise fall back to a detour.
     // This should be the preferred method for adding hooks.
 
-    if (!mModuleName) {
+    if (!mModule) {
       return false;
     }
 
     if (mNopSpacePatcher.AddHook(aName, aHookDest, aOrigFunc)) {
       return true;
     }
 
     return AddDetour(aName, aHookDest, aOrigFunc);
@@ -1473,22 +1494,22 @@ public:
    * succeeds now, updates to the detoured DLL could cause it to fail in
    * the future.
    */
   bool AddDetour(const char* aName, intptr_t aHookDest, void** aOrigFunc)
   {
     // Generally, code should not call this method directly. Use AddHook unless
     // there is a specific need to avoid nop space patches.
 
-    if (!mModuleName) {
+    if (!mModule) {
       return false;
     }
 
     if (!mDetourPatcher.Initialized()) {
-      mDetourPatcher.Init(mModuleName, mNHooks);
+      mDetourPatcher.Init(mModule, mNHooks);
     }
 
     return mDetourPatcher.AddHook(aName, aHookDest, aOrigFunc);
   }
 };
 
 } // namespace mozilla