# HG changeset patch # User Bob Owen # Date 1577387989 0 # Thu Dec 26 19:19:49 2019 +0000 # Node ID 32adf437117bdca54be4959813acbb604f65137f # Parent 214214029beb6cca606e11ba519d11cc7dbb37af Bug 1605867: Don't duplicate IPC shared memory when we might fail to launch the process correctly. r=handyman Differential Revision: https://phabricator.services.mozilla.com/D58271 diff --git a/security/sandbox/chromium/sandbox/win/src/target_process.cc b/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 @@ -286,45 +286,28 @@ ResultCode TargetProcess::Init(Dispatche shared_section_.Set(::CreateFileMappingW(INVALID_HANDLE_VALUE, nullptr, PAGE_READWRITE | SEC_COMMIT, 0, shared_mem_size, nullptr)); 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 (!shared_memory) { *win_error = ::GetLastError(); return SBOX_ERROR_MAP_VIEW_OF_SHARED_SECTION; } CopyPolicyToTarget(policy, shared_policy_size, reinterpret_cast(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 = nullptr; - 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; } @@ -339,16 +322,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 = nullptr; + 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())