diff options
author | Jason A. Donenfeld <Jason@zx2c4.com> | 2021-01-31 17:54:27 +0100 |
---|---|---|
committer | Jason A. Donenfeld <Jason@zx2c4.com> | 2021-02-26 18:23:29 +0000 |
commit | 2d760816ff30bea82f54682f3049cfb6c6027da7 (patch) | |
tree | 606cb6fe0271c72e0235f8102c70697254df32c5 | |
parent | ba9168bd07bfed6071e3eadcaae40b5481c52c24 (diff) | |
download | go-2d760816ff30bea82f54682f3049cfb6c6027da7.tar.gz go-2d760816ff30bea82f54682f3049cfb6c6027da7.zip |
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 <Jason@zx2c4.com>
Trust: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
-rw-r--r-- | src/syscall/exec_windows.go | 25 |
1 files 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 |