syscall: fix accidental close of exec status pipe in StartProcess

In syscall.forkAndExecInChild, blocks of code labelled Pass 1
and Pass 2 permute the file descriptors (if necessary) which are
passed to the child process.  If Pass 1 begins with fds = {0,2,1},
nextfd = 4 and pipe = 4, then the statement labelled "don't stomp
on pipe" is too late -- the pipe (which will be needed to pass
exec status back to the parent) will have been closed by the
preceding DUP call.

Moving the "don't stomp" test earlier ensures that the pipe is
protected.

Fixes #14979

Change-Id: I890c311527f6aa255be48b3277c1e84e2049ee22
Reviewed-on: https://go-review.googlesource.com/21184
Run-TryBot: David du Colombier <0intro@gmail.com>
Reviewed-by: David du Colombier <0intro@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/syscall/exec_bsd.go b/src/syscall/exec_bsd.go
index 47adffd60c8f44b82a26c05d8358d13c7780895a..317645fae545fa0ee5ec7cbb6917b3f348695a2d 100644 (file)
--- a/src/syscall/exec_bsd.go
+++ b/src/syscall/exec_bsd.go
@@ -181,6 +181,9 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
        }
        for i = 0; i < len(fd); i++ {
                if fd[i] >= 0 && fd[i] < int(i) {
+                       if nextfd == pipe { // don't stomp on pipe
+                               nextfd++
+                       }
                        _, _, err1 = RawSyscall(SYS_DUP2, uintptr(fd[i]), uintptr(nextfd), 0)
                        if err1 != 0 {
                                goto childerror
@@ -188,9 +191,6 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
                        RawSyscall(SYS_FCNTL, uintptr(nextfd), F_SETFD, FD_CLOEXEC)
                        fd[i] = nextfd
                        nextfd++
-                       if nextfd == pipe { // don't stomp on pipe
-                               nextfd++
-                       }
                }
        }
 

diff --git a/src/syscall/exec_linux.go b/src/syscall/exec_linux.go
index c1fd53cc6ec8a17a23633fe574f934148b85323e..e49bad75b284ff3bdd08646317868cb508020f5f 100644 (file)
--- a/src/syscall/exec_linux.go
+++ b/src/syscall/exec_linux.go
@@ -255,6 +255,9 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
        }
        for i = 0; i < len(fd); i++ {
                if fd[i] >= 0 && fd[i] < int(i) {
+                       if nextfd == pipe { // don't stomp on pipe
+                               nextfd++
+                       }
                        _, _, err1 = RawSyscall(_SYS_dup, uintptr(fd[i]), uintptr(nextfd), 0)
                        if err1 != 0 {
                                goto childerror
@@ -262,9 +265,6 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
                        RawSyscall(SYS_FCNTL, uintptr(nextfd), F_SETFD, FD_CLOEXEC)
                        fd[i] = nextfd
                        nextfd++
-                       if nextfd == pipe { // don't stomp on pipe
-                               nextfd++
-                       }
                }
        }
 

diff --git a/src/syscall/exec_plan9.go b/src/syscall/exec_plan9.go
index 28a746580b2ecc0ac43a00fcb690794c2278bc89..bccea5105cb7bb2266760a55c3b4fc63ae0fbb40 100644 (file)
--- a/src/syscall/exec_plan9.go
+++ b/src/syscall/exec_plan9.go
@@ -270,6 +270,9 @@ func forkAndExecInChild(argv0 *byte, argv []*byte, envv []envItem, dir *byte, at
        }
        for i = 0; i < len(fd); i++ {
                if fd[i] >= 0 && fd[i] < int(i) {
+                       if nextfd == pipe { // don't stomp on pipe
+                               nextfd++
+                       }
                        r1, _, _ = RawSyscall(SYS_DUP, uintptr(fd[i]), uintptr(nextfd), 0)
                        if int32(r1) == -1 {
                                goto childerror
@@ -277,9 +280,6 @@ func forkAndExecInChild(argv0 *byte, argv []*byte, envv []envItem, dir *byte, at
 
                        fd[i] = nextfd
                        nextfd++
-                       if nextfd == pipe { // don't stomp on pipe
-                               nextfd++
-                       }
                }
        }
 

diff --git a/src/syscall/exec_solaris.go b/src/syscall/exec_solaris.go
index c2b294946209ed2fa811ec6885010c463e74aee3..fcb481c078af50881ff4e85bf11d72c268014a42 100644 (file)
--- a/src/syscall/exec_solaris.go
+++ b/src/syscall/exec_solaris.go
@@ -178,6 +178,9 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
        }
        for i = 0; i < len(fd); i++ {
                if fd[i] >= 0 && fd[i] < int(i) {
+                       if nextfd == pipe { // don't stomp on pipe
+                               nextfd++
+                       }
                        _, err1 = fcntl1(uintptr(fd[i]), F_DUP2FD, uintptr(nextfd))
                        if err1 != 0 {
                                goto childerror
@@ -185,9 +188,6 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
                        fcntl1(uintptr(nextfd), F_SETFD, FD_CLOEXEC)
                        fd[i] = nextfd
                        nextfd++
-                       if nextfd == pipe { // don't stomp on pipe
-                               nextfd++
-                       }
                }
        }
 

diff --git a/src/syscall/syscall_test.go b/src/syscall/syscall_test.go
index 846c4873d28aadda67541932ebebbd93638cdb90..0a0b8b7a26da1be69c3e15a0a3aadbea22f7dd78 100644 (file)
--- a/src/syscall/syscall_test.go
+++ b/src/syscall/syscall_test.go
@@ -6,6 +6,8 @@ package syscall_test
 
 import (
        "fmt"
+       "internal/testenv"
+       "os"
        "syscall"
        "testing"
 )
@@ -45,3 +47,15 @@ func TestItoa(t *testing.T) {
                t.Fatalf("itoa(%d) = %s, want %s", i, s, f)
        }
 }
+
+// Check that permuting child process fds doesn't interfere with
+// reporting of fork/exec status. See Issue 14979.
+func TestExecErrPermutedFds(t *testing.T) {
+       testenv.MustHaveExec(t)
+
+       attr := &os.ProcAttr{Files: []*os.File{os.Stdin, os.Stderr, os.Stdout}}
+       _, err := os.StartProcess("/", []string{"/"}, attr)
+       if err == nil {
+               t.Fatalf("StartProcess of invalid program returned err = nil")
+       }
+}