netbird/gvisor
0
0
Fork 0
mirror of https://github.com/netbirdio/gvisor.git synced 2026-05-22 17:12:49 -07:00
Code Issues Packages Projects Releases Wiki Activity

runsc: Fix stdin/stdout/stderr in multi-container mode.

Browse Source 
The issue with the previous change was that the stdin/stdout/stderr passed to
the sentry were dup'd by host.ImportFile. This left a dangling FD that by never
closing caused containerd to timeout waiting on container stop.

PiperOrigin-RevId: 213753032
Change-Id: Ia5e4c0565c42c8610d3b59f65599a5643b0901e4
This commit is contained in:
Kevin Krakauer
2018-09-19 22:20:41 -07:00
committed by Shentubot
parent 915d76aa92
commit ffb5fdd690
5 changed files with 50 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
runsc/boot/controller.go
+7 -5
View File
@@ -193,8 +193,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
}
@@ -222,8 +224,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)
@@ -408,7 +410,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)
runsc/boot/fds.go
+9 -5
View File
@@ -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,20 @@ 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) {
// Upon success, createFDMap dups then closes stdioFDs.
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)
runsc/boot/fs.go
+7 -7
View File
@@ -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)
if conf.MultiContainer {
// Create a tmpfs mount where we create and mount a root filesystem for
@@ -92,7 +92,7 @@ func createMountNamespace(userCtx context.Context, rootCtx context.Context, spec
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)
@@ -587,14 +587,14 @@ 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 {
// procArgs are passed by reference and the FDMap field is modified. It dups stdioFDs.
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)
}
@@ -618,7 +618,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)
}
@@ -630,7 +630,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)
runsc/boot/loader.go
+21 -6
View File
@@ -78,8 +78,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
@@ -139,7 +142,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) {
if err := usage.Init(); err != nil {
return nil, fmt.Errorf("Error setting up memory usage: %v", err)
}
@@ -278,7 +281,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,
@@ -390,7 +394,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,
@@ -474,11 +479,14 @@ func (l *Loader) startContainer(k *kernel.Kernel, spec *specs.Spec, conf *Config
ioFDs = append(ioFDs, fd)
}
stdioFDs := ioFDs[:3]
goferFDs := ioFDs[3:]
if err := setFileSystemForProcess(
&procArgs,
spec,
conf,
ioFDs,
stdioFDs,
goferFDs,
false,
creds,
procArgs.Limits,
@@ -487,6 +495,13 @@ func (l *Loader) startContainer(k *kernel.Kernel, spec *specs.Spec, conf *Config
return fmt.Errorf("failed to create new process: %v", err)
}
// setFileSystemForProcess dup'd stdioFDs, so we can close them.
for i, fd := range stdioFDs {
if err := syscall.Close(fd); err != nil {
return fmt.Errorf("failed to close stdioFD #%d: %v", i, fd)
}
}
ctx := procArgs.NewContext(l.k)
mns := k.RootMountNamespace()
if err := setExecutablePath(ctx, mns, &procArgs); err != nil {
runsc/sandbox/sandbox.go
+6 -3
View File
@@ -101,8 +101,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()
}
@@ -113,12 +113,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)