From e215b9970ad82915a8d544b81b3c49d7d84a0eb0 Mon Sep 17 00:00:00 2001 From: Nicolas Lacasse Date: Wed, 3 Oct 2018 10:31:01 -0700 Subject: [PATCH] runsc: Pass root container's stdio via FD. We were previously using the sandbox process's stdio as the root container's stdio. This makes it difficult/impossible to distinguish output application output from sandbox output, such as panics, which are always written to stderr. Also close the console socket when we are done with it. PiperOrigin-RevId: 215585180 Change-Id: I980b8c69bd61a8b8e0a496fd7bc90a06446764e0 --- runsc/boot/loader.go | 6 ++--- runsc/boot/loader_test.go | 3 ++- runsc/cmd/boot.go | 7 +++++- runsc/console/console.go | 2 ++ runsc/sandbox/sandbox.go | 50 +++++++++++++++++++++++++++++---------- 5 files changed, 50 insertions(+), 18 deletions(-) diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index 766a2e968..726482bb2 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -140,7 +140,7 @@ func init() { // New initializes a new kernel loader configured by spec. // New also handles setting up a kernel for restoring a container. -func New(id string, spec *specs.Spec, conf *Config, controllerFD, deviceFD int, goferFDs []int, console bool) (*Loader, error) { +func New(id string, spec *specs.Spec, conf *Config, controllerFD, deviceFD int, goferFDs []int, stdioFDs []int, console bool) (*Loader, error) { if err := usage.Init(); err != nil { return nil, fmt.Errorf("Error setting up memory usage: %v", err) } @@ -279,9 +279,9 @@ func New(id string, spec *specs.Spec, conf *Config, controllerFD, deviceFD int, conf: conf, console: console, watchdog: watchdog, - stdioFDs: []int{syscall.Stdin, syscall.Stdout, syscall.Stderr}, - goferFDs: goferFDs, spec: spec, + goferFDs: goferFDs, + stdioFDs: stdioFDs, startSignalForwarding: startSignalForwarding, rootProcArgs: procArgs, sandboxID: id, diff --git a/runsc/boot/loader_test.go b/runsc/boot/loader_test.go index 0b363253d..ea8411a8b 100644 --- a/runsc/boot/loader_test.go +++ b/runsc/boot/loader_test.go @@ -101,7 +101,8 @@ func createLoader() (*Loader, func(), error) { return nil, nil, err } - l, err := New("foo", spec, conf, fd, -1 /* device fd */, []int{sandEnd}, false) + stdio := []int{int(os.Stdin.Fd()), int(os.Stdout.Fd()), int(os.Stderr.Fd())} + l, err := New("foo", spec, conf, fd, -1 /* device fd */, []int{sandEnd}, stdio, false) if err != nil { cleanup() return nil, nil, err diff --git a/runsc/cmd/boot.go b/runsc/cmd/boot.go index 82e534479..c6f78f63f 100644 --- a/runsc/cmd/boot.go +++ b/runsc/cmd/boot.go @@ -48,6 +48,10 @@ type Boot struct { // ioFDs is the list of FDs used to connect to FS gofers. ioFDs intFlags + // stdioFDs are the fds for stdin, stdout, and stderr. They must be + // provided in that order. + stdioFDs intFlags + // console is set to true if the sandbox should allow terminal ioctl(2) // syscalls. console bool @@ -79,6 +83,7 @@ func (b *Boot) SetFlags(f *flag.FlagSet) { f.IntVar(&b.controllerFD, "controller-fd", -1, "required FD of a stream socket for the control server that must be donated to this process") f.IntVar(&b.deviceFD, "device-fd", -1, "FD for the platform device file") f.Var(&b.ioFDs, "io-fds", "list of FDs to connect 9P clients. They must follow this order: root first, then mounts as defined in the spec") + f.Var(&b.stdioFDs, "stdio-fds", "list of FDs containing sandbox stdin, stdout, and stderr in that order") f.BoolVar(&b.console, "console", false, "set to true if the sandbox should allow terminal ioctl(2) syscalls") f.BoolVar(&b.applyCaps, "apply-caps", false, "if true, apply capabilities defined in the spec to the process") } @@ -138,7 +143,7 @@ func (b *Boot) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) } // Create the loader. - l, err := boot.New(f.Arg(0), spec, conf, b.controllerFD, b.deviceFD, b.ioFDs.GetArray(), b.console) + l, err := boot.New(f.Arg(0), spec, conf, b.controllerFD, b.deviceFD, b.ioFDs.GetArray(), b.stdioFDs.GetArray(), b.console) if err != nil { Fatalf("error creating loader: %v", err) } diff --git a/runsc/console/console.go b/runsc/console/console.go index 2f2745b2b..3df184742 100644 --- a/runsc/console/console.go +++ b/runsc/console/console.go @@ -40,6 +40,7 @@ func NewWithSocket(socketPath string) (*os.File, error) { ptySlave.Close() return nil, fmt.Errorf("error dial socket %q: %v", socketPath, err) } + defer conn.Close() uc, ok := conn.(*net.UnixConn) if !ok { ptySlave.Close() @@ -50,6 +51,7 @@ func NewWithSocket(socketPath string) (*os.File, error) { ptySlave.Close() return nil, fmt.Errorf("error getting file for unix socket %v: %v", uc, err) } + defer socket.Close() // Send the master FD over the connection. msg := unix.UnixRights(int(ptyMaster.Fd())) diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index e4853af69..1ed1ab61d 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -285,9 +285,6 @@ func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bund // All flags after this must be for the boot command cmd.Args = append(cmd.Args, "boot", "--bundle="+bundleDir) - consoleEnabled := consoleSocket != "" - cmd.Args = append(cmd.Args, "--console="+strconv.FormatBool(consoleEnabled)) - // Create a socket for the control server and donate it to the sandbox. addr := boot.ControlSocketAddr(s.ID) sockFD, err := server.CreateSocket(addr) @@ -332,27 +329,54 @@ func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bund nextFD++ } - // Sandbox stdio defaults to current process stdio. - cmd.Stdin = os.Stdin - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - // If the console control socket file is provided, then create a new // pty master/slave pair and set the TTY on the sandbox process. - if consoleEnabled { - // console.NewWithSocket will send the master on the socket, - // and return the slave. + if consoleSocket != "" { + cmd.Args = append(cmd.Args, "--console=true") + + // console.NewWithSocket will send the master on the given + // socket, and return the slave. tty, err := console.NewWithSocket(consoleSocket) if err != nil { return fmt.Errorf("error setting up console with socket %q: %v", consoleSocket, err) } defer tty.Close() + fd := int(tty.Fd()) + // Set the TTY as a controlling TTY on the sandbox process. + cmd.SysProcAttr.Setctty = true + cmd.SysProcAttr.Ctty = fd + + // Ideally we would set the sandbox stdin to this process' + // stdin, but for some reason Docker does not like that (it + // never calls `runsc start`). Instead we set stdio to the + // console TTY, but note that this is distinct from the + // container stdio, which is passed via the flags below. cmd.Stdin = tty cmd.Stdout = tty cmd.Stderr = tty - cmd.SysProcAttr.Setctty = true - cmd.SysProcAttr.Ctty = int(tty.Fd()) + + // Pass the tty as all stdio fds to sandbox. + for i := 0; i < 3; i++ { + cmd.ExtraFiles = append(cmd.ExtraFiles, tty) + cmd.Args = append(cmd.Args, "--stdio-fds="+strconv.Itoa(nextFD)) + nextFD++ + } + } else { + // Connect the sandbox process to this process's stdios. Note + // that this is distinct from the container's stdio, which is + // passed by the flags below. + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + + // If not using a console, pass our current stdio as the + // container stdio via flags. + for _, f := range []*os.File{os.Stdin, os.Stdout, os.Stderr} { + cmd.ExtraFiles = append(cmd.ExtraFiles, f) + cmd.Args = append(cmd.Args, "--stdio-fds="+strconv.Itoa(nextFD)) + nextFD++ + } } // Detach from this session, otherwise cmd will get SIGHUP and SIGCONT