Bug 1605867: Don't duplicate IPC shared memory when we might fail to launch the process correctly. r=handyman, a=jcristau FENNEC_68_4_0_BUILD1 FENNEC_68_4_0_RELEASE FENNEC_68_4b4_BUILD1 FENNEC_68_4b4_RELEASE FIREFOX_68_4_0esr_BUILD1 FIREFOX_68_4_0esr_RELEASE
authorBob Owen <bobowencode@gmail.com>
Mon, 30 Dec 2019 14:09:00 +0100 (2019-12-30)
changeset 524312 b9ced0dd1026bf43339ecd41b942d19f63ea72c6
parent 524311 abb620ef4695c289e6c384d4ddb6400b9613ba4c
child 524313 3fc15374903a6d690f2272cff325043a7d1dcc2e
child 524320 39573a76dadbeafd8e647f6cd58a9c18bdf93e07
push id704
push userjcristau@mozilla.com
push dateMon, 30 Dec 2019 13:14:29 +0000 (2019-12-30)
treeherdermozilla-esr68@b9ced0dd1026 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershandyman, jcristau
bugs1605867
milestone68.4.0
Bug 1605867: Don't duplicate IPC shared memory when we might fail to launch the process correctly. r=handyman, a=jcristau
security/sandbox/chromium/sandbox/win/src/target_process.cc
--- a/security/sandbox/chromium/sandbox/win/src/target_process.cc
+++ b/security/sandbox/chromium/sandbox/win/src/target_process.cc
@@ -225,46 +225,29 @@ ResultCode TargetProcess::Init(Dispatche
   shared_section_.Set(::CreateFileMappingW(INVALID_HANDLE_VALUE, NULL,
                                            PAGE_READWRITE | SEC_COMMIT,
                                            0, shared_mem_size, NULL));
   if (!shared_section_.IsValid()) {
     *win_error = ::GetLastError();
     return SBOX_ERROR_CREATE_FILE_MAPPING;
   }
 
-  DWORD access = FILE_MAP_READ | FILE_MAP_WRITE | SECTION_QUERY;
-  HANDLE target_shared_section;
-  if (!::DuplicateHandle(::GetCurrentProcess(), shared_section_.Get(),
-                         sandbox_process_info_.process_handle(),
-                         &target_shared_section, access, FALSE, 0)) {
-    *win_error = ::GetLastError();
-    return SBOX_ERROR_DUPLICATE_SHARED_SECTION;
-  }
-
   void* shared_memory = ::MapViewOfFile(shared_section_.Get(),
                                         FILE_MAP_WRITE|FILE_MAP_READ,
                                         0, 0, 0);
   if (NULL == shared_memory) {
     *win_error = ::GetLastError();
     return SBOX_ERROR_MAP_VIEW_OF_SHARED_SECTION;
   }
 
   CopyPolicyToTarget(policy, shared_policy_size,
                      reinterpret_cast<char*>(shared_memory) + shared_IPC_size);
 
   ResultCode ret;
   // Set the global variables in the target. These are not used on the broker.
-  g_shared_section = target_shared_section;
-  ret = TransferVariable("g_shared_section", &g_shared_section,
-                         sizeof(g_shared_section));
-  g_shared_section = NULL;
-  if (SBOX_ALL_OK != ret) {
-    *win_error = ::GetLastError();
-    return ret;
-  }
   g_shared_IPC_size = shared_IPC_size;
   ret = TransferVariable("g_shared_IPC_size", &g_shared_IPC_size,
                          sizeof(g_shared_IPC_size));
   g_shared_IPC_size = 0;
   if (SBOX_ALL_OK != ret) {
     *win_error = ::GetLastError();
     return ret;
   }
@@ -280,16 +263,34 @@ ResultCode TargetProcess::Init(Dispatche
   ipc_server_.reset(
       new SharedMemIPCServer(sandbox_process_info_.process_handle(),
                              sandbox_process_info_.process_id(),
                              thread_pool_, ipc_dispatcher));
 
   if (!ipc_server_->Init(shared_memory, shared_IPC_size, kIPCChannelSize))
     return SBOX_ERROR_NO_SPACE;
 
+  DWORD access = FILE_MAP_READ | FILE_MAP_WRITE | SECTION_QUERY;
+  HANDLE target_shared_section;
+  if (!::DuplicateHandle(::GetCurrentProcess(), shared_section_.Get(),
+                         sandbox_process_info_.process_handle(),
+                         &target_shared_section, access, FALSE, 0)) {
+    *win_error = ::GetLastError();
+    return SBOX_ERROR_DUPLICATE_SHARED_SECTION;
+  }
+
+  g_shared_section = target_shared_section;
+  ret = TransferVariable("g_shared_section", &g_shared_section,
+                         sizeof(g_shared_section));
+  g_shared_section = NULL;
+  if (SBOX_ALL_OK != ret) {
+    *win_error = ::GetLastError();
+    return ret;
+  }
+
   // After this point we cannot use this handle anymore.
   ::CloseHandle(sandbox_process_info_.TakeThreadHandle());
 
   return SBOX_ALL_OK;
 }
 
 void TargetProcess::Terminate() {
   if (!sandbox_process_info_.IsValid())