From 25add7b22b1b0b6a4bac1e72536d3f3a0c70f048 Mon Sep 17 00:00:00 2001 From: Kevin Krakauer Date: Mon, 17 Sep 2018 11:30:16 -0700 Subject: runsc: Fix stdin/out/err in multi-container mode. Stdin/out/err weren't being sent to the sentry. PiperOrigin-RevId: 213307171 Change-Id: Ie4b634a58b1b69aa934ce8597e5cc7a47a2bcda2 --- runsc/boot/controller.go | 12 +++++++----- runsc/boot/fds.go | 13 ++++++++----- runsc/boot/fs.go | 12 ++++++------ runsc/boot/loader.go | 18 ++++++++++++------ runsc/sandbox/sandbox.go | 9 ++++++--- 5 files changed, 39 insertions(+), 25 deletions(-) diff --git a/runsc/boot/controller.go b/runsc/boot/controller.go index 69154ff23..4d41dcd6c 100644 --- a/runsc/boot/controller.go +++ b/runsc/boot/controller.go @@ -186,8 +186,10 @@ type StartArgs struct { // CID is the ID of the container to start. CID string - // FilePayload contains the file descriptor over which the sandbox will - // request files from its root filesystem. + // FilePayload contains, in order: + // * stdin, stdout, and stderr. + // * the file descriptor over which the sandbox will + // request files from its root filesystem. urpc.FilePayload } @@ -215,8 +217,8 @@ func (cm *containerManager) Start(args *StartArgs, _ *struct{}) error { if path.Clean(args.CID) != args.CID { return fmt.Errorf("container ID shouldn't contain directory traversals such as \"..\": %q", args.CID) } - if len(args.FilePayload.Files) == 0 { - return fmt.Errorf("start arguments must contain at least one file for the container root") + if len(args.FilePayload.Files) < 4 { + return fmt.Errorf("start arguments must contain stdin, stderr, and stdout followed by at least one file for the container root gofer") } err := cm.l.startContainer(cm.l.k, args.Spec, args.Conf, args.CID, args.FilePayload.Files) @@ -339,7 +341,7 @@ func (cm *containerManager) Restore(o *RestoreOpts, _ *struct{}) error { cm.l.k = k // Set up the restore environment. - fds := &fdDispenser{fds: cm.l.ioFDs} + fds := &fdDispenser{fds: cm.l.goferFDs} renv, err := createRestoreEnvironment(cm.l.spec, cm.l.conf, fds) if err != nil { return fmt.Errorf("error creating RestoreEnvironment: %v", err) diff --git a/runsc/boot/fds.go b/runsc/boot/fds.go index 9de5a78b1..91c698fea 100644 --- a/runsc/boot/fds.go +++ b/runsc/boot/fds.go @@ -16,7 +16,6 @@ package boot import ( "fmt" - "syscall" "gvisor.googlesource.com/gvisor/pkg/sentry/context" "gvisor.googlesource.com/gvisor/pkg/sentry/fs" @@ -28,15 +27,19 @@ import ( // createFDMap creates an fd map that contains stdin, stdout, and stderr. If // console is true, then ioctl calls will be passed through to the host fd. -func createFDMap(ctx context.Context, k *kernel.Kernel, l *limits.LimitSet, console bool) (*kernel.FDMap, error) { +func createFDMap(ctx context.Context, k *kernel.Kernel, l *limits.LimitSet, console bool, stdioFDs []int) (*kernel.FDMap, error) { + if len(stdioFDs) != 3 { + return nil, fmt.Errorf("stdioFDs should contain exactly 3 FDs (stdin, stdout, and stderr), but %d FDs received", len(stdioFDs)) + } + fdm := k.NewFDMap() defer fdm.DecRef() // Maps sandbox fd to host fd. fdMap := map[int]int{ - 0: syscall.Stdin, - 1: syscall.Stdout, - 2: syscall.Stderr, + 0: stdioFDs[0], + 1: stdioFDs[1], + 2: stdioFDs[2], } mounter := fs.FileOwnerFromContext(ctx) diff --git a/runsc/boot/fs.go b/runsc/boot/fs.go index 5ec9a7d03..45843fe7b 100644 --- a/runsc/boot/fs.go +++ b/runsc/boot/fs.go @@ -82,7 +82,7 @@ func (f *fdDispenser) empty() bool { // createMountNamespace creates a mount namespace containing the root filesystem // and all mounts. 'rootCtx' is used to walk directories to find mount points. -func createMountNamespace(userCtx context.Context, rootCtx context.Context, spec *specs.Spec, conf *Config, ioFDs []int) (*fs.MountNamespace, error) { +func createMountNamespace(userCtx context.Context, rootCtx context.Context, spec *specs.Spec, conf *Config, goferFDs []int) (*fs.MountNamespace, error) { mounts := compileMounts(spec) // Create a tmpfs mount where we create and mount a root filesystem for // each child container. @@ -90,7 +90,7 @@ func createMountNamespace(userCtx context.Context, rootCtx context.Context, spec Type: tmpfs, Destination: childContainersDir, }) - fds := &fdDispenser{fds: ioFDs} + fds := &fdDispenser{fds: goferFDs} rootInode, err := createRootMount(rootCtx, spec, conf, fds, mounts) if err != nil { return nil, fmt.Errorf("failed to create root mount: %v", err) @@ -595,13 +595,13 @@ func subtargets(root string, mnts []specs.Mount) []string { // setFileSystemForProcess is used to set up the file system and amend the procArgs accordingly. // procArgs are passed by reference and the FDMap field is modified. -func setFileSystemForProcess(procArgs *kernel.CreateProcessArgs, spec *specs.Spec, conf *Config, ioFDs []int, console bool, creds *auth.Credentials, ls *limits.LimitSet, k *kernel.Kernel, cid string) error { +func setFileSystemForProcess(procArgs *kernel.CreateProcessArgs, spec *specs.Spec, conf *Config, stdioFDs, goferFDs []int, console bool, creds *auth.Credentials, ls *limits.LimitSet, k *kernel.Kernel, cid string) error { ctx := procArgs.NewContext(k) // Create the FD map, which will set stdin, stdout, and stderr. If // console is true, then ioctl calls will be passed through to the host // fd. - fdm, err := createFDMap(ctx, k, ls, console) + fdm, err := createFDMap(ctx, k, ls, console, stdioFDs) if err != nil { return fmt.Errorf("error importing fds: %v", err) } @@ -625,7 +625,7 @@ func setFileSystemForProcess(procArgs *kernel.CreateProcessArgs, spec *specs.Spe mns := k.RootMountNamespace() if mns == nil { // Create the virtual filesystem. - mns, err := createMountNamespace(ctx, rootCtx, spec, conf, ioFDs) + mns, err := createMountNamespace(ctx, rootCtx, spec, conf, goferFDs) if err != nil { return fmt.Errorf("error creating mounts: %v", err) } @@ -637,7 +637,7 @@ func setFileSystemForProcess(procArgs *kernel.CreateProcessArgs, spec *specs.Spe // Create the container's root filesystem mount. log.Infof("Creating new process in child container.") - fds := &fdDispenser{fds: append([]int{}, ioFDs...)} + fds := &fdDispenser{fds: append([]int{}, goferFDs...)} rootInode, err := createRootMount(rootCtx, spec, conf, fds, nil) if err != nil { return fmt.Errorf("error creating filesystem for container: %v", err) diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index 2ddb358bd..5e9ccb96f 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -77,8 +77,11 @@ type Loader struct { watchdog *watchdog.Watchdog - // ioFDs are the FDs that attach the sandbox to the gofers. - ioFDs []int + // stdioFDs contains stdin, stdout, and stderr. + stdioFDs []int + + // goferFDs are the FDs that attach the sandbox to the gofers. + goferFDs []int // spec is the base configuration for the root container. spec *specs.Spec @@ -121,7 +124,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(spec *specs.Spec, conf *Config, controllerFD, deviceFD int, ioFDs []int, console bool) (*Loader, error) { +func New(spec *specs.Spec, conf *Config, controllerFD, deviceFD int, goferFDs []int, console bool) (*Loader, error) { // Create kernel and platform. p, err := createPlatform(conf, deviceFD) if err != nil { @@ -252,7 +255,8 @@ func New(spec *specs.Spec, conf *Config, controllerFD, deviceFD int, ioFDs []int conf: conf, console: console, watchdog: watchdog, - ioFDs: ioFDs, + stdioFDs: []int{syscall.Stdin, syscall.Stdout, syscall.Stderr}, + goferFDs: goferFDs, spec: spec, startSignalForwarding: startSignalForwarding, rootProcArgs: procArgs, @@ -364,7 +368,8 @@ func (l *Loader) run() error { &l.rootProcArgs, l.spec, l.conf, - l.ioFDs, + l.stdioFDs, + l.goferFDs, l.console, l.rootProcArgs.Credentials, l.rootProcArgs.Limits, @@ -446,7 +451,8 @@ func (l *Loader) startContainer(k *kernel.Kernel, spec *specs.Spec, conf *Config &procArgs, spec, conf, - ioFDs, + ioFDs[:3], // stdioFDs + ioFDs[3:], // goferFDs false, creds, procArgs.Limits, diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index 156b2f769..8c4d0d495 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -100,8 +100,8 @@ func (s *Sandbox) StartRoot(spec *specs.Spec, conf *boot.Config) error { } // Start starts running a non-root container inside the sandbox. -func (s *Sandbox) Start(spec *specs.Spec, conf *boot.Config, cid string, ioFiles []*os.File) error { - for _, f := range ioFiles { +func (s *Sandbox) Start(spec *specs.Spec, conf *boot.Config, cid string, goferFiles []*os.File) error { + for _, f := range goferFiles { defer f.Close() } @@ -112,12 +112,15 @@ func (s *Sandbox) Start(spec *specs.Spec, conf *boot.Config, cid string, ioFiles } defer sandboxConn.Close() + // The payload must container stdin/stdout/stderr followed by gofer + // files. + files := append([]*os.File{os.Stdin, os.Stdout, os.Stderr}, goferFiles...) // Start running the container. args := boot.StartArgs{ Spec: spec, Conf: conf, CID: cid, - FilePayload: urpc.FilePayload{Files: ioFiles}, + FilePayload: urpc.FilePayload{Files: files}, } if err := sandboxConn.Call(boot.ContainerStart, &args, nil); err != nil { return fmt.Errorf("error starting non-root container %v: %v", spec.Process.Args, err) -- cgit v1.2.3