Bug 1509748 - Do not touch ntdll's PE header directly if EAF+ is enabled. r=mhowell,mstange

If EAF+ is enabled for firefox.exe, the process does not launch because we parse
the PE headers of ntdll.dll at startup, which is prohibited by EAF+.

With this patch, we skip two operations when EAF+ is enabled.

The first one is to cache ntdll's IAT at startup.  Because EAF+ is expected to
prevent an injected module from parsing PE headers and modifying IAT, we can skip
this caching safely.

The second one is to load ntdll's debug information for the profiler.  With this
patch, the profiler's callstack will not show a raw address instead of a symbol
name.  It's a bad side effect, but much better than startup crash.

Differential Revision: https://phabricator.services.mozilla.com/D76959

mozglue/misc/WindowsProcessMitigations.cpp

@@ -15,6 +15,34 @@ BOOL WINAPI GetProcessMitigationPolicy(
     SIZE_T dwLength);
 #endif  // (_WIN32_WINNT < 0x0602)
 
+// MinGW does not have these definitions in winnt.h yet
+#if defined(__MINGW32__)
+constexpr PROCESS_MITIGATION_POLICY ProcessPayloadRestrictionPolicy =
+    static_cast<PROCESS_MITIGATION_POLICY>(12);
+
+typedef struct _PROCESS_MITIGATION_PAYLOAD_RESTRICTION_POLICY {
+  union {
+    DWORD Flags;
+    struct {
+      DWORD EnableExportAddressFilter : 1;
+      DWORD AuditExportAddressFilter : 1;
+      DWORD EnableExportAddressFilterPlus : 1;
+      DWORD AuditExportAddressFilterPlus : 1;
+      DWORD EnableImportAddressFilter : 1;
+      DWORD AuditImportAddressFilter : 1;
+      DWORD EnableRopStackPivot : 1;
+      DWORD AuditRopStackPivot : 1;
+      DWORD EnableRopCallerCheck : 1;
+      DWORD AuditRopCallerCheck : 1;
+      DWORD EnableRopSimExec : 1;
+      DWORD AuditRopSimExec : 1;
+      DWORD ReservedFlags : 20;
+    } DUMMYSTRUCTNAME;
+  } DUMMYUNIONNAME;
+} PROCESS_MITIGATION_PAYLOAD_RESTRICTION_POLICY,
+    *PPROCESS_MITIGATION_PAYLOAD_RESTRICTION_POLICY;
+#endif  // defined(__MINGW32__)
+
 namespace mozilla {
 
 static decltype(&::GetProcessMitigationPolicy)
@@ -58,4 +86,20 @@ MFBT_API bool IsDynamicCodeDisabled() {
   return polInfo.ProhibitDynamicCode;
 }
 
+MFBT_API bool IsEafPlusEnabled() {
+  auto pGetProcessMitigationPolicy = FetchGetProcessMitigationPolicyFunc();
+  if (!pGetProcessMitigationPolicy) {
+    return false;
+  }
+
+  PROCESS_MITIGATION_PAYLOAD_RESTRICTION_POLICY polInfo;
+  if (!pGetProcessMitigationPolicy(::GetCurrentProcess(),
+                                   ProcessPayloadRestrictionPolicy, &polInfo,
+                                   sizeof(polInfo))) {
+    return false;
+  }
+
+  return polInfo.EnableExportAddressFilterPlus;
+}
+
 }  // namespace mozilla

mozglue/misc/WindowsProcessMitigations.h

@@ -13,6 +13,7 @@ namespace mozilla {
 
 MFBT_API bool IsWin32kLockedDown();
 MFBT_API bool IsDynamicCodeDisabled();
+MFBT_API bool IsEafPlusEnabled();
 
 }  // namespace mozilla
 

toolkit/xre/nsAppRunner.cpp

@@ -101,6 +101,7 @@
 #  include <math.h>
 #  include "cairo/cairo-features.h"
 #  include "mozilla/WindowsDllBlocklist.h"
+#  include "mozilla/WindowsProcessMitigations.h"
 #  include "mozilla/WinHeaderOnlyUtils.h"
 #  include "mozilla/mscom/ProcessRuntime.h"
 #  include "mozilla/widget/AudioSession.h"
@@ -4764,7 +4765,12 @@ int XREMain::XRE_main(int argc, char* argv[], const BootstrapConfig& aConfig) {
   // this browser process fails to launch sandbox processes because we cannot
   // copy a modified IAT into a remote process (See SandboxBroker::LaunchApp).
   // To prevent that, we cache the intact IAT before COM initialization.
-  mozilla::ipc::GeckoChildProcessHost::CacheNtDllThunk();
+  // If EAF+ is enabled, CacheNtDllThunk() causes a crash, but EAF+ will
+  // also prevent an injected module from parsing the PE headers and modifying
+  // the IAT.  Therefore, we can skip CacheNtDllThunk().
+  if (!mozilla::IsEafPlusEnabled()) {
+    mozilla::ipc::GeckoChildProcessHost::CacheNtDllThunk();
+  }
 
   // Some COM settings are global to the process and must be set before any non-
   // trivial COM is run in the application. Since these settings may affect

tools/profiler/core/shared-libraries-win32.cc

@@ -12,6 +12,7 @@
 #include "nsWindowsHelpers.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/Unused.h"
+#include "mozilla/WindowsProcessMitigations.h"
 #include "mozilla/WindowsVersion.h"
 #include "nsNativeCharsetUtils.h"
 #include "nsPrintfCString.h"
@@ -182,6 +183,11 @@ SharedLibraryInfo SharedLibraryInfo::GetInfoForSelf() {
     }
 #endif  // !defined(_M_ARM64)
 
+    // If EAF+ is enabled, parsing ntdll's PE header via GetPdbInfo() causes
+    // a crash.  We don't include PDB information in SharedLibrary.
+    bool canGetPdbInfo = (!mozilla::IsEafPlusEnabled() ||
+                          !moduleNameStr.LowerCaseEqualsLiteral("ntdll.dll"));
+
     nsCString breakpadId;
     // Load the module again to make sure that its handle will remain
     // valid as we attempt to read the PDB information from it.  We load the
@@ -205,7 +211,7 @@ SharedLibraryInfo SharedLibraryInfo::GetInfoForSelf() {
     if (handleLock &&
         sizeof(vmemInfo) ==
             VirtualQuery(module.lpBaseOfDll, &vmemInfo, sizeof(vmemInfo)) &&
-        vmemInfo.State == MEM_COMMIT &&
+        vmemInfo.State == MEM_COMMIT && canGetPdbInfo &&
         GetPdbInfo((uintptr_t)module.lpBaseOfDll, pdbSig, pdbAge, &pdbName)) {
       MOZ_ASSERT(breakpadId.IsEmpty());
       breakpadId.AppendPrintf(