Bug 1523526 - Don't allow CFG on old releases of Windows for arm64 r=bobowen,aklotz
☠☠ backed out by 8fe96c8d21b7 ☠ ☠
authorDavid Major <dmajor@mozilla.com>
Tue, 07 May 2019 18:37:13 +0000
changeset 474527 e8ac4b512f9d03dae670be39b399ea8189e1ca05
parent 474526 6df30b02e311b72fcee1c45161eec060bc3f9ce9
child 474528 98013639d60026eb3a0344fa499a321aec176d2b
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());
     }
   }