Skip to content

Commit

Permalink
send slavefd to the other side when terminal=true
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
dqminh committed May 11, 2017
1 parent 2daa115 commit 380eb67
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 4 deletions.
6 changes: 6 additions & 0 deletions libcontainer/console_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,18 @@ 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
}

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
}
Expand Down Expand Up @@ -94,6 +99,7 @@ func (c *linuxConsole) dupStdio() error {
return err
}
}
c.slave = slave
return nil
}

Expand Down
8 changes: 7 additions & 1 deletion libcontainer/init_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 9 additions & 2 deletions libcontainer/integration/execin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
13 changes: 12 additions & 1 deletion tty.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (

type tty struct {
console libcontainer.Console
slave *os.File
state *term.State
closers []io.Closer
postStart []io.Closer
Expand Down Expand Up @@ -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)
Expand All @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 380eb67

Please sign in to comment.