]> Cypherpunks repositories - gostls13.git/commit
syscall: use Ctty before fd shuffle
authorGreg Thelen <gthelen@google.com>
Sat, 25 May 2019 18:44:44 +0000 (11:44 -0700)
committerIan Lance Taylor <iant@golang.org>
Thu, 30 May 2019 22:02:03 +0000 (22:02 +0000)
commit103b5b66921b351f8db4fc6e83bf147b1a0d7580
tree6ca7f1ac34bf4e4ec4a9ff2aaec174954111bce8
parenta53b46557267492c8f5e6e285c8d4c415183cacf
syscall: use Ctty before fd shuffle

On unix if exec.Command() is given both ExtraFiles and Ctty, and the
Ctty file descriptor overlaps the range of FDs intended for the child,
then cmd.Start() the ioctl(fd,TIOCSCTTY) call fails with an
"inappropriate ioctl for device" error.

When child file descriptors overlap the new child's ctty the ctty will
be closed in the fd shuffle before the TIOCSCTTY.  Thus TIOCSCTTY is
used on one of the ExtraFiles rather than the intended Ctty file.  Thus
the error.

exec.Command() callers can workaround this by ensuring the Ctty fd is
larger than any ExtraFiles destined for the child.

Fix this by doing the ctty ioctl before the fd shuffle.

Test for this issue by modifying TestTerminalSignal to use more
ExtraFiles.  The test fails on linux and freebsd without this change's
syscall/*.go changes.  Other platforms (e.g. darwin, aix, solaris) have
the same fd shuffle logic, so the same fix is applied to them.  However,
I was only able to test on linux (32 and 64 bit) and freebsd (64 bit).

Manual runs of the test in https://golang.org/issue/29458 start passing
with this patch:
  Before:
    % /tmp/src/go/bin/go run t
    successfully ran child process with ParentExtraFileFdNum=5, ChildExtraFileFd=6, ParentPtyFd=7

    panic: failed to run child process with ParentExtraFileFdNum=10, ChildExtraFileFd=11, ParentPtyFd=11: fork/exec /bin/true: inappropriate ioctl for device

  After:
    % /tmp/src/go/bin/go run t
    successfully ran child process with ParentExtraFileFdNum=5, ChildExtraFileFd=6, ParentPtyFd=7

    successfully ran child process with ParentExtraFileFdNum=10, ChildExtraFileFd=11, ParentPtyFd=11

Fixes #29458
Change-Id: I99513de7b6073c7eb855f1eeb4d1f9dc0454ef8b
Reviewed-on: https://go-review.googlesource.com/c/go/+/178919
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
src/os/signal/signal_cgo_test.go
src/syscall/exec_bsd.go
src/syscall/exec_darwin.go
src/syscall/exec_libc.go
src/syscall/exec_linux.go