Bug 1523526 - Don't allow CFG on old releases of Windows for arm64 r=bobowen,aklotz
authorDavid Major <dmajor@mozilla.com>
Mon, 20 May 2019 17:25:30 +0000
changeset 474611 65f1f6d80936cb7557183664b37038d83cda64c1
parent 474610 3e5aa7cb74603d4871493847d5644f1cc7919fa8
child 474612 c34880f0f63009f9bafaf5e5064c8c30085618c1
push id36042
push userdvarga@mozilla.com
push dateTue, 21 May 2019 04:19:40 +0000
treeherdermozilla-central@ca560ff55451 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbobowen, aklotz
bugs1523526
milestone69.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1523526 - Don't allow CFG on old releases of Windows for arm64 r=bobowen,aklotz There's a bug in ole32.dll on arm64 versions of Windows prior to 1809, that crashes our content processes if we enable CFG. We've reported the issue, but even if it gets fixed, we can't assume users will have the update. This patch uses process mitigation policy flags to disable CFG on arm64 before 1809. Based on testing, we only need to do this in the sandbox for child processes, and it's not strictly necessary for the launcher stub to set the flag on the main process. But I've included that anyway as a guard against some yet-undiscovered scenario that might hit the issue and make the browser unusable. The effects of this patch won't be visible until we actually enable CFG in a subsequent landing. Differential Revision: https://phabricator.services.mozilla.com/D29474
browser/app/winlauncher/LauncherProcessWin.cpp
security/sandbox/chromium-shim/base/win/sdkdecls.h
security/sandbox/chromium-shim/patches/with_update/aarch64_control_flow_guard.patch
security/sandbox/chromium-shim/patches/with_update/patch_order.txt
security/sandbox/chromium/sandbox/win/src/process_mitigations.cc
security/sandbox/chromium/sandbox/win/src/security_level.h
security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
--- a/browser/app/winlauncher/LauncherProcessWin.cpp
+++ b/browser/app/winlauncher/LauncherProcessWin.cpp
@@ -55,32 +55,45 @@ static mozilla::LauncherVoidResult PostC
 }
 
 #if !defined( \
     PROCESS_CREATION_MITIGATION_POLICY_IMAGE_LOAD_PREFER_SYSTEM32_ALWAYS_ON)
 #  define PROCESS_CREATION_MITIGATION_POLICY_IMAGE_LOAD_PREFER_SYSTEM32_ALWAYS_ON \
     (0x00000001ULL << 60)
 #endif  // !defined(PROCESS_CREATION_MITIGATION_POLICY_IMAGE_LOAD_PREFER_SYSTEM32_ALWAYS_ON)
 
+#if !defined(PROCESS_CREATION_MITIGATION_POLICY_CONTROL_FLOW_GUARD_ALWAYS_OFF)
+#  define PROCESS_CREATION_MITIGATION_POLICY_CONTROL_FLOW_GUARD_ALWAYS_OFF \
+    (0x00000002ULL << 40)
+#endif  // !defined(PROCESS_CREATION_MITIGATION_POLICY_CONTROL_FLOW_GUARD_ALWAYS_OFF)
+
 #if (_WIN32_WINNT < 0x0602)
 BOOL WINAPI
 SetProcessMitigationPolicy(PROCESS_MITIGATION_POLICY aMitigationPolicy,
                            PVOID aBuffer, SIZE_T aBufferLen);
 #endif  // (_WIN32_WINNT >= 0x0602)
 
 /**
  * Any mitigation policies that should be set on the browser process should go
  * here.
  */
 static void SetMitigationPolicies(mozilla::ProcThreadAttributes& aAttrs,
                                   const bool aIsSafeMode) {
   if (mozilla::IsWin10AnniversaryUpdateOrLater()) {
     aAttrs.AddMitigationPolicy(
         PROCESS_CREATION_MITIGATION_POLICY_IMAGE_LOAD_PREFER_SYSTEM32_ALWAYS_ON);
   }
+
+#if defined(_M_ARM64)
+  // Disable CFG on older versions of ARM64 Windows to avoid a crash in COM.
+  if (!mozilla::IsWin10Sep2018UpdateOrLater()) {
+    aAttrs.AddMitigationPolicy(
+        PROCESS_CREATION_MITIGATION_POLICY_CONTROL_FLOW_GUARD_ALWAYS_OFF);
+  }
+#endif  // defined(_M_ARM64)
 }
 
 static mozilla::LauncherFlags ProcessCmdLine(int& aArgc, wchar_t* aArgv[]) {
   mozilla::LauncherFlags result = mozilla::LauncherFlags::eNone;
 
   if (mozilla::CheckArg(aArgc, aArgv, L"wait-for-browser",
                         static_cast<const wchar_t**>(nullptr),
                         mozilla::CheckArgFlag::RemoveArg) ==
--- a/security/sandbox/chromium-shim/base/win/sdkdecls.h
+++ b/security/sandbox/chromium-shim/base/win/sdkdecls.h
@@ -206,16 +206,27 @@ typedef struct _PROCESS_MITIGATION_EXTEN
 
 #define PROCESS_CREATION_MITIGATION_POLICY_PROHIBIT_DYNAMIC_CODE_MASK                     (0x00000003uLL << 36)
 #define PROCESS_CREATION_MITIGATION_POLICY_PROHIBIT_DYNAMIC_CODE_DEFER                    (0x00000000uLL << 36)
 #define PROCESS_CREATION_MITIGATION_POLICY_PROHIBIT_DYNAMIC_CODE_ALWAYS_ON                (0x00000001uLL << 36)
 #define PROCESS_CREATION_MITIGATION_POLICY_PROHIBIT_DYNAMIC_CODE_ALWAYS_OFF               (0x00000002uLL << 36)
 #define PROCESS_CREATION_MITIGATION_POLICY_PROHIBIT_DYNAMIC_CODE_ALWAYS_ON_ALLOW_OPT_OUT  (0x00000003uLL << 36)
 
 //
+// Define Control Flow Guard (CFG) mitigation policy options.  Control Flow
+// Guard allows indirect control transfers to be checked at runtime.
+//
+
+#define PROCESS_CREATION_MITIGATION_POLICY_CONTROL_FLOW_GUARD_MASK                        (0x00000003uLL << 40)
+#define PROCESS_CREATION_MITIGATION_POLICY_CONTROL_FLOW_GUARD_DEFER                       (0x00000000uLL << 40)
+#define PROCESS_CREATION_MITIGATION_POLICY_CONTROL_FLOW_GUARD_ALWAYS_ON                   (0x00000001uLL << 40)
+#define PROCESS_CREATION_MITIGATION_POLICY_CONTROL_FLOW_GUARD_ALWAYS_OFF                  (0x00000002uLL << 40)
+#define PROCESS_CREATION_MITIGATION_POLICY_CONTROL_FLOW_GUARD_EXPORT_SUPPRESSION          (0x00000003uLL << 40)
+
+//
 // Define module signature options.  When enabled, this option will
 // block mapping of non-microsoft binaries.
 //
 
 #define PROCESS_CREATION_MITIGATION_POLICY_BLOCK_NON_MICROSOFT_BINARIES_MASK              (0x00000003uLL << 44)
 #define PROCESS_CREATION_MITIGATION_POLICY_BLOCK_NON_MICROSOFT_BINARIES_DEFER             (0x00000000uLL << 44)
 #define PROCESS_CREATION_MITIGATION_POLICY_BLOCK_NON_MICROSOFT_BINARIES_ALWAYS_ON         (0x00000001uLL << 44)
 #define PROCESS_CREATION_MITIGATION_POLICY_BLOCK_NON_MICROSOFT_BINARIES_ALWAYS_OFF        (0x00000002uLL << 44)
new file mode 100644
--- /dev/null
+++ b/security/sandbox/chromium-shim/patches/with_update/aarch64_control_flow_guard.patch
@@ -0,0 +1,44 @@
+Bug 1523526: Don't allow CFG on old releases of Windows for arm64
+
+There's a bug in ole32.dll on arm64 versions of Windows prior to 1809, that crashes our content processes if we enable CFG. We've reported the issue, but even if it gets fixed, we can't assume users will have the update.
+
+This patch uses process mitigation policy flags to disable CFG on arm64 before 1809. Based on testing, we only need to do this in the sandbox for child processes, and it's not strictly necessary for the launcher stub to set the flag on the main process. But I've included that anyway as a guard against some yet-undiscovered scenario that might hit the issue and make the browser unusable.
+
+The effects of this patch won't be visible until we actually enable CFG in a subsequent landing.
+
+Differential Revision: https://phabricator.services.mozilla.com/D29474
+
+diff --git a/security/sandbox/chromium/sandbox/win/src/process_mitigations.cc b/security/sandbox/chromium/sandbox/win/src/process_mitigations.cc
+--- a/security/sandbox/chromium/sandbox/win/src/process_mitigations.cc
++++ b/security/sandbox/chromium/sandbox/win/src/process_mitigations.cc
+@@ -354,6 +354,11 @@ void ConvertProcessMitigationsToPolicy(M
+         PROCESS_CREATION_MITIGATION_POLICY_PROHIBIT_DYNAMIC_CODE_ALWAYS_ON;
+   }
+ 
++  if (flags & MITIGATION_CONTROL_FLOW_GUARD_DISABLE) {
++    *policy_flags |=
++        PROCESS_CREATION_MITIGATION_POLICY_CONTROL_FLOW_GUARD_ALWAYS_OFF;
++  }
++
+   if (version < base::win::VERSION_WIN10)
+     return;
+ 
+diff --git a/security/sandbox/chromium/sandbox/win/src/security_level.h b/security/sandbox/chromium/sandbox/win/src/security_level.h
+--- a/security/sandbox/chromium/sandbox/win/src/security_level.h
++++ b/security/sandbox/chromium/sandbox/win/src/security_level.h
+@@ -255,6 +255,15 @@ const MitigationFlags MITIGATION_IMAGE_L
+ // PROCESS_CREATION_MITIGATION_POLICY_IMAGE_LOAD_PREFER_SYSTEM32_ALWAYS_ON.
+ const MitigationFlags MITIGATION_IMAGE_LOAD_PREFER_SYS32 = 0x00100000;
+ 
++// Begin Mozilla-added flags.
++// Working down from the high bit to avoid conflict with new upstream flags.
++
++// Disable Control Flow Guard. This may seem more like an anti-mitigation, but
++// this flag allows code to make targeted changes to CFG to avoid bugs, while
++// leaving it enabled in the common case. Corresponds to
++// PROCESS_CREATION_MITIGATION_POLICY_CONTROL_FLOW_GUARD_ALWAYS_ON.
++const MitigationFlags MITIGATION_CONTROL_FLOW_GUARD_DISABLE = 0x80000000;
++
+ }  // namespace sandbox
+ 
+ #endif  // SANDBOX_SRC_SECURITY_LEVEL_H_
--- a/security/sandbox/chromium-shim/patches/with_update/patch_order.txt
+++ b/security/sandbox/chromium-shim/patches/with_update/patch_order.txt
@@ -16,8 +16,9 @@ mingw_duplicate_instatinations.patch
 mingw_copy_s.patch
 mingw_operator_new.patch
 mingw_cast_getprocaddress.patch
 mingw_capitalization.patch
 mingw_disable_one_try.patch
 mingw_noexports_casts.patch
 mingw_offsetof.patch
 add_aarch64_windows_support.patch
+aarch64_control_flow_guard.patch
--- a/security/sandbox/chromium/sandbox/win/src/process_mitigations.cc
+++ b/security/sandbox/chromium/sandbox/win/src/process_mitigations.cc
@@ -349,16 +349,21 @@ void ConvertProcessMitigationsToPolicy(M
   if (version < base::win::VERSION_WIN8_1)
     return;
 
   if (flags & MITIGATION_DYNAMIC_CODE_DISABLE) {
     *policy_flags |=
         PROCESS_CREATION_MITIGATION_POLICY_PROHIBIT_DYNAMIC_CODE_ALWAYS_ON;
   }
 
+  if (flags & MITIGATION_CONTROL_FLOW_GUARD_DISABLE) {
+    *policy_flags |=
+        PROCESS_CREATION_MITIGATION_POLICY_CONTROL_FLOW_GUARD_ALWAYS_OFF;
+  }
+
   if (version < base::win::VERSION_WIN10)
     return;
 
   if (flags & MITIGATION_NONSYSTEM_FONT_DISABLE) {
     *policy_flags |= PROCESS_CREATION_MITIGATION_POLICY_FONT_DISABLE_ALWAYS_ON;
   }
 
   // Threshold 2
--- a/security/sandbox/chromium/sandbox/win/src/security_level.h
+++ b/security/sandbox/chromium/sandbox/win/src/security_level.h
@@ -250,11 +250,20 @@ const MitigationFlags MITIGATION_IMAGE_L
 
 // Forces image load preference to prioritize the Windows install System32
 // folder before dll load dir, application dir and any user dirs set.
 // - Affects IAT resolution standard search path only, NOT direct LoadLibrary or
 //   executable search path.
 // PROCESS_CREATION_MITIGATION_POLICY_IMAGE_LOAD_PREFER_SYSTEM32_ALWAYS_ON.
 const MitigationFlags MITIGATION_IMAGE_LOAD_PREFER_SYS32 = 0x00100000;
 
+// Begin Mozilla-added flags.
+// Working down from the high bit to avoid conflict with new upstream flags.
+
+// Disable Control Flow Guard. This may seem more like an anti-mitigation, but
+// this flag allows code to make targeted changes to CFG to avoid bugs, while
+// leaving it enabled in the common case. Corresponds to
+// PROCESS_CREATION_MITIGATION_POLICY_CONTROL_FLOW_GUARD_ALWAYS_ON.
+const MitigationFlags MITIGATION_CONTROL_FLOW_GUARD_DISABLE = 0x80000000;
+
 }  // namespace sandbox
 
 #endif  // SANDBOX_SRC_SECURITY_LEVEL_H_
--- a/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
+++ b/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
@@ -503,16 +503,23 @@ void SandboxBroker::SetSecurityLevelForC
       "SetDelayedIntegrityLevel should never fail, what happened?");
 
   sandbox::MitigationFlags mitigations =
       sandbox::MITIGATION_BOTTOM_UP_ASLR | sandbox::MITIGATION_HEAP_TERMINATE |
       sandbox::MITIGATION_SEHOP | sandbox::MITIGATION_DEP_NO_ATL_THUNK |
       sandbox::MITIGATION_DEP | sandbox::MITIGATION_EXTENSION_POINT_DISABLE |
       sandbox::MITIGATION_IMAGE_LOAD_PREFER_SYS32;
 
+#if defined(_M_ARM64)
+  // Disable CFG on older versions of ARM64 Windows to avoid a crash in COM.
+  if (!IsWin10Sep2018UpdateOrLater()) {
+    mitigations |= sandbox::MITIGATION_CONTROL_FLOW_GUARD_DISABLE;
+  }
+#endif
+
   if (aSandboxLevel > 4) {
     result = mPolicy->SetAlternateDesktop(false);
     if (NS_WARN_IF(result != sandbox::SBOX_ALL_OK)) {
       LOG_W("SetAlternateDesktop failed, result: %i, last error: %x", result,
             ::GetLastError());
     }
   }