From 2d760816ff30bea82f54682f3049cfb6c6027da7 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Sun, 31 Jan 2021 17:54:27 +0100 Subject: syscall: restrict inherited handles on Windows Windows does not have CLOEXEC, but rather handles are marked explicitly for being inherited by new processes. This can cause problems when different Windows functions create new processes from different threads. syscall.StartProcess has traditionally used a mutex to prevent races with itself, but this doesn't handle races with other win32 functions. Fortunately there's a solution: PROC_THREAD_ATTRIBUTE_HANDLE_LIST allows us to pass the entire list of handles that we want to be inherited. This lets us get rid of the mutex and also makes process creation safe across the Go runtime, no matter the context. Updates #44011. Change-Id: Ia3424cd2ec64868849cbd6cbb5b0d765224bf4ab Reviewed-on: https://go-review.googlesource.com/c/go/+/288297 Trust: Jason A. Donenfeld Trust: Alex Brainman Reviewed-by: Alex Brainman --- src/syscall/exec_windows.go | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/syscall/exec_windows.go b/src/syscall/exec_windows.go index 46cbd7567d..ff9f7a3913 100644 --- a/src/syscall/exec_windows.go +++ b/src/syscall/exec_windows.go @@ -310,12 +310,6 @@ func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid int, handle } } - // Acquire the fork lock so that no other threads - // create new fds that are not yet close-on-exec - // before we fork. - ForkLock.Lock() - defer ForkLock.Unlock() - p, _ := GetCurrentProcess() fd := make([]Handle, len(attr.Files)) for i := range attr.Files { @@ -327,7 +321,12 @@ func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid int, handle defer CloseHandle(Handle(fd[i])) } } - si := new(StartupInfo) + si := new(_STARTUPINFOEXW) + si.ProcThreadAttributeList, err = newProcThreadAttributeList(1) + if err != nil { + return 0, 0, err + } + defer deleteProcThreadAttributeList(si.ProcThreadAttributeList) si.Cb = uint32(unsafe.Sizeof(*si)) si.Flags = STARTF_USESTDHANDLES if sys.HideWindow { @@ -338,13 +337,19 @@ func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid int, handle si.StdOutput = fd[1] si.StdErr = fd[2] + // Do not accidentally inherit more than these handles. + err = updateProcThreadAttribute(si.ProcThreadAttributeList, 0, _PROC_THREAD_ATTRIBUTE_HANDLE_LIST, uintptr(unsafe.Pointer(&fd[0])), uintptr(len(fd))*unsafe.Sizeof(fd[0]), 0, nil) + if err != nil { + return 0, 0, err + } + pi := new(ProcessInformation) - flags := sys.CreationFlags | CREATE_UNICODE_ENVIRONMENT + flags := sys.CreationFlags | CREATE_UNICODE_ENVIRONMENT | _EXTENDED_STARTUPINFO_PRESENT if sys.Token != 0 { - err = CreateProcessAsUser(sys.Token, argv0p, argvp, sys.ProcessAttributes, sys.ThreadAttributes, !sys.NoInheritHandles, flags, createEnvBlock(attr.Env), dirp, si, pi) + err = CreateProcessAsUser(sys.Token, argv0p, argvp, sys.ProcessAttributes, sys.ThreadAttributes, !sys.NoInheritHandles, flags, createEnvBlock(attr.Env), dirp, &si.StartupInfo, pi) } else { - err = CreateProcess(argv0p, argvp, sys.ProcessAttributes, sys.ThreadAttributes, !sys.NoInheritHandles, flags, createEnvBlock(attr.Env), dirp, si, pi) + err = CreateProcess(argv0p, argvp, sys.ProcessAttributes, sys.ThreadAttributes, !sys.NoInheritHandles, flags, createEnvBlock(attr.Env), dirp, &si.StartupInfo, pi) } if err != nil { return 0, 0, err -- cgit v1.2.3-54-g00ecf