From b79cb04886d833d2f256eadffbc2317a42dfe3c2 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 30 Jul 2020 20:20:54 -0700 Subject: [PATCH 1/2] runc run/exec: fix terminal wrt stdin redirection This fixes the following failure: > sudo runc run -b bundle ctr WARN[0000] exit status 2 > ERRO[0000] container_linux.go:367: starting container process caused: process_linux.go:459: container init caused: The "exit status 2" with no error message is caused by SIGHUP which is sent to init by the kernel when we are losing the controlling terminal. If we choose to ignore that, we'll get panic in console.Current(), which is addressed by [1]. Otherwise, the issue here is simple: the code assumes stdin is opened to a terminal, and fails to work otherwise. Some standard Linux tools (e.g. stty, top) do the same (modulo panic), while some others (reset, tput) use the trick of trying all the three std streams (starting with stderr as it is least likely to be redirected), and if all three fails, open /dev/tty. This commit does a similar thing (see initHostConsole). It also replaces the call to console.Current(), which may panic (see [1]), by reusing the t.hostConsole. Finally, a simple test case is added. Fixes: https://github.com/opencontainers/runc/issues/2485 [1] https://github.com/containerd/console/pull/37 Signed-off-by: Kir Kolyshkin --- tty.go | 65 +++++++++++++++++++++++++++++++++++--------------- utils_linux.go | 3 +++ 2 files changed, 49 insertions(+), 19 deletions(-) diff --git a/tty.go b/tty.go index 6106c2db368..76a2054679d 100644 --- a/tty.go +++ b/tty.go @@ -12,16 +12,17 @@ import ( "github.com/containerd/console" "github.com/opencontainers/runc/libcontainer" "github.com/opencontainers/runc/libcontainer/utils" + "github.com/pkg/errors" ) type tty struct { - epoller *console.Epoller - console *console.EpollConsole - stdin console.Console - closers []io.Closer - postStart []io.Closer - wg sync.WaitGroup - consoleC chan error + epoller *console.Epoller + console *console.EpollConsole + hostConsole console.Console + closers []io.Closer + postStart []io.Closer + wg sync.WaitGroup + consoleC chan error } func (t *tty) copyIO(w io.Writer, r io.ReadCloser) { @@ -71,6 +72,37 @@ func inheritStdio(process *libcontainer.Process) error { return nil } +func (t *tty) initHostConsole() error { + // Usually all three (stdin, stdout, and stderr) streams are open to + // the terminal, but they might be redirected, so try them all. + for _, s := range []*os.File{os.Stderr, os.Stdout, os.Stdin} { + c, err := console.ConsoleFromFile(s) + switch err { + case nil: + t.hostConsole = c + return nil + case console.ErrNotAConsole: + continue + default: + // should not happen + return errors.Wrap(err, "unable to get console") + } + } + // If all streams are redirected, but we still have a controlling + // terminal, it can be obtained by opening /dev/tty. + tty, err := os.Open("/dev/tty") + if err != nil { + return err + } + c, err := console.ConsoleFromFile(tty) + if err != nil { + return errors.Wrap(err, "unable to get console") + } + + t.hostConsole = c + return nil +} + func (t *tty) recvtty(process *libcontainer.Process, socket *os.File) (Err error) { f, err := utils.RecvFd(socket) if err != nil { @@ -99,18 +131,13 @@ func (t *tty) recvtty(process *libcontainer.Process, socket *os.File) (Err error t.wg.Add(1) go t.copyIO(os.Stdout, epollConsole) - // set raw mode to stdin and also handle interrupt - stdin, err := console.ConsoleFromFile(os.Stdin) - if err != nil { - return err - } - if err := stdin.SetRaw(); err != nil { + // Set raw mode for the controlling terminal. + if err := t.hostConsole.SetRaw(); err != nil { return fmt.Errorf("failed to set the terminal from the stdin: %v", err) } - go handleInterrupt(stdin) + go handleInterrupt(t.hostConsole) t.epoller = epoller - t.stdin = stdin t.console = epollConsole t.closers = []io.Closer{epollConsole} return nil @@ -156,15 +183,15 @@ func (t *tty) Close() error { for _, c := range t.closers { c.Close() } - if t.stdin != nil { - t.stdin.Reset() + if t.hostConsole != nil { + t.hostConsole.Reset() } return nil } func (t *tty) resize() error { - if t.console == nil { + if t.console == nil || t.hostConsole == nil { return nil } - return t.console.ResizeFrom(console.Current()) + return t.console.ResizeFrom(t.hostConsole) } diff --git a/utils_linux.go b/utils_linux.go index 07c8f6ad73e..c24ee8d6d61 100644 --- a/utils_linux.go +++ b/utils_linux.go @@ -157,6 +157,9 @@ func setupIO(process *libcontainer.Process, rootuid, rootgid int, createTTY, det process.Stderr = nil t := &tty{} if !detach { + if err := t.initHostConsole(); err != nil { + return nil, err + } parent, child, err := utils.NewSockPair("console") if err != nil { return nil, err From 42d9a6b43bd9335fa30a52b2e0ed8912e14f6ac9 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 30 Jul 2020 20:45:14 -0700 Subject: [PATCH 2/2] tty.bats: add test cases when stdin is not a tty Note that stdout/stderr are already redirected by bats' `run` command, so the only way to get a controlling terminal is to open /dev/tty (which fails if there isn't one). Here's how I tested the failure to open /dev/tty: > [root@kir-rhat ~]# ssh -T root@localhost cat ./runme > cd /home/kir/go/src/github.com/opencontainers/runc > ./runc run -b tst xxx-$$ > echo $? > > [root@kir-rhat ~]# ssh -T root@localhost ./runme > time="2020-07-31T16:35:47-07:00" level=error msg="chdir tst: no such file or directory" > 1 If anyone knows how to obtain an tty-less environment without using ssh -T, please raise your hand. Signed-off-by: Kir Kolyshkin --- tests/integration/tty.bats | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/integration/tty.bats b/tests/integration/tty.bats index 7193f327c28..8d4919a2703 100644 --- a/tests/integration/tty.bats +++ b/tests/integration/tty.bats @@ -11,6 +11,14 @@ function teardown() { teardown_busybox } +@test "runc run [stdin not a tty]" { + # stty size fails without a tty + update_config '(.. | select(.[]? == "sh")) += ["-c", "stty size"]' + # note that stdout/stderr are already redirected by bats' run + runc run test_busybox < /dev/null + [ "$status" -eq 0 ] +} + @test "runc run [tty ptsname]" { # Replace sh script with readlink. # shellcheck disable=SC2016 @@ -61,6 +69,18 @@ function teardown() { [[ ${lines[1]} =~ 5 ]] } +@test "runc exec [stdin not a tty]" { + runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox + [ "$status" -eq 0 ] + + # make sure we're running + testcontainer test_busybox running + + # note that stdout/stderr are already redirected by bats' run + runc exec -t test_busybox sh -c "stty size" < /dev/null + [ "$status" -eq 0 ] +} + @test "runc exec [tty ptsname]" { # run busybox detached runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox