]> Cypherpunks repositories - gostls13.git/commitdiff
syscall: restrict inherited handles on Windows
authorJason A. Donenfeld <Jason@zx2c4.com>
Sun, 31 Jan 2021 16:54:27 +0000 (17:54 +0100)
committerJason A. Donenfeld <Jason@zx2c4.com>
Fri, 26 Feb 2021 18:23:29 +0000 (18:23 +0000)
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>
src/syscall/exec_windows.go

index 46cbd7567dd5607f3fc853267de7cfcbd71747b0..ff9f7a3913bf61ce0c4b9b72d724c9a27f443f38 100644 (file)
@@ -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