diff options
author | gVisor bot <gvisor-bot@google.com> | 2020-04-14 11:20:11 -0700 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2020-04-14 11:20:11 -0700 |
commit | ac9b32c36b40e51a77e44c0b82ecd349d77a4379 (patch) | |
tree | dc6a23f210030adae6acb1e725e5ac9a5a0e152d | |
parent | 81c44c4cd7cfa121d9ef028db18b3ee550845811 (diff) | |
parent | 0cfdd47391d30dfe8214e2d11bdad9b27419ad26 (diff) |
Merge pull request #2212 from aaronlu:dup_stdioFDs
PiperOrigin-RevId: 306477639
-rw-r--r-- | runsc/boot/BUILD | 1 | ||||
-rw-r--r-- | runsc/boot/loader.go | 45 | ||||
-rw-r--r-- | runsc/boot/loader_test.go | 12 |
3 files changed, 52 insertions, 6 deletions
diff --git a/runsc/boot/BUILD b/runsc/boot/BUILD index 26f68fe3d..23f42382f 100644 --- a/runsc/boot/BUILD +++ b/runsc/boot/BUILD @@ -119,5 +119,6 @@ go_test( "//pkg/unet", "//runsc/fsgofer", "@com_github_opencontainers_runtime-spec//specs-go:go_default_library", + "@org_golang_x_sys//unix:go_default_library", ], ) diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index e7ca98134..654441f65 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -156,13 +156,17 @@ type Args struct { Spec *specs.Spec // Conf is the system configuration. Conf *Config - // ControllerFD is the FD to the URPC controller. + // ControllerFD is the FD to the URPC controller. The Loader takes ownership + // of this FD and may close it at any time. ControllerFD int - // Device is an optional argument that is passed to the platform. + // Device is an optional argument that is passed to the platform. The Loader + // takes ownership of this file and may close it at any time. Device *os.File - // GoferFDs is an array of FDs used to connect with the Gofer. + // GoferFDs is an array of FDs used to connect with the Gofer. The Loader + // takes ownership of these FDs and may close them at any time. GoferFDs []int - // StdioFDs is the stdio for the application. + // StdioFDs is the stdio for the application. The Loader takes ownership of + // these FDs and may close them at any time. StdioFDs []int // Console is set to true if using TTY. Console bool @@ -175,6 +179,9 @@ type Args struct { UserLogFD int } +// make sure stdioFDs are always the same on initial start and on restore +const startingStdioFD = 64 + // New initializes a new kernel loader configured by spec. // New also handles setting up a kernel for restoring a container. func New(args Args) (*Loader, error) { @@ -319,6 +326,24 @@ func New(args Args) (*Loader, error) { return nil, fmt.Errorf("creating pod mount hints: %v", err) } + // Make host FDs stable between invocations. Host FDs must map to the exact + // same number when the sandbox is restored. Otherwise the wrong FD will be + // used. + var stdioFDs []int + newfd := startingStdioFD + for _, fd := range args.StdioFDs { + err := syscall.Dup3(fd, newfd, syscall.O_CLOEXEC) + if err != nil { + return nil, fmt.Errorf("dup3 of stdioFDs failed: %v", err) + } + stdioFDs = append(stdioFDs, newfd) + err = syscall.Close(fd) + if err != nil { + return nil, fmt.Errorf("close original stdioFDs failed: %v", err) + } + newfd++ + } + eid := execID{cid: args.ID} l := &Loader{ k: k, @@ -327,7 +352,7 @@ func New(args Args) (*Loader, error) { watchdog: dog, spec: args.Spec, goferFDs: args.GoferFDs, - stdioFDs: args.StdioFDs, + stdioFDs: stdioFDs, rootProcArgs: procArgs, sandboxID: args.ID, processes: map[execID]*execProcess{eid: {}}, @@ -569,6 +594,16 @@ func (l *Loader) run() error { } }) + // l.stdioFDs are derived from dup() in boot.New() and they are now dup()ed again + // either in createFDTable() during initial start or in descriptor.initAfterLoad() + // during restore, we can release l.stdioFDs now. + for _, fd := range l.stdioFDs { + err := syscall.Close(fd) + if err != nil { + return fmt.Errorf("close dup()ed stdioFDs: %v", err) + } + } + log.Infof("Process should have started...") l.watchdog.Start() return l.k.Start() diff --git a/runsc/boot/loader_test.go b/runsc/boot/loader_test.go index 44aa63196..c9a75b76d 100644 --- a/runsc/boot/loader_test.go +++ b/runsc/boot/loader_test.go @@ -24,6 +24,7 @@ import ( "time" specs "github.com/opencontainers/runtime-spec/specs-go" + "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/control/server" "gvisor.dev/gvisor/pkg/log" "gvisor.dev/gvisor/pkg/p9" @@ -113,7 +114,16 @@ func createLoader() (*Loader, func(), error) { return nil, nil, err } - stdio := []int{int(os.Stdin.Fd()), int(os.Stdout.Fd()), int(os.Stderr.Fd())} + // Loader takes ownership of stdio. + var stdio []int + for _, f := range []*os.File{os.Stdin, os.Stdout, os.Stderr} { + newFd, err := unix.Dup(int(f.Fd())) + if err != nil { + return nil, nil, err + } + stdio = append(stdio, newFd) + } + args := Args{ ID: "foo", Spec: spec, |