Return the thread handle when launching a process in a different session via the execution server o…

remoting/host/win/launch_process_with_token.cc

Index: remoting/host/win/launch_process_with_token.cc
diff --git a/remoting/host/win/launch_process_with_token.cc b/remoting/host/win/launch_process_with_token.cc
index 1320e2a3dd911fa932d1c3fafc93f62109e045f1..191f8967e4e0e0002afb9c1e11328deee9fddf0c 100644
--- a/remoting/host/win/launch_process_with_token.cc
+++ b/remoting/host/win/launch_process_with_token.cc
@@ -40,6 +40,89 @@ const int kMinLaunchDelaySeconds = 1;
 // Name of the default session desktop.
 const char kDefaultDesktopName[] = "winsta0\\default";
 
+// Terminates the process and closes process and thread handles in
+// |process_information| structure.
+void CloseHandlesAndTerminateProcess(PROCESS_INFORMATION* process_information) {
+  if (process_information->hThread) {
+    CloseHandle(process_information->hThread);
+    process_information->hThread = NULL;
+  }
+
+  if (process_information->hProcess) {
+    TerminateProcess(process_information->hProcess, CONTROL_C_EXIT);
+    CloseHandle(process_information->hProcess);
+    process_information->hProcess = NULL;
+  }
+}
+
+// Connects to the executor server corresponding to |session_id|.
+bool ConnectToExecutionServer(uint32 session_id,
+                              base::win::ScopedHandle* pipe_out) {
+  string16 pipe_name;
+
+  // Use winsta!WinStationQueryInformationW() to determine the process creation
+  // pipe name for the session.
+  FilePath winsta_path(base::GetNativeLibraryName(UTF8ToUTF16("winsta")));
+  base::ScopedNativeLibrary winsta(winsta_path);
+  if (winsta.is_valid()) {
+    PWINSTATIONQUERYINFORMATIONW win_station_query_information =
+        static_cast<PWINSTATIONQUERYINFORMATIONW>(
+            winsta.GetFunctionPointer("WinStationQueryInformationW"));
+    if (win_station_query_information) {
+      wchar_t name[MAX_PATH];
+      ULONG name_length;
+      if (win_station_query_information(0,
+                                        session_id,
+                                        kCreateProcessPipeNameClass,
+                                        name,
+                                        sizeof(name),
+                                        &name_length)) {
+        pipe_name.assign(name);
+      }
+    }
+  }
+
+  // Use the default pipe name if we couldn't query its name.
+  if (pipe_name.empty()) {
+    pipe_name = UTF8ToUTF16(
+        StringPrintf(kCreateProcessDefaultPipeNameFormat, session_id));
+  }
+
+  // Try to connect to the named pipe.
+  base::win::ScopedHandle pipe;
+  for (int i = 0; i < kPipeConnectMaxAttempts; ++i) {
+    pipe.Set(CreateFile(pipe_name.c_str(),
+                        GENERIC_READ | GENERIC_WRITE,
+                        0,
+                        NULL,
+                        OPEN_EXISTING,
+                        0,
+                        NULL));
+    if (pipe.IsValid()) {
+      break;
+    }
+
+    // Cannot continue retrying if error is something other than
+    // ERROR_PIPE_BUSY.
+    if (GetLastError() != ERROR_PIPE_BUSY) {
+      break;
+    }
+
+    // Cannot continue retrying if wait on pipe fails.
+    if (!WaitNamedPipe(pipe_name.c_str(), kPipeBusyWaitTimeoutMs)) {
+      break;
+    }
+  }
+
+  if (!pipe.IsValid()) {
+    LOG_GETLASTERROR(ERROR) << "Failed to connect to '" << pipe_name << "'";
+    return false;
+  }
+
+  *pipe_out = pipe.Pass();
+  return true;
+}
+
 // Copies the process token making it a primary impersonation token.
 // The returned handle will have |desired_access| rights.
 bool CopyProcessToken(DWORD desired_access, ScopedHandle* token_out) {
@@ -96,79 +179,121 @@ bool CreatePrivilegedToken(ScopedHandle* token_out) {
   return true;
 }
 
-// Requests the execution server to create a process in the specified session
-// using the default (i.e. Winlogon) token. This routine relies on undocumented
-// OS functionality and will likely not work on anything but XP or W2K3.
-bool CreateRemoteSessionProcess(
-    uint32 session_id,
-    const FilePath::StringType& application_name,
-    const CommandLine::StringType& command_line,
-    DWORD creation_flags,
-    PROCESS_INFORMATION* process_information_out)
-{
-  DCHECK(base::win::GetVersion() == base::win::VERSION_XP);
-
-  string16 pipe_name;
-
-  // Use winsta!WinStationQueryInformationW() to determine the process creation
-  // pipe name for the session.
-  FilePath winsta_path(base::GetNativeLibraryName(UTF8ToUTF16("winsta")));
-  base::ScopedNativeLibrary winsta(winsta_path);
-  if (winsta.is_valid()) {
-    PWINSTATIONQUERYINFORMATIONW win_station_query_information =
-        static_cast<PWINSTATIONQUERYINFORMATIONW>(
-            winsta.GetFunctionPointer("WinStationQueryInformationW"));
-    if (win_station_query_information) {
-      wchar_t name[MAX_PATH];
-      ULONG name_length;
-      if (win_station_query_information(0,
-                                        session_id,
-                                        kCreateProcessPipeNameClass,
-                                        name,
-                                        sizeof(name),
-                                        &name_length)) {
-        pipe_name.assign(name);
-      }
+// Fills the process and thread handles in the passed |process_information|
+// structure and resume the process if the caller didn't want to suspend it.
+bool ProcessCreateProcessResponse(DWORD creation_flags,
+                                  PROCESS_INFORMATION* process_information) {
+  // The execution server does not return handles to the created process and
+  // thread.
+  if (!process_information->hProcess) {
+    // N.B. PROCESS_ALL_ACCESS is different in XP and Vista+ versions of
+    // the SDK. |desired_access| below is effectively PROCESS_ALL_ACCESS from
+    // the XP version of the SDK.
+    DWORD desired_access =
+        STANDARD_RIGHTS_REQUIRED |
+        SYNCHRONIZE |
+        PROCESS_TERMINATE |
+        PROCESS_CREATE_THREAD |
+        PROCESS_SET_SESSIONID |
+        PROCESS_VM_OPERATION |
+        PROCESS_VM_READ |
+        PROCESS_VM_WRITE |
+        PROCESS_DUP_HANDLE |
+        PROCESS_CREATE_PROCESS |
+        PROCESS_SET_QUOTA |
+        PROCESS_SET_INFORMATION |
+        PROCESS_QUERY_INFORMATION |
+        PROCESS_SUSPEND_RESUME;
+    process_information->hProcess =
+        OpenProcess(desired_access,
+                    FALSE,
+                    process_information->dwProcessId);
+    if (!process_information->hProcess) {
+      LOG_GETLASTERROR(ERROR) << "Failed to open the process "
+                              << process_information->dwProcessId;
+      return false;
     }
   }
 
-  // Use the default pipe name if we couldn't query its name.
-  if (pipe_name.empty()) {
-    pipe_name = UTF8ToUTF16(
-        StringPrintf(kCreateProcessDefaultPipeNameFormat, session_id));
+  if (!process_information->hThread) {
+    // N.B. THREAD_ALL_ACCESS is different in XP and Vista+ versions of
+    // the SDK. |desired_access| below is effectively THREAD_ALL_ACCESS from
+    // the XP version of the SDK.
+    DWORD desired_access =
+        STANDARD_RIGHTS_REQUIRED |
+        SYNCHRONIZE |
+        THREAD_TERMINATE |
+        THREAD_SUSPEND_RESUME |
+        THREAD_GET_CONTEXT |
+        THREAD_SET_CONTEXT |
+        THREAD_QUERY_INFORMATION |
+        THREAD_SET_INFORMATION |
+        THREAD_SET_THREAD_TOKEN |
+        THREAD_IMPERSONATE |
+        THREAD_DIRECT_IMPERSONATION;
+    process_information->hThread =
+        OpenThread(desired_access,
+                   FALSE,
+                   process_information->dwThreadId);
+    if (!process_information->hThread) {
+      LOG_GETLASTERROR(ERROR) << "Failed to open the thread "
+                              << process_information->dwThreadId;
+      return false;
+    }
   }
 
-  // Try to connect to the named pipe.
-  base::win::ScopedHandle pipe;
-  for (int i = 0; i < kPipeConnectMaxAttempts; ++i) {
-    pipe.Set(CreateFile(pipe_name.c_str(),
-                        GENERIC_READ | GENERIC_WRITE,
-                        0,
-                        NULL,
-                        OPEN_EXISTING,
-                        0,
-                        NULL));
-    if (pipe.IsValid()) {
-      break;
+  // Resume the thread if the caller didn't want to suspend the process.
+  if ((creation_flags & CREATE_SUSPENDED) == 0) {
+    if (!ResumeThread(process_information->hThread)) {
+      LOG_GETLASTERROR(ERROR) << "Failed to resume the thread "
+                              << process_information->dwThreadId;
+      return false;
     }
+  }
 
-    // Cannot continue retrying if error is something other than
-    // ERROR_PIPE_BUSY.
-    if (GetLastError() != ERROR_PIPE_BUSY) {
-      break;
-    }
+  return true;
+}
 
-    // Cannot continue retrying if wait on pipe fails.
-    if (!WaitNamedPipe(pipe_name.c_str(), kPipeBusyWaitTimeoutMs)) {
-      break;
-    }
+// Receives the response to a remote process create request.
+bool ReceiveCreateProcessResponse(
+    HANDLE pipe,
+    PROCESS_INFORMATION* process_information_out) {
+  struct CreateProcessResponse {
+    DWORD size;
+    BOOL success;
+    DWORD last_error;
+    PROCESS_INFORMATION process_information;
+  };
+
+  DWORD bytes;
+  CreateProcessResponse response;
+  if (!ReadFile(pipe, &response, sizeof(response), &bytes, NULL)) {
+    LOG_GETLASTERROR(ERROR) << "Failed to receive CreateProcessAsUser response";
+    return false;
   }
 
-  if (!pipe.IsValid()) {
-    LOG_GETLASTERROR(ERROR) << "Failed to connect to '" << pipe_name << "'";
+  // The server sends the data in one chunk so if we didn't received a complete
+  // answer something bad happend and there is no point in retrying.
+  if (bytes != sizeof(response)) {
+    SetLastError(ERROR_RECEIVE_PARTIAL);
     return false;
   }
 
+  if (!response.success) {
+    SetLastError(response.last_error);
+    return false;
+  }
+
+  *process_information_out = response.process_information;
+  return true;
+}
+
+// Sends a remote process create request to the execution server.
+bool SendCreateProcessRequest(
+    HANDLE pipe,
+    const FilePath::StringType& application_name,
+    const CommandLine::StringType& command_line,
+    DWORD creation_flags) {
   string16 desktop_name(UTF8ToUTF16(kDefaultDesktopName));
 
   // |CreateProcessRequest| structure passes the same parameters to
@@ -206,7 +331,9 @@ bool CreateRemoteSessionProcess(
   request->size = size;
   request->process_id = GetCurrentProcessId();
   request->use_default_token = TRUE;
-  request->creation_flags = creation_flags;
+  // Always pass CREATE_SUSPENDED to avoid a race between the created process
+  // exiting too soon and OpenProcess() call below.
+  request->creation_flags = creation_flags | CREATE_SUSPENDED;
   request->startup_info.cb = sizeof(request->startup_info);
 
   size_t buffer_offset = sizeof(CreateProcessRequest);
@@ -236,65 +363,40 @@ bool CreateRemoteSessionProcess(
     return false;
   }
 
-  // Receive the response.
-  struct CreateProcessResponse {
-    DWORD size;
-    BOOL success;
-    DWORD last_error;
-    PROCESS_INFORMATION process_information;
-  };
+  return true;
+}
 
-  CreateProcessResponse response;
-  if (!ReadFile(pipe, &response, sizeof(response), &bytes, NULL)) {
-    LOG_GETLASTERROR(ERROR) << "Failed to receive CreateProcessAsUser response";
+// Requests the execution server to create a process in the specified session
+// using the default (i.e. Winlogon) token. This routine relies on undocumented
+// OS functionality and will likely not work on anything but XP or W2K3.
+bool CreateRemoteSessionProcess(
+    uint32 session_id,
+    const FilePath::StringType& application_name,
+    const CommandLine::StringType& command_line,
+    DWORD creation_flags,
+    PROCESS_INFORMATION* process_information_out)
+{
+  DCHECK(base::win::GetVersion() == base::win::VERSION_XP);
+
+  base::win::ScopedHandle pipe;
+  if (!ConnectToExecutionServer(session_id, &pipe))
     return false;
-  }
 
-  // The server sends the data in one chunk so if we didn't received a complete
-  // answer something bad happend and there is no point in retrying.
-  if (bytes != sizeof(response)) {
-    SetLastError(ERROR_RECEIVE_PARTIAL);
+  if (!SendCreateProcessRequest(pipe, application_name, command_line,
+                                creation_flags)) {
     return false;
   }
 
-  if (!response.success) {
-    SetLastError(response.last_error);
+  PROCESS_INFORMATION process_information;
+  if (!ReceiveCreateProcessResponse(pipe, &process_information))
     return false;
-  }
 
-  // The execution server does not return handles to the created process and
-  // thread.
-  if (response.process_information.hProcess == NULL) {
-    // N.B. PROCESS_ALL_ACCESS is different in XP and Vista+ versions of
-    // the SDK. |desired_access| below is effectively PROCESS_ALL_ACCESS from
-    // the XP version of the SDK.
-    DWORD desired_access =
-        STANDARD_RIGHTS_REQUIRED |
-        SYNCHRONIZE |
-        PROCESS_TERMINATE |
-        PROCESS_CREATE_THREAD |
-        PROCESS_SET_SESSIONID |
-        PROCESS_VM_OPERATION |
-        PROCESS_VM_READ |
-        PROCESS_VM_WRITE |
-        PROCESS_DUP_HANDLE |
-        PROCESS_CREATE_PROCESS |
-        PROCESS_SET_QUOTA |
-        PROCESS_SET_INFORMATION |
-        PROCESS_QUERY_INFORMATION |
-        PROCESS_SUSPEND_RESUME;
-    response.process_information.hProcess =
-        OpenProcess(desired_access,
-                    FALSE,
-                    response.process_information.dwProcessId);
-    if (!response.process_information.hProcess) {
-      LOG_GETLASTERROR(ERROR) << "Failed to open process "
-                              << response.process_information.dwProcessId;
-      return false;
-    }
+  if (!ProcessCreateProcessResponse(creation_flags, &process_information)) {
+    CloseHandlesAndTerminateProcess(&process_information);
+    return false;
   }
 
-  *process_information_out = response.process_information;
+  *process_information_out = process_information;
   return true;
 }