From 380eb673f54673d64e4bc72ac5c5ec32b9e7fb8a Mon Sep 17 00:00:00 2001 From: Daniel Dao Date: Sun, 7 May 2017 22:33:57 -0500 Subject: [PATCH] send slavefd to the other side when terminal=true Currently when terminal=true and container side decides to close stdio and also eventually close /dev/console, it effectively looses the slave side of the pty pair, and IO on the master results in EIO If you use EPOLL on all IO we can pretend EIO is ok and continue, but we dont. And moving to EPOLL is not really easy or straighforward. Instead when terminal=true, this patch attempts to send over the valid slavefd when it was opened, and the master side will hold on to it, so no matter what the container side is doing we will always have a valid pty pair. The following Go program simulate the container behavior: ``` package main import ( "fmt" "log" "os" "syscall" "time" ) func main() { console, err := os.OpenFile("/dev/console", os.O_RDWR, 0) if err != nil { log.Fatal(err) } fmt.Fprintln(console, "write before dup /dev/null to stdio") makeNullStdio() fmt.Fprintln(console, "write after dup /dev/null to stdio") console.Close() logf, _ := os.Create("/tmp/logfile") for { console, err = os.OpenFile("/dev/console", os.O_RDWR, 0) if err != nil { fmt.Fprint(logf, err) os.Exit(1) } fmt.Fprintln(console, "open, write to console, and close") console.Close() time.Sleep(time.Second) } } func makeNullStdio() { nullFd, _ := os.OpenFile("/dev/null", os.O_RDWR, 0) defer nullFd.Close() fd := int(nullFd.Fd()) syscall.Dup2(fd, 0) syscall.Dup2(fd, 1) syscall.Dup2(fd, 2) } ``` Without the patch, we have the following output: ``` hanamura# runc run hello write before dup /dev/null to stdio write after dup /dev/null to stdio ``` After the patch ``` hanamura# runc run hello write before dup /dev/null to stdio write after dup /dev/null to stdio open, write to console, and close open, write to console, and close open, write to console, and close ``` Signed-off-by: Daniel Dao --- libcontainer/console_linux.go | 6 ++++++ libcontainer/init_linux.go | 8 +++++++- libcontainer/integration/execin_test.go | 11 +++++++++-- tty.go | 13 ++++++++++++- 4 files changed, 34 insertions(+), 4 deletions(-) diff --git a/libcontainer/console_linux.go b/libcontainer/console_linux.go index 5e364a88a1e..cbc0d59fc83 100644 --- a/libcontainer/console_linux.go +++ b/libcontainer/console_linux.go @@ -40,6 +40,7 @@ func newConsole() (Console, error) { // linuxConsole is a linux pseudo TTY for use within a container. type linuxConsole struct { master *os.File + slave *os.File slavePath string } @@ -47,6 +48,10 @@ func (c *linuxConsole) File() *os.File { return c.master } +func (c *linuxConsole) Slave() *os.File { + return c.slave +} + func (c *linuxConsole) Path() string { return c.slavePath } @@ -94,6 +99,7 @@ func (c *linuxConsole) dupStdio() error { return err } } + c.slave = slave return nil } diff --git a/libcontainer/init_linux.go b/libcontainer/init_linux.go index 99cc02cbd02..9f0aac2934c 100644 --- a/libcontainer/init_linux.go +++ b/libcontainer/init_linux.go @@ -189,7 +189,13 @@ func setupConsole(socket *os.File, config *initConfig, mount bool) error { return err } // Now, dup over all the things. - return linuxConsole.dupStdio() + if err := linuxConsole.dupStdio(); err != nil { + return err + } + // send the slave over to the other side to keep track of it. This makes sure + // that no matter what we do on the container side, the master-slave pipe + // will never be invalidated as long as we hold on to the valid slavefd. + return utils.SendFd(socket, linuxConsole.Slave()) } // syncParentReady sends to the given pipe a JSON payload which indicates that diff --git a/libcontainer/integration/execin_test.go b/libcontainer/integration/execin_test.go index 019757f643c..fb02d921e52 100644 --- a/libcontainer/integration/execin_test.go +++ b/libcontainer/integration/execin_test.go @@ -293,14 +293,21 @@ func TestExecInTTY(t *testing.T) { } dc := make(chan *cdata, 1) go func() { - f, err := utils.RecvFd(parent) + master, err := utils.RecvFd(parent) if err != nil { dc <- &cdata{ err: err, } } + slave, err := utils.RecvFd(parent) + if err != nil { + dc <- &cdata{ + err: err, + } + } + slave.Close() dc <- &cdata{ - c: libcontainer.ConsoleFromFile(f), + c: libcontainer.ConsoleFromFile(master), } }() err = container.Run(ps) diff --git a/tty.go b/tty.go index 9824df144c5..659cf91cdcc 100644 --- a/tty.go +++ b/tty.go @@ -15,6 +15,7 @@ import ( type tty struct { console libcontainer.Console + slave *os.File state *term.State closers []io.Closer postStart []io.Closer @@ -72,7 +73,13 @@ func inheritStdio(process *libcontainer.Process) error { func (t *tty) recvtty(process *libcontainer.Process, socket *os.File) error { f, err := utils.RecvFd(socket) if err != nil { - return err + return fmt.Errorf("failed to receive master %s", err) + } + // read the slave here and hold on to it to make sure that the master-slave + // pipe is valid for the container lifetime. + slave, err := utils.RecvFd(socket) + if err != nil { + return fmt.Errorf("failed to receive slave %s", err) } console := libcontainer.ConsoleFromFile(f) go io.Copy(console, os.Stdin) @@ -84,6 +91,7 @@ func (t *tty) recvtty(process *libcontainer.Process, socket *os.File) error { } t.state = state t.console = console + t.slave = slave t.closers = []io.Closer{console} return nil } @@ -111,6 +119,9 @@ func (t *tty) Close() error { for _, c := range t.postStart { c.Close() } + // close the slave first before we wait for all io to be done then close the + // master in t.closers + t.slave.Close() // wait for the copy routines to finish before closing the fds t.wg.Wait() for _, c := range t.closers {