From bf94ffe21a6ff64e5ba2f2889fafd5dabd068f43 Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Mon, 18 May 2026 17:30:05 -0400 Subject: [PATCH 1/5] add handle inheritance allow list --- contrib/win32/win32compat/w32fd.c | 106 ++++++++++++++++++++++++------ 1 file changed, 85 insertions(+), 21 deletions(-) diff --git a/contrib/win32/win32compat/w32fd.c b/contrib/win32/win32compat/w32fd.c index b4c436864dc4..25a86c13f95b 100644 --- a/contrib/win32/win32compat/w32fd.c +++ b/contrib/win32/win32compat/w32fd.c @@ -37,6 +37,7 @@ #include "inc\sys\stat.h" #include "inc\unistd.h" #include "inc\fcntl.h" +#include "inc\spawn.h" #include "inc\sys\un.h" #include "inc\utf.h" #include "inc\stdio.h" @@ -1094,18 +1095,55 @@ get_username_from_token(HANDLE as_user) return user_name; } +/* + * Build a PROC_THREAD_ATTRIBUTE_LIST that restricts handle inheritance to the + * supplied set. posix_spawn_internal's dup_handle() guarantees every handle + * is non-NULL, distinct, and already marked HANDLE_FLAG_INHERIT. + * + * On success returns a heap-allocated attribute list; caller must release it + * with DeleteProcThreadAttributeList() followed by free(). Returns NULL on + * any Win32 failure; caller should fall back to default inherit-all behavior. + */ +static LPPROC_THREAD_ATTRIBUTE_LIST build_inherit_handle_attr_list(HANDLE *handles, DWORD count) +{ + LPPROC_THREAD_ATTRIBUTE_LIST attr_list = NULL; + SIZE_T attr_list_size = 0; + + if (count == 0) + return NULL; + + InitializeProcThreadAttributeList(NULL, 1, 0, &attr_list_size); + attr_list = (LPPROC_THREAD_ATTRIBUTE_LIST)malloc(attr_list_size); + if (!attr_list) + return NULL; + if (!InitializeProcThreadAttributeList(attr_list, 1, 0, &attr_list_size)) + { + free(attr_list); + return NULL; + } + if (!UpdateProcThreadAttribute(attr_list, 0, PROC_THREAD_ATTRIBUTE_HANDLE_LIST, handles, count * sizeof(HANDLE), NULL, NULL)) + { + DeleteProcThreadAttributeList(attr_list); + free(attr_list); + return NULL; + } + return attr_list; +} + /* * spawn a child process -* - specified by cmd with agruments argv -* - with std handles set to in, out, err -* - flags are passed to CreateProcess call +* - specified by cmd with arguments argv +* - si_ex provides StartupInfo (including stdio handles) and optional +* PROC_THREAD_ATTRIBUTE_LIST; caller owns its storage and any +* buffers it references (e.g. handle-inherit list) +* - flags are passed to CreateProcess call (caller should include +* EXTENDED_STARTUPINFO_PRESENT when si_ex->lpAttributeList is set) * spawned child will run as as_user if its not NULL */ static int -spawn_child_internal(const char* cmd, char *const argv[], HANDLE in, HANDLE out, HANDLE err, unsigned long flags, HANDLE as_user, BOOLEAN prepend_module_path) +spawn_child_internal(const char* cmd, char *const argv[], STARTUPINFOEXW *si_ex, unsigned long flags, HANDLE as_user, BOOLEAN prepend_module_path) { PROCESS_INFORMATION pi; - STARTUPINFOW si; BOOL b; char *cmdline; wchar_t * cmdline_utf16 = NULL; @@ -1118,14 +1156,6 @@ spawn_child_internal(const char* cmd, char *const argv[], HANDLE in, HANDLE out, errno = ENOMEM; goto cleanup; } - - memset(&si, 0, sizeof(STARTUPINFOW)); - si.cb = sizeof(STARTUPINFOW); - si.hStdInput = in; - si.hStdOutput = out; - si.hStdError = err; - si.dwFlags = STARTF_USESTDHANDLES; - if (strstr(cmd, "sshd-session.exe") || strstr(cmd, "sshd-auth.exe")) { flags |= DETACHED_PROCESS; } @@ -1154,16 +1184,16 @@ spawn_child_internal(const char* cmd, char *const argv[], HANDLE in, HANDLE out, free(as_user_name); } if (lpEnvironment) { /* Pass the user environment block to the new process. */ - b = CreateProcessAsUserW(as_user, NULL, t, NULL, NULL, TRUE, flags | CREATE_UNICODE_ENVIRONMENT, lpEnvironment, NULL, &si, &pi); + b = CreateProcessAsUserW(as_user, NULL, t, NULL, NULL, TRUE, flags | CREATE_UNICODE_ENVIRONMENT, lpEnvironment, NULL, &si_ex->StartupInfo, &pi); DestroyEnvironmentBlock(lpEnvironment); } else { /* Pass the current context's environment block to the new process. */ - b = CreateProcessAsUserW(as_user, NULL, t, NULL, NULL, TRUE, flags, NULL, NULL, &si, &pi); + b = CreateProcessAsUserW(as_user, NULL, t, NULL, NULL, TRUE, flags, NULL, NULL, &si_ex->StartupInfo, &pi); } } else { debug3("spawning %ls as subprocess", t); - b = CreateProcessW(NULL, t, NULL, NULL, TRUE, flags, NULL, NULL, &si, &pi); + b = CreateProcessW(NULL, t, NULL, NULL, TRUE, flags, NULL, NULL, &si_ex->StartupInfo, &pi); } if(b || GetLastError() != ERROR_FILE_NOT_FOUND || (argv != NULL && *argv != NULL) || cmd[0] == '\"') break; @@ -1189,13 +1219,10 @@ spawn_child_internal(const char* cmd, char *const argv[], HANDLE in, HANDLE out, if (cmdline) free(cmdline); if (cmdline_utf16) - free(cmdline_utf16); - + free(cmdline_utf16); return ret; } -#include "inc\spawn.h" - /* structures defining binary layout of fd info to be transmitted between parent and child processes*/ struct std_fd_state { int num_inherited; @@ -1318,6 +1345,10 @@ posix_spawn_internal(pid_t *pidp, const char *path, const posix_spawn_file_actio char* fd_info = NULL; HANDLE aux_handles[MAX_INHERITED_FDS]; HANDLE stdio_handles[STDERR_FILENO + 1]; + STARTUPINFOEXW si_ex; + LPPROC_THREAD_ATTRIBUTE_LIST attr_list = NULL; + HANDLE inherit_list[3 + MAX_INHERITED_FDS]; + DWORD inherit_count = 3; if (file_actions == NULL || envp) { errno = ENOTSUP; return -1; @@ -1347,13 +1378,46 @@ posix_spawn_internal(pid_t *pidp, const char *path, const posix_spawn_file_actio if (_putenv_s(POSIX_FD_STATE, fd_info) != 0) goto cleanup; - i = spawn_child_internal(path, argv + 1, stdio_handles[STDIN_FILENO], stdio_handles[STDOUT_FILENO], stdio_handles[STDERR_FILENO], sc_flags, user_token, prepend_module_path); + + /* assemble extended startup info: stdio handles + handle-inheritance allow-list */ + memset(&si_ex, 0, sizeof(si_ex)); + si_ex.StartupInfo.cb = sizeof(STARTUPINFOW); + si_ex.StartupInfo.hStdInput = stdio_handles[STDIN_FILENO]; + si_ex.StartupInfo.hStdOutput = stdio_handles[STDOUT_FILENO]; + si_ex.StartupInfo.hStdError = stdio_handles[STDERR_FILENO]; + si_ex.StartupInfo.dwFlags = STARTF_USESTDHANDLES; + + /* + * Build an allow-list of handles for PROC_THREAD_ATTRIBUTE_HANDLE_LIST. + * With bInheritHandles=TRUE the kernel would otherwise duplicate every handle in + * this process marked HANDLE_FLAG_INHERIT into the child. + */ + inherit_list[0] = stdio_handles[STDIN_FILENO]; + inherit_list[1] = stdio_handles[STDOUT_FILENO]; + inherit_list[2] = stdio_handles[STDERR_FILENO]; + for (i = 0; i < file_actions->num_aux_fds && inherit_count < _countof(inherit_list); i++) + inherit_list[inherit_count++] = aux_handles[i]; + + attr_list = build_inherit_handle_attr_list(inherit_list, inherit_count); + if (attr_list) { + si_ex.lpAttributeList = attr_list; + sc_flags |= EXTENDED_STARTUPINFO_PRESENT; + } else { + /* Fall back to the default inherit-all behavior rather than fail the spawn. */ + debug3("PROC_THREAD_ATTRIBUTE_HANDLE_LIST setup failed; using default handle inheritance"); + } + + i = spawn_child_internal(path, argv + 1, &si_ex, sc_flags, user_token, prepend_module_path); if (i == -1) goto cleanup; if (pidp) *pidp = i; ret = 0; cleanup: + if (attr_list) { + DeleteProcThreadAttributeList(attr_list); + free(attr_list); + } _putenv_s(POSIX_FD_STATE, ""); for (i = 0; i <= STDERR_FILENO; i++) { if (stdio_handles[i] != NULL) { From 4920ae92bd38523c1b8893beb33037e77bc1ce11 Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Tue, 19 May 2026 12:08:02 -0400 Subject: [PATCH 2/5] tweak comments and error handling --- contrib/win32/win32compat/w32fd.c | 41 ++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/contrib/win32/win32compat/w32fd.c b/contrib/win32/win32compat/w32fd.c index 25a86c13f95b..9215493bdbab 100644 --- a/contrib/win32/win32compat/w32fd.c +++ b/contrib/win32/win32compat/w32fd.c @@ -1097,14 +1097,15 @@ get_username_from_token(HANDLE as_user) /* * Build a PROC_THREAD_ATTRIBUTE_LIST that restricts handle inheritance to the - * supplied set. posix_spawn_internal's dup_handle() guarantees every handle - * is non-NULL, distinct, and already marked HANDLE_FLAG_INHERIT. + * supplied set. Caller is expected to pass inheritable handles (entries that + * are not inheritable will cause CreateProcess to fail with ERROR_INVALID_PARAMETER); + * duplicate entries are tolerated by the kernel. * * On success returns a heap-allocated attribute list; caller must release it * with DeleteProcThreadAttributeList() followed by free(). Returns NULL on * any Win32 failure; caller should fall back to default inherit-all behavior. */ -static LPPROC_THREAD_ATTRIBUTE_LIST build_inherit_handle_attr_list(HANDLE *handles, DWORD count) +static LPPROC_THREAD_ATTRIBUTE_LIST build_inherit_handle_attr_list(HANDLE *handles, SIZE_T count) { LPPROC_THREAD_ATTRIBUTE_LIST attr_list = NULL; SIZE_T attr_list_size = 0; @@ -1116,13 +1117,13 @@ static LPPROC_THREAD_ATTRIBUTE_LIST build_inherit_handle_attr_list(HANDLE *handl attr_list = (LPPROC_THREAD_ATTRIBUTE_LIST)malloc(attr_list_size); if (!attr_list) return NULL; - if (!InitializeProcThreadAttributeList(attr_list, 1, 0, &attr_list_size)) - { + if (!InitializeProcThreadAttributeList(attr_list, 1, 0, &attr_list_size)) { + debug3("InitializeProcThreadAttributeList failed, error:%d", GetLastError()); free(attr_list); return NULL; } - if (!UpdateProcThreadAttribute(attr_list, 0, PROC_THREAD_ATTRIBUTE_HANDLE_LIST, handles, count * sizeof(HANDLE), NULL, NULL)) - { + if (!UpdateProcThreadAttribute(attr_list, 0, PROC_THREAD_ATTRIBUTE_HANDLE_LIST, handles, count * sizeof(HANDLE), NULL, NULL)) { + debug3("UpdateProcThreadAttribute failed, error:%d", GetLastError()); DeleteProcThreadAttributeList(attr_list); free(attr_list); return NULL; @@ -1337,6 +1338,13 @@ fd_decode_state(char* enc_buf) return; } +/* + * Duplicate stdio + aux fds for the child, build a STARTUPINFOEXW carrying a + * PROC_THREAD_ATTRIBUTE_HANDLE_LIST allow-list of just those handles, then + * hand off to spawn_child_internal(). Owns the lifetime of attr_list and the + * duplicated handles: both are released on the cleanup path after CreateProcess + * has either consumed or failed to consume them. + */ int posix_spawn_internal(pid_t *pidp, const char *path, const posix_spawn_file_actions_t *file_actions, const posix_spawnattr_t *attrp, char *const argv[], char *const envp[], HANDLE user_token, BOOLEAN prepend_module_path) { @@ -1347,8 +1355,8 @@ posix_spawn_internal(pid_t *pidp, const char *path, const posix_spawn_file_actio HANDLE stdio_handles[STDERR_FILENO + 1]; STARTUPINFOEXW si_ex; LPPROC_THREAD_ATTRIBUTE_LIST attr_list = NULL; - HANDLE inherit_list[3 + MAX_INHERITED_FDS]; - DWORD inherit_count = 3; + HANDLE inherit_list[(STDERR_FILENO + 1) + MAX_INHERITED_FDS]; + size_t inherit_count = STDERR_FILENO + 1; if (file_actions == NULL || envp) { errno = ENOTSUP; return -1; @@ -1381,7 +1389,12 @@ posix_spawn_internal(pid_t *pidp, const char *path, const posix_spawn_file_actio /* assemble extended startup info: stdio handles + handle-inheritance allow-list */ memset(&si_ex, 0, sizeof(si_ex)); - si_ex.StartupInfo.cb = sizeof(STARTUPINFOW); + /* + * Always size cb as STARTUPINFOEXW. CreateProcess ignores the bytes past + * STARTUPINFOW when EXTENDED_STARTUPINFO_PRESENT isn't set, so this is + * safe on the attr_list-build-failed fallback path below. + */ + si_ex.StartupInfo.cb = sizeof(STARTUPINFOEXW); si_ex.StartupInfo.hStdInput = stdio_handles[STDIN_FILENO]; si_ex.StartupInfo.hStdOutput = stdio_handles[STDOUT_FILENO]; si_ex.StartupInfo.hStdError = stdio_handles[STDERR_FILENO]; @@ -1392,10 +1405,10 @@ posix_spawn_internal(pid_t *pidp, const char *path, const posix_spawn_file_actio * With bInheritHandles=TRUE the kernel would otherwise duplicate every handle in * this process marked HANDLE_FLAG_INHERIT into the child. */ - inherit_list[0] = stdio_handles[STDIN_FILENO]; - inherit_list[1] = stdio_handles[STDOUT_FILENO]; - inherit_list[2] = stdio_handles[STDERR_FILENO]; - for (i = 0; i < file_actions->num_aux_fds && inherit_count < _countof(inherit_list); i++) + inherit_list[STDIN_FILENO] = stdio_handles[STDIN_FILENO]; + inherit_list[STDOUT_FILENO] = stdio_handles[STDOUT_FILENO]; + inherit_list[STDERR_FILENO] = stdio_handles[STDERR_FILENO]; + for (i = 0; i < file_actions->num_aux_fds; i++) inherit_list[inherit_count++] = aux_handles[i]; attr_list = build_inherit_handle_attr_list(inherit_list, inherit_count); From 7ade1728a7ca09c48f758adf21060950f42a6d5b Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Tue, 19 May 2026 16:01:50 -0400 Subject: [PATCH 3/5] Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- contrib/win32/win32compat/w32fd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/win32/win32compat/w32fd.c b/contrib/win32/win32compat/w32fd.c index 9215493bdbab..834feb74cba2 100644 --- a/contrib/win32/win32compat/w32fd.c +++ b/contrib/win32/win32compat/w32fd.c @@ -1220,7 +1220,7 @@ spawn_child_internal(const char* cmd, char *const argv[], STARTUPINFOEXW *si_ex, if (cmdline) free(cmdline); if (cmdline_utf16) - free(cmdline_utf16); + free(cmdline_utf16); return ret; } From a4d68c676ed70fe60819d2b38485f2eaf501a0b3 Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Tue, 19 May 2026 16:13:14 -0400 Subject: [PATCH 4/5] fix error message formatting --- contrib/win32/win32compat/w32fd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/win32/win32compat/w32fd.c b/contrib/win32/win32compat/w32fd.c index 834feb74cba2..3798f3082dc2 100644 --- a/contrib/win32/win32compat/w32fd.c +++ b/contrib/win32/win32compat/w32fd.c @@ -1118,12 +1118,12 @@ static LPPROC_THREAD_ATTRIBUTE_LIST build_inherit_handle_attr_list(HANDLE *handl if (!attr_list) return NULL; if (!InitializeProcThreadAttributeList(attr_list, 1, 0, &attr_list_size)) { - debug3("InitializeProcThreadAttributeList failed, error:%d", GetLastError()); + debug3("InitializeProcThreadAttributeList failed, error:%lu", GetLastError()); free(attr_list); return NULL; } if (!UpdateProcThreadAttribute(attr_list, 0, PROC_THREAD_ATTRIBUTE_HANDLE_LIST, handles, count * sizeof(HANDLE), NULL, NULL)) { - debug3("UpdateProcThreadAttribute failed, error:%d", GetLastError()); + debug3("UpdateProcThreadAttribute failed, error:%lu", GetLastError()); DeleteProcThreadAttributeList(attr_list); free(attr_list); return NULL; From 789958324b6b9ffcd4935479305ff8856df71117 Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Tue, 19 May 2026 16:28:14 -0400 Subject: [PATCH 5/5] modify error handling Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- contrib/win32/win32compat/w32fd.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/contrib/win32/win32compat/w32fd.c b/contrib/win32/win32compat/w32fd.c index 3798f3082dc2..ed468be54f7a 100644 --- a/contrib/win32/win32compat/w32fd.c +++ b/contrib/win32/win32compat/w32fd.c @@ -1109,11 +1109,20 @@ static LPPROC_THREAD_ATTRIBUTE_LIST build_inherit_handle_attr_list(HANDLE *handl { LPPROC_THREAD_ATTRIBUTE_LIST attr_list = NULL; SIZE_T attr_list_size = 0; + BOOL success; + DWORD error; if (count == 0) return NULL; - InitializeProcThreadAttributeList(NULL, 1, 0, &attr_list_size); + success = InitializeProcThreadAttributeList(NULL, 1, 0, &attr_list_size); + error = GetLastError(); + if (success || error != ERROR_INSUFFICIENT_BUFFER || attr_list_size == 0) { + debug3("InitializeProcThreadAttributeList size query failed, success:%d error:%lu size:%zu", + success, error, attr_list_size); + return NULL; + } + attr_list = (LPPROC_THREAD_ATTRIBUTE_LIST)malloc(attr_list_size); if (!attr_list) return NULL;