Bug 951827 - Force a detour-style hook for LdrLoadDll. r=ehsan

mozglue/build/WindowsDllBlocklist.cpp

@@ -614,7 +614,10 @@ DllBlocklist_Initialize()
 
   ReentrancySentinel::InitializeStatics();
 
-  bool ok = NtDllIntercept.AddHook("LdrLoadDll", reinterpret_cast<intptr_t>(patched_LdrLoadDll), (void**) &stub_LdrLoadDll);
+  // We specifically use a detour, because there are cases where external
+  // code also tries to hook LdrLoadDll, and doesn't know how to relocate our
+  // nop space patches. (Bug 951827)
+  bool ok = NtDllIntercept.AddDetour("LdrLoadDll", reinterpret_cast<intptr_t>(patched_LdrLoadDll), (void**) &stub_LdrLoadDll);
 
   if (!ok) {
     sBlocklistInitFailed = true;

toolkit/xre/nsWindowsDllInterceptor.h

@@ -671,24 +671,34 @@ public:
 
   bool AddHook(const char *pname, intptr_t hookDest, void **origFunc)
   {
+    // Use a nop space patch if possible, otherwise fall back to a detour.
+    // This should be the preferred method for adding hooks.
+
     if (!mModuleName) {
-      // printf("AddHook before initialized?\n");
       return false;
     }
 
     if (mNopSpacePatcher.AddHook(pname, hookDest, origFunc)) {
-      // printf("nopSpacePatcher succeeded.\n");
       return true;
     }
 
+    return AddDetour(pname, hookDest, origFunc);
+  }
+
+  bool AddDetour(const char *pname, intptr_t hookDest, void **origFunc)
+  {
+    // Generally, code should not call this method directly. Use AddHook unless
+    // there is a specific need to avoid nop space patches.
+
+    if (!mModuleName) {
+      return false;
+    }
+
     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;
+    return mDetourPatcher.AddHook(pname, hookDest, origFunc);
   }
 };
 

toolkit/xre/test/win/TestDllInterceptor.cpp

@@ -57,6 +57,25 @@ bool TestHook(const char *dll, const char *func)
   }
 }
 
+bool TestDetour(const char *dll, const char *func)
+{
+  void *orig_func;
+  bool successful = false;
+  {
+    WindowsDllInterceptor TestIntercept;
+    TestIntercept.Init(dll);
+    successful = TestIntercept.AddDetour(func, 0, &orig_func);
+  }
+
+  if (successful) {
+    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;
+  }
+}
+
 int main()
 {
   payload initial = { 0x12345678, 0xfc4e9d31, 0x87654321 };
@@ -139,7 +158,7 @@ int main()
       TestHook("kernel32.dll", "MapViewOfFile") &&
       TestHook("gdi32.dll", "CreateDIBSection") &&
 #endif
-      TestHook("ntdll.dll", "LdrLoadDll")) {
+      TestDetour("ntdll.dll", "LdrLoadDll")) {
     printf("TEST-PASS | WindowsDllInterceptor | all checks passed\n");
     return 0;
   }