Bug 1473371: Create a separate function hook type for use with cross-process DLL interceptors; r=handyman

browser/app/winlauncher/DllBlocklistWin.cpp

@@ -357,21 +357,13 @@ InitializeDllBlocklistOOP(HANDLE aChildProcess)
 {
   mozilla::CrossProcessDllInterceptor intcpt(aChildProcess);
   intcpt.Init(L"ntdll.dll");
-  bool ok = stub_NtMapViewOfSection.SetDetour(intcpt, "NtMapViewOfSection",
+  bool ok = stub_NtMapViewOfSection.SetDetour(aChildProcess, 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
@@ -407,6 +399,8 @@ 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);

mozglue/misc/nsWindowsDllInterceptor.h

@@ -84,32 +84,32 @@
 namespace mozilla {
 namespace interceptor {
 
+template <typename T>
+struct OriginalFunctionPtrTraits;
+
+template <typename R, typename... Args>
+struct OriginalFunctionPtrTraits<R (*)(Args...)>
+{
+  using ReturnType = R;
+};
+
+#if defined(_M_IX86)
+template <typename R, typename... Args>
+struct OriginalFunctionPtrTraits<R (__stdcall*)(Args...)>
+{
+  using ReturnType = R;
+};
+
+template <typename R, typename... Args>
+struct OriginalFunctionPtrTraits<R (__fastcall*)(Args...)>
+{
+  using ReturnType = R;
+};
+#endif // defined(_M_IX86)
+
 template <typename InterceptorT, typename FuncPtrT>
 class FuncHook final
 {
-  template <typename T>
-  struct OriginalFunctionPtrTraits;
-
-  template <typename R, typename... Args>
-  struct OriginalFunctionPtrTraits<R (*)(Args...)>
-  {
-    using ReturnType = R;
-  };
-
-#if defined(_M_IX86)
-  template <typename R, typename... Args>
-  struct OriginalFunctionPtrTraits<R (__stdcall*)(Args...)>
-  {
-    using ReturnType = R;
-  };
-
-  template <typename R, typename... Args>
-  struct OriginalFunctionPtrTraits<R (__fastcall*)(Args...)>
-  {
-    using ReturnType = R;
-  };
-#endif // defined(_M_IX86)
-
 public:
   using ThisType = FuncHook<InterceptorT, FuncPtrT>;
   using ReturnType = typename OriginalFunctionPtrTraits<FuncPtrT>::ReturnType;
@@ -221,15 +221,96 @@ private:
   INIT_ONCE mInitOnce;
 };
 
+template <typename InterceptorT, typename FuncPtrT>
+class MOZ_ONLY_USED_TO_AVOID_STATIC_CONSTRUCTORS FuncHookCrossProcess final
+{
+public:
+  using ThisType = FuncHookCrossProcess<InterceptorT, FuncPtrT>;
+  using ReturnType = typename OriginalFunctionPtrTraits<FuncPtrT>::ReturnType;
+
+  FuncHookCrossProcess() = default;
+  ~FuncHookCrossProcess() = default;
+
+  bool Set(HANDLE aProcess, InterceptorT& aInterceptor, const char* aName,
+           FuncPtrT aHookDest)
+  {
+    if (!aInterceptor.AddHook(aName, reinterpret_cast<intptr_t>(aHookDest),
+                              reinterpret_cast<void**>(&mOrigFunc))) {
+      return false;
+    }
+
+    return CopyStubToChildProcess(aProcess);
+  }
+
+  bool SetDetour(HANDLE aProcess, InterceptorT& aInterceptor, const char* aName,
+                 FuncPtrT aHookDest)
+  {
+    if (!aInterceptor.AddDetour(aName, reinterpret_cast<intptr_t>(aHookDest),
+                                reinterpret_cast<void**>(&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 <typename... ArgsType>
+  ReturnType operator()(ArgsType... aArgs) const
+  {
+    return mOrigFunc(std::forward<ArgsType>(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 <typename MMPolicyT, typename InterceptorT>
+struct TypeResolver;
+
+template <typename InterceptorT>
+struct TypeResolver<mozilla::interceptor::MMPolicyInProcess, InterceptorT>
+{
+  template <typename FuncPtrT>
+  using FuncHookType = FuncHook<InterceptorT, FuncPtrT>;
+};
+
+template <typename InterceptorT>
+struct TypeResolver<mozilla::interceptor::MMPolicyOutOfProcess, InterceptorT>
+{
+  template <typename FuncPtrT>
+  using FuncHookType = FuncHookCrossProcess<InterceptorT, FuncPtrT>;
+};
+
 template <typename VMPolicy =
             mozilla::interceptor::VMSharingPolicyShared<
               mozilla::interceptor::MMPolicyInProcess, kDefaultTrampolineSize>>
-class WindowsDllInterceptor final
+class WindowsDllInterceptor final : public TypeResolver<typename VMPolicy::MMPolicyT,
+                                                        WindowsDllInterceptor<VMPolicy>>
 {
   typedef WindowsDllInterceptor<VMPolicy> ThisType;
 
@@ -372,13 +453,12 @@ private:
     return mDetourPatcher.AddHook(aProc, aHookDest, aOrigFunc);
   }
 
-public:
-  template <typename FuncPtrT>
-  using FuncHookType = FuncHook<ThisType, FuncPtrT>;
-
 private:
   template <typename InterceptorT, typename FuncPtrT>
   friend class FuncHook;
+
+  template <typename InterceptorT, typename FuncPtrT>
+  friend class FuncHookCrossProcess;
 };
 
 } // namespace interceptor

mozglue/tests/interceptor/TestDllInterceptorCrossProcess.cpp

@@ -73,22 +73,14 @@ int ParentMain()
   mozilla::CrossProcessDllInterceptor intcpt(childProcess.get());
   intcpt.Init("TestDllInterceptorCrossProcess.exe");
 
-  if (!gOrigReturnResult.Set(intcpt, "ReturnResult", &ReturnResultHook)) {
+  if (!gOrigReturnResult.Set(childProcess.get(), 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<DWORD>(-1)) {
     printf("TEST-UNEXPECTED-FAIL | DllInterceptorCrossProcess | Failed to resume child thread\n");
     return 1;