From 9751b800a6835f7febf99f1dee22a5aedd43f381 Mon Sep 17 00:00:00 2001 From: Nicolas Lacasse Date: Fri, 7 Sep 2018 17:38:34 -0700 Subject: runsc: Support multi-container exec. We must use a context.Context with a Root Dirent that corresponds to the container's chroot. Previously we were using the root context, which does not have a chroot. Getting the correct context required refactoring some of the path-lookup code. We can't lookup the path without a context.Context, which requires kernel.CreateProcArgs, which we only get inside control.Execute. So we have to do the path lookup much later than we previously were. PiperOrigin-RevId: 212064734 Change-Id: I84a5cfadacb21fd9c3ab9c393f7e308a40b9b537 --- pkg/sentry/control/proc.go | 18 +++++++++++- pkg/sentry/fs/mounts.go | 66 ++++++++++++++++++++++++++++++++++++++++++++ runsc/boot/controller.go | 36 ++++++++++++++++++------ runsc/boot/fs.go | 58 ++------------------------------------ runsc/sandbox/sandbox.go | 7 ++++- runsc/specutils/specutils.go | 12 -------- 6 files changed, 118 insertions(+), 79 deletions(-) diff --git a/pkg/sentry/control/proc.go b/pkg/sentry/control/proc.go index 6949a3ae5..289b8ba0e 100644 --- a/pkg/sentry/control/proc.go +++ b/pkg/sentry/control/proc.go @@ -54,6 +54,11 @@ type ExecArgs struct { // Envv is a list of environment variables. Envv []string `json:"envv"` + // Root defines the root directory for the new process. A reference on + // Root must be held for the lifetime of the ExecArgs. If Root is nil, + // it will default to the VFS root. + Root *fs.Dirent + // WorkingDirectory defines the working directory for the new process. WorkingDirectory string `json:"wd"` @@ -99,6 +104,7 @@ func (proc *Proc) Exec(args *ExecArgs, waitStatus *uint32) error { Argv: args.Argv, Envv: args.Envv, WorkingDirectory: args.WorkingDirectory, + Root: args.Root, Credentials: creds, FDMap: fdm, Umask: 0022, @@ -109,8 +115,18 @@ func (proc *Proc) Exec(args *ExecArgs, waitStatus *uint32) error { AbstractSocketNamespace: proc.Kernel.RootAbstractSocketNamespace(), } ctx := initArgs.NewContext(proc.Kernel) - mounter := fs.FileOwnerFromContext(ctx) + if initArgs.Filename == "" { + // Get the full path to the filename from the PATH env variable. + paths := fs.GetPath(initArgs.Envv) + f, err := proc.Kernel.RootMountNamespace().ResolveExecutablePath(ctx, initArgs.WorkingDirectory, initArgs.Argv[0], paths) + if err != nil { + return fmt.Errorf("error finding executable %q in PATH %v: %v", initArgs.Argv[0], paths, err) + } + initArgs.Filename = f + } + + mounter := fs.FileOwnerFromContext(ctx) for appFD, f := range args.FilePayload.Files { enableIoctl := args.StdioIsPty && appFD <= 2 diff --git a/pkg/sentry/fs/mounts.go b/pkg/sentry/fs/mounts.go index 0318f135d..c0a803b2d 100644 --- a/pkg/sentry/fs/mounts.go +++ b/pkg/sentry/fs/mounts.go @@ -16,9 +16,13 @@ package fs import ( "fmt" + "path" + "strings" "sync" "syscall" + "gvisor.googlesource.com/gvisor/pkg/abi/linux" + "gvisor.googlesource.com/gvisor/pkg/log" "gvisor.googlesource.com/gvisor/pkg/refs" "gvisor.googlesource.com/gvisor/pkg/sentry/context" "gvisor.googlesource.com/gvisor/pkg/sentry/kernel/auth" @@ -509,3 +513,65 @@ func (mns *MountNamespace) SyncAll(ctx context.Context) { defer mns.mu.Unlock() mns.root.SyncAll(ctx) } + +// ResolveExecutablePath resolves the given executable name given a set of +// paths that might contain it. +func (mns *MountNamespace) ResolveExecutablePath(ctx context.Context, wd, name string, paths []string) (string, error) { + // Absolute paths can be used directly. + if path.IsAbs(name) { + return name, nil + } + + // Paths with '/' in them should be joined to the working directory, or + // to the root if working directory is not set. + if strings.IndexByte(name, '/') > 0 { + if wd == "" { + wd = "/" + } + if !path.IsAbs(wd) { + return "", fmt.Errorf("working directory %q must be absolute", wd) + } + return path.Join(wd, name), nil + } + + // Otherwise, We must lookup the name in the paths, starting from the + // calling context's root directory. + root := RootFromContext(ctx) + if root == nil { + // Caller has no root. Don't bother traversing anything. + return "", syserror.ENOENT + } + defer root.DecRef() + for _, p := range paths { + binPath := path.Join(p, name) + d, err := mns.FindInode(ctx, root, nil, binPath, linux.MaxSymlinkTraversals) + if err == syserror.ENOENT || err == syserror.EACCES { + // Didn't find it here. + continue + } + if err != nil { + return "", err + } + defer d.DecRef() + + // Check whether we can read and execute the found file. + if err := d.Inode.CheckPermission(ctx, PermMask{Read: true, Execute: true}); err != nil { + log.Infof("Found executable at %q, but user cannot execute it: %v", binPath, err) + continue + } + return path.Join("/", p, name), nil + } + return "", syserror.ENOENT +} + +// GetPath returns the PATH as a slice of strings given the environemnt +// variables. +func GetPath(env []string) []string { + const prefix = "PATH=" + for _, e := range env { + if strings.HasPrefix(e, prefix) { + return strings.Split(strings.TrimPrefix(e, prefix), ":") + } + } + return nil +} diff --git a/runsc/boot/controller.go b/runsc/boot/controller.go index 45aa255c4..fd5b7cc9e 100644 --- a/runsc/boot/controller.go +++ b/runsc/boot/controller.go @@ -224,21 +224,39 @@ func (cm *containerManager) Start(args *StartArgs, _ *struct{}) error { return nil } +// ExecArgs contains arguments to Execute. +type ExecArgs struct { + control.ExecArgs + + // CID is the ID of the container to exec in. + CID string +} + // Execute runs a command on a created or running sandbox. -func (cm *containerManager) Execute(e *control.ExecArgs, waitStatus *uint32) error { +func (cm *containerManager) Execute(e *ExecArgs, waitStatus *uint32) error { log.Debugf("containerManager.Execute: %+v", *e) - if e.Filename == "" { - rootCtx := cm.l.rootProcArgs.NewContext(cm.l.k) - rootMns := cm.l.k.RootMountNamespace() - var err error - if e.Filename, err = getExecutablePath(rootCtx, rootMns, e.Argv[0], e.Envv); err != nil { - return fmt.Errorf("error getting executable path for %q: %v", e.Argv[0], err) - } + // Get the container Root Dirent from the Task, since we must run this + // process with the same Root. + cm.l.mu.Lock() + tgid, ok := cm.l.containerRootTGIDs[e.CID] + cm.l.mu.Unlock() + if !ok { + return fmt.Errorf("cannot exec in container %q: no such container", e.CID) + } + t := cm.l.k.TaskSet().Root.TaskWithID(kernel.ThreadID(tgid)) + if t == nil { + return fmt.Errorf("cannot exec in container %q: no thread group with ID %d", e.CID, tgid) + } + t.WithMuLocked(func(t *kernel.Task) { + e.Root = t.FSContext().RootDirectory() + }) + if e.Root != nil { + defer e.Root.DecRef() } proc := control.Proc{Kernel: cm.l.k} - if err := proc.Exec(e, waitStatus); err != nil { + if err := proc.Exec(&e.ExecArgs, waitStatus); err != nil { return fmt.Errorf("error executing: %+v: %v", e, err) } return nil diff --git a/runsc/boot/fs.go b/runsc/boot/fs.go index 3df276170..5ec9a7d03 100644 --- a/runsc/boot/fs.go +++ b/runsc/boot/fs.go @@ -16,7 +16,6 @@ package boot import ( "fmt" - "path" "path/filepath" "strconv" "strings" @@ -683,64 +682,11 @@ func setFileSystemForProcess(procArgs *kernel.CreateProcessArgs, spec *specs.Spe return nil } -// getExecutablePath traverses the *container's* filesystem to resolve exec's -// absolute path. For example, if the container is being served files by the -// fsgofer serving /foo/bar as the container root, it will search within -// /foo/bar, not the host root. -// TODO: Unit test this. -func getExecutablePath(ctx context.Context, mns *fs.MountNamespace, filename string, env []string) (string, error) { - exec := filepath.Clean(filename) - - // Don't search PATH if exec is a path to a file (absolute or relative). - if strings.IndexByte(exec, '/') >= 0 { - return exec, nil - } - - // Search the PATH for a file whose name matches the one we are looking - // for. - pathDirs := specutils.GetPath(env) - for _, p := range pathDirs { - // Try to find the binary inside path p. - binPath := path.Join(p, filename) - root := fs.RootFromContext(ctx) - defer root.DecRef() - d, err := mns.FindInode(ctx, root, nil, binPath, linux.MaxSymlinkTraversals) - if err == syserror.ENOENT || err == syserror.EACCES { - continue - } - if err != nil { - return "", fmt.Errorf("FindInode(%q) failed: %v", binPath, err) - } - defer d.DecRef() - - // Check whether we can read and execute the found file. - if err := d.Inode.CheckPermission(ctx, fs.PermMask{Read: true, Execute: true}); err != nil { - log.Infof("Found executable at %q, but user cannot execute it: %v", binPath, err) - continue - } - return path.Join("/", p, exec), nil - } - - return "", fmt.Errorf("could not find executable %q in path %v", exec, pathDirs) -} - // setExecutablePath sets the procArgs.Filename by searching the PATH for an // executable matching the procArgs.Argv[0]. func setExecutablePath(ctx context.Context, mns *fs.MountNamespace, procArgs *kernel.CreateProcessArgs) error { - if procArgs.Filename != "" { - // Sanity check. - if !path.IsAbs(procArgs.Filename) { - return fmt.Errorf("filename must be absolute: %q", procArgs.Filename) - } - // Nothing to set. - return nil - } - - if len(procArgs.Argv) == 0 { - return fmt.Errorf("Argv must not be empty") - } - - f, err := getExecutablePath(ctx, mns, procArgs.Argv[0], procArgs.Envv) + paths := fs.GetPath(procArgs.Envv) + f, err := mns.ResolveExecutablePath(ctx, procArgs.WorkingDirectory, procArgs.Argv[0], paths) if err != nil { return err } diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index 697210669..f272496a1 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -187,11 +187,16 @@ func (s *Sandbox) Execute(cid string, e *control.ExecArgs) (syscall.WaitStatus, } defer conn.Close() + ea := &boot.ExecArgs{ + ExecArgs: *e, + CID: cid, + } + // Send a message to the sandbox control server to start the container. var waitStatus uint32 // TODO: Pass in the container id (cid) here. The sandbox // should execute in the context of that container. - if err := conn.Call(boot.ContainerExecute, e, &waitStatus); err != nil { + if err := conn.Call(boot.ContainerExecute, ea, &waitStatus); err != nil { return 0, fmt.Errorf("error executing in sandbox: %v", err) } diff --git a/runsc/specutils/specutils.go b/runsc/specutils/specutils.go index f3fa8d129..fdc9007e0 100644 --- a/runsc/specutils/specutils.go +++ b/runsc/specutils/specutils.go @@ -163,18 +163,6 @@ func ReadSpecFromFile(bundleDir string, specFile *os.File) (*specs.Spec, error) return &spec, nil } -// GetPath returns the PATH as a slice of strings given the environemnt -// variables. -func GetPath(env []string) []string { - const prefix = "PATH=" - for _, e := range env { - if strings.HasPrefix(e, prefix) { - return strings.Split(strings.TrimPrefix(e, prefix), ":") - } - } - return nil -} - // Capabilities takes in spec and returns a TaskCapabilities corresponding to // the spec. func Capabilities(specCaps *specs.LinuxCapabilities) (*auth.TaskCapabilities, error) { -- cgit v1.2.3