Bug 1614885 - Do not attempt to bootstrap a child process if the launcher failed to boostrap the browser process. r=aklotz, a=RyanVM
authorToshihito Kikuchi <tkikuchi@mozilla.com>
Wed, 12 Feb 2020 17:44:13 +0000
changeset 574980 806d21c68f121efe042da7904920b58a2d454670
parent 574979 2798e8ae6c25535b1441388e9ceedacf865cc3b8
child 574981 759e62765cc41f186d0981e84708465dc41acbec
push id2266
push userryanvm@gmail.com
push dateMon, 17 Feb 2020 14:21:56 +0000
treeherdermozilla-release@806d21c68f12 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaklotz, RyanVM
bugs1614885, 1522830
milestone73.0.1
Bug 1614885 - Do not attempt to bootstrap a child process if the launcher failed to boostrap the browser process. r=aklotz, a=RyanVM Bug 1522830 added the call to `InitializeDllBlocklistOOP` in `SandboxBroker::LaunchApp` to enable the new dll blocklist and telemetry in sandbox processes. If the browser process fails to bootstrap a process for some reason, firefox starts without any crash nor any content processes because of that change. What is worse is that this problem persists even after the launcher process was disabled. To mitigate it, this patch stops an attempt to bootstrap a child process if the launcher process already failed to do it. With this, if something bad happens in the first launch, the launcher process is automatically disabled via registry and next time firefox will work normally. So a user will see the launching problem only once. We will follow up the bootstrap issue. Differential Revision: https://phabricator.services.mozilla.com/D62636
browser/app/winlauncher/DllBlocklistInit.cpp
browser/app/winlauncher/DllBlocklistInit.h
browser/app/winlauncher/LauncherProcessWin.cpp
--- a/browser/app/winlauncher/DllBlocklistInit.cpp
+++ b/browser/app/winlauncher/DllBlocklistInit.cpp
@@ -28,19 +28,24 @@ namespace mozilla {
 // This DLL blocking code is incompatible with ASAN because
 // it is able to execute before ASAN itself has even initialized.
 // Also, AArch64 has not been tested with this.
 LauncherVoidResultWithLineInfo InitializeDllBlocklistOOP(
     const wchar_t* aFullImagePath, HANDLE aChildProcess) {
   return mozilla::Ok();
 }
 
+LauncherVoidResultWithLineInfo InitializeDllBlocklistOOPFromLauncher(
+    const wchar_t* aFullImagePath, HANDLE aChildProcess) {
+  return mozilla::Ok();
+}
+
 #else
 
-LauncherVoidResultWithLineInfo InitializeDllBlocklistOOP(
+static LauncherVoidResultWithLineInfo InitializeDllBlocklistOOPInternal(
     const wchar_t* aFullImagePath, HANDLE aChildProcess) {
   CrossProcessDllInterceptor intcpt(aChildProcess);
   intcpt.Init(L"ntdll.dll");
 
   bool ok = freestanding::stub_NtMapViewOfSection.SetDetour(
       aChildProcess, intcpt, "NtMapViewOfSection",
       &freestanding::patched_NtMapViewOfSection);
   if (!ok) {
@@ -122,11 +127,28 @@ LauncherVoidResultWithLineInfo Initializ
                               sizeof(newFlags), &bytesWritten);
   if (!ok || bytesWritten != sizeof(newFlags)) {
     return LAUNCHER_ERROR_FROM_LAST();
   }
 
   return Ok();
 }
 
+LauncherVoidResultWithLineInfo InitializeDllBlocklistOOP(
+    const wchar_t* aFullImagePath, HANDLE aChildProcess) {
+  // We come here when the browser process launches a sandbox process.
+  // If the launcher process already failed to bootstrap the browser process,
+  // we should not attempt to bootstrap a child process.
+  if (!(gBlocklistInitFlags & eDllBlocklistInitFlagWasBootstrapped)) {
+    return Ok();
+  }
+
+  return InitializeDllBlocklistOOPInternal(aFullImagePath, aChildProcess);
+}
+
+LauncherVoidResultWithLineInfo InitializeDllBlocklistOOPFromLauncher(
+    const wchar_t* aFullImagePath, HANDLE aChildProcess) {
+  return InitializeDllBlocklistOOPInternal(aFullImagePath, aChildProcess);
+}
+
 #endif  // defined(MOZ_ASAN) || defined(_M_ARM64)
 
 }  // namespace mozilla
--- a/browser/app/winlauncher/DllBlocklistInit.h
+++ b/browser/app/winlauncher/DllBlocklistInit.h
@@ -11,11 +11,14 @@
 
 #include "mozilla/WinHeaderOnlyUtils.h"
 
 namespace mozilla {
 
 LauncherVoidResultWithLineInfo InitializeDllBlocklistOOP(
     const wchar_t* aFullImagePath, HANDLE aChildProcess);
 
+LauncherVoidResultWithLineInfo InitializeDllBlocklistOOPFromLauncher(
+    const wchar_t* aFullImagePath, HANDLE aChildProcess);
+
 }  // namespace mozilla
 
 #endif  // mozilla_DllBlocklistInit_h
--- a/browser/app/winlauncher/LauncherProcessWin.cpp
+++ b/browser/app/winlauncher/LauncherProcessWin.cpp
@@ -38,17 +38,18 @@
  * At this point the child process has been created in a suspended state. Any
  * additional startup work (eg, blocklist setup) should go here.
  *
  * @return Ok if browser startup should proceed
  */
 static mozilla::LauncherVoidResult PostCreationSetup(
     const wchar_t* aFullImagePath, HANDLE aChildProcess,
     HANDLE aChildMainThread, const bool aIsSafeMode) {
-  return mozilla::InitializeDllBlocklistOOP(aFullImagePath, aChildProcess);
+  return mozilla::InitializeDllBlocklistOOPFromLauncher(aFullImagePath,
+                                                        aChildProcess);
 }
 
 #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)