diff --git a/remoting/host/win/unprivileged_process_delegate.cc b/remoting/host/win/unprivileged_process_delegate.cc index b82eceb72ae27..1d21a2f68e52d 100644 --- a/remoting/host/win/unprivileged_process_delegate.cc +++ b/remoting/host/win/unprivileged_process_delegate.cc @@ -401,6 +401,13 @@ void UnprivilegedProcessDelegate::KillProcess() { CloseChannel(); event_handler_ = nullptr; + // Stop the ObjectWatcher before closing the handle it is waiting on. + // Per base/win/object_watcher.h, closing a handle while a wait is pending is + // undefined behavior (MSDN: RegisterWaitForSingleObject). This also prevents + // a second StartWatchingOnce() on the same watcher when the launcher + // relaunches the worker through this same delegate instance. + process_watcher_.StopWatching(); + if (worker_process_.is_valid()) { TerminateProcess(worker_process_.Get(), CONTROL_C_EXIT); worker_process_.Close(); @@ -454,9 +461,12 @@ void UnprivilegedProcessDelegate::ReportProcessLaunched( DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); // Report a handle that can be used to wait for the worker process completion, - // query information about the process and duplicate handles. - DWORD desired_access = - SYNCHRONIZE | PROCESS_DUP_HANDLE | PROCESS_QUERY_INFORMATION; + // query information about the process, duplicate handles, and terminate it. + // PROCESS_TERMINATE is required so that KillProcess() can forcibly reap the + // worker after the kill timer fires; without it TerminateProcess() fails + // with ERROR_ACCESS_DENIED and a misbehaving low-IL worker survives. + DWORD desired_access = SYNCHRONIZE | PROCESS_DUP_HANDLE | + PROCESS_QUERY_INFORMATION | PROCESS_TERMINATE; HANDLE temp_handle; if (!DuplicateHandle(GetCurrentProcess(), worker_process.Get(), GetCurrentProcess(), &temp_handle, desired_access, FALSE,