Commit 1f0bebcc authored by Richard Miller's avatar Richard Miller Committed by Brad Fitzpatrick

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: default avatarDavid du Colombier <0intro@gmail.com>
Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
parent 272df158
......@@ -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++
}
}
}
......
......@@ -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++
}
}
}
......
......@@ -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++
}
}
}
......
......@@ -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++
}
}
}
......
......@@ -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")
}
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment