Skip to content

Commit

Permalink
Inherit only specific handles on Windows
Browse files Browse the repository at this point in the history
To be able to communicate with a child process via stdin, stdout and
stderr, the CreateProcess() parameter bInheritHandles must be set to
TRUE. But this causes *all* handles to be inherited, including sockets.

As a result, the server socket is inherited by the process running adb
to execute the server on the device, so it may not be closed properly,
causing other scrcpy instances to fail.

To fix the issue, use an extended API to explicitly set the HANDLE to
inherit:
 - <https://stackoverflow.com/a/28185363/1987178>
 - <https://devblogs.microsoft.com/oldnewthing/20111216-00/?p=8873>

Fixes #2779 <#2779>
  • Loading branch information
rom1v committed Nov 14, 2021
1 parent 6a27062 commit 3f52af2
Showing 1 changed file with 84 additions and 12 deletions.
96 changes: 84 additions & 12 deletions app/src/sys/win/process.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
// <https://devblogs.microsoft.com/oldnewthing/20111216-00/?p=8873>
#define _WIN32_WINNT 0x0600 // For extended process API

#include "util/process.h"

#include <processthreadsapi.h>

#include <assert.h>

#include "util/log.h"
Expand All @@ -26,6 +31,17 @@ sc_process_execute_p(const char *const argv[], HANDLE *handle,
HANDLE *pin, HANDLE *pout, HANDLE *perr) {
enum sc_process_result ret = SC_PROCESS_ERROR_GENERIC;

unsigned handle_count = 0;
if (pin) {
++handle_count;
}
if (pout) {
++handle_count;
}
if (perr) {
++handle_count;
}

SECURITY_ATTRIBUTES sa;
sa.nLength = sizeof(SECURITY_ATTRIBUTES);
sa.lpSecurityDescriptor = NULL;
Expand Down Expand Up @@ -65,45 +81,96 @@ sc_process_execute_p(const char *const argv[], HANDLE *handle,
}
}

STARTUPINFOW si;
STARTUPINFOEXW si;
PROCESS_INFORMATION pi;
memset(&si, 0, sizeof(si));
si.cb = sizeof(si);
if (pin || pout || perr) {
si.dwFlags = STARTF_USESTDHANDLES;
si.StartupInfo.cb = sizeof(si);

LPPROC_THREAD_ATTRIBUTE_LIST lpAttributeList = NULL;
if (handle_count) {
si.StartupInfo.dwFlags = STARTF_USESTDHANDLES;
if (pin) {
si.StartupInfo.hStdInput = stdin_read_handle;
}
if (pout) {
si.StartupInfo.hStdOutput = stdout_write_handle;
}
if (perr) {
si.StartupInfo.hStdError = stderr_write_handle;
}

SIZE_T size = 0;
// Call it once to know the required buffer size
BOOL ok =
InitializeProcThreadAttributeList(NULL, handle_count, 0, &size)
|| GetLastError() == ERROR_INSUFFICIENT_BUFFER;
if (!ok) {
goto error_close_stderr;
}

lpAttributeList = malloc(size);
if (!lpAttributeList) {
goto error_close_stderr;
}

ok = InitializeProcThreadAttributeList(lpAttributeList, handle_count, 0,
&size);
if (!ok) {
free(lpAttributeList);
goto error_close_stderr;
}

// Pass explicitly the HANDLEs that must be inherited
HANDLE handles[3];
unsigned i = 0;
if (pin) {
si.hStdInput = stdin_read_handle;
handles[i++] = stdin_read_handle;
}
if (pout) {
si.hStdOutput = stdout_write_handle;
handles[i++] = stdout_write_handle;
}
if (perr) {
si.hStdError = stderr_write_handle;
handles[i++] = stderr_write_handle;
}
ok = UpdateProcThreadAttribute(lpAttributeList, 0,
PROC_THREAD_ATTRIBUTE_HANDLE_LIST,
handles, handle_count * sizeof(HANDLE),
NULL, NULL);
if (!ok) {
goto error_free_attribute_list;
}
}

char *cmd = malloc(CMD_MAX_LEN);
if (!cmd || !build_cmd(cmd, CMD_MAX_LEN, argv)) {
*handle = NULL;
goto error_close_stderr;
goto error_free_attribute_list;
}

wchar_t *wide = sc_str_to_wchars(cmd);
free(cmd);
if (!wide) {
LOGC("Could not allocate wide char string");
goto error_close_stderr;
goto error_free_attribute_list;
}

if (!CreateProcessW(NULL, wide, NULL, NULL, TRUE, 0, NULL, NULL, &si,
&pi)) {
si.lpAttributeList = lpAttributeList;
BOOL bInheritHandles = handle_count > 0;
DWORD dwCreationFlags = handle_count > 0 ? EXTENDED_STARTUPINFO_PRESENT : 0;
if (!CreateProcessW(NULL, wide, NULL, NULL, bInheritHandles,
dwCreationFlags, NULL, NULL, &si.StartupInfo, &pi)) {
free(wide);
*handle = NULL;

if (GetLastError() == ERROR_FILE_NOT_FOUND) {
ret = SC_PROCESS_ERROR_MISSING_BINARY;
}
goto error_close_stderr;
goto error_free_attribute_list;
}

if (lpAttributeList) {
DeleteProcThreadAttributeList(lpAttributeList);
free(lpAttributeList);
}

// These handles are used by the child process, close them for this process
Expand All @@ -122,6 +189,11 @@ sc_process_execute_p(const char *const argv[], HANDLE *handle,

return SC_PROCESS_SUCCESS;

error_free_attribute_list:
if (lpAttributeList) {
DeleteProcThreadAttributeList(lpAttributeList);
free(lpAttributeList);
}
error_close_stderr:
if (perr) {
CloseHandle(*perr);
Expand Down

0 comments on commit 3f52af2

Please sign in to comment.