diff --git a/runsc/boot/controller.go b/runsc/boot/controller.go index fdb6be5b1..ec1110059 100644 --- a/runsc/boot/controller.go +++ b/runsc/boot/controller.go @@ -228,6 +228,16 @@ func (cm *containerManager) Start(args *StartArgs, _ *struct{}) error { // Execute runs a command on a created or running sandbox. func (cm *containerManager) Execute(e *control.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) + } + } + proc := control.Proc{Kernel: cm.l.k} if err := proc.Exec(e, waitStatus); err != nil { return fmt.Errorf("error executing: %+v: %v", e, err) diff --git a/runsc/boot/filter/config.go b/runsc/boot/filter/config.go index 113023bdd..f864b1f45 100644 --- a/runsc/boot/filter/config.go +++ b/runsc/boot/filter/config.go @@ -164,10 +164,7 @@ var allowedSyscalls = seccomp.SyscallRules{ seccomp.AllowAny{}, /* winsize struct */ }, }, - syscall.SYS_LSEEK: {}, - // TODO: Remove SYS_LSTAT when executable lookup moves - // into the gofer. - syscall.SYS_LSTAT: {}, + syscall.SYS_LSEEK: {}, syscall.SYS_MADVISE: {}, syscall.SYS_MINCORE: {}, syscall.SYS_MMAP: []seccomp.Rule{ diff --git a/runsc/boot/fs.go b/runsc/boot/fs.go index 20d0e42ef..4a11b30f1 100644 --- a/runsc/boot/fs.go +++ b/runsc/boot/fs.go @@ -701,13 +701,13 @@ func setFileSystemForProcess(procArgs *kernel.CreateProcessArgs, spec *specs.Spe return nil } -// GetExecutablePathInternal 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 +// 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 GetExecutablePathInternal(ctx context.Context, procArgs *kernel.CreateProcessArgs) (string, error) { - exec := filepath.Clean(procArgs.Filename) +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 { @@ -716,31 +716,52 @@ func GetExecutablePathInternal(ctx context.Context, procArgs *kernel.CreateProce // Search the PATH for a file whose name matches the one we are looking // for. - pathDirs := specutils.GetPath(procArgs.Envv) + pathDirs := specutils.GetPath(env) for _, p := range pathDirs { - // Walk to the end of the path. - curDir := procArgs.Root - for _, pc := range strings.Split(p, "/") { - var err error - if curDir, err = curDir.Walk(ctx, curDir, pc); err != nil { - break - } - } - if curDir == nil { + // 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 } - // Check for the executable in the path directory. - dirent, err := curDir.Walk(ctx, curDir, exec) if err != nil { - continue + return "", fmt.Errorf("FindInode(%q) failed: %v", binPath, err) } - // Check whether we can read and execute the file in question. - if err := dirent.Inode.CheckPermission(ctx, fs.PermMask{Read: true, Execute: true}); err != nil { - log.Infof("Found executable at %q, but user cannot execute it: %v", path.Join(p, exec), 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 %s in path %v", exec, pathDirs) + 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) + if err != nil { + return err + } + procArgs.Filename = f + return nil } diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index 74d0c2534..2733c4d69 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -271,16 +271,8 @@ func newProcess(spec *specs.Spec, creds *auth.Credentials, utsns *kernel.UTSName return kernel.CreateProcessArgs{}, fmt.Errorf("error creating limits: %v", err) } - // Get the executable path, which is a bit tricky because we have to - // inspect the environment PATH which is relative to the root path. - exec, err := specutils.GetExecutablePath(spec.Process.Args[0], spec.Root.Path, spec.Process.Env) - if err != nil { - return kernel.CreateProcessArgs{}, fmt.Errorf("error getting executable path: %v", err) - } - // Create the process arguments. procArgs := kernel.CreateProcessArgs{ - Filename: exec, Argv: spec.Process.Args, Envv: spec.Process.Env, WorkingDirectory: spec.Process.Cwd, // Defaults to '/' if empty. @@ -365,7 +357,7 @@ func (l *Loader) run() error { // If we are restoring, we do not want to create a process. // l.restore is set by the container manager when a restore call is made. if !l.restore { - err := setFileSystemForProcess( + if err := setFileSystemForProcess( &l.rootProcArgs, l.spec, l.conf, @@ -374,10 +366,16 @@ func (l *Loader) run() error { l.rootProcArgs.Credentials, l.rootProcArgs.Limits, l.k, - "" /* CID, which isn't needed for the root container */) - if err != nil { + "" /* CID, which isn't needed for the root container */); err != nil { return err } + + rootCtx := l.rootProcArgs.NewContext(l.k) + rootMns := l.k.RootMountNamespace() + if err := setExecutablePath(rootCtx, rootMns, &l.rootProcArgs); err != nil { + return fmt.Errorf("error setting executable path for %+v: %v", l.rootProcArgs, err) + } + // Create the root container init task. if _, err := l.k.CreateProcess(l.rootProcArgs); err != nil { return fmt.Errorf("failed to create init process: %v", err) @@ -443,7 +441,7 @@ func (l *Loader) startContainer(k *kernel.Kernel, spec *specs.Spec, conf *Config ioFDs = append(ioFDs, fd) } - err = setFileSystemForProcess( + if err := setFileSystemForProcess( &procArgs, spec, conf, @@ -452,13 +450,14 @@ func (l *Loader) startContainer(k *kernel.Kernel, spec *specs.Spec, conf *Config creds, procArgs.Limits, k, - cid) - if err != nil { + cid); err != nil { return 0, fmt.Errorf("failed to create new process: %v", err) } - if procArgs.Filename, err = GetExecutablePathInternal(procArgs.NewContext(k), &procArgs); err != nil { - return 0, err + ctx := procArgs.NewContext(l.k) + mns := k.RootMountNamespace() + if err := setExecutablePath(ctx, mns, &procArgs); err != nil { + return 0, fmt.Errorf("error setting executable path for %+v: %v", procArgs, err) } tg, err := l.k.CreateProcess(procArgs) diff --git a/runsc/cmd/exec.go b/runsc/cmd/exec.go index b84a80119..966d2e258 100644 --- a/runsc/cmd/exec.go +++ b/runsc/cmd/exec.go @@ -140,16 +140,6 @@ func (ex *Exec) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) } } - // Get the executable path, which is a bit tricky because we have to - // inspect the environment PATH which is relative to the root path. - // If the user is overriding environment variables, PATH may have been - // overwritten. - rootPath := c.Spec.Root.Path - e.Filename, err = specutils.GetExecutablePath(e.Argv[0], rootPath, e.Envv) - if err != nil { - Fatalf("error getting executable path: %v", err) - } - ws, err := c.Execute(e) if err != nil { Fatalf("error getting processes for container: %v", err) diff --git a/runsc/container/container_test.go b/runsc/container/container_test.go index e7e53c492..5f452acbf 100644 --- a/runsc/container/container_test.go +++ b/runsc/container/container_test.go @@ -521,7 +521,6 @@ func TestExec(t *testing.T) { execArgs := control.ExecArgs{ Filename: "/bin/sleep", Argv: []string{"sleep", "5"}, - Envv: []string{"PATH=" + os.Getenv("PATH")}, WorkingDirectory: "/", KUID: uid, } @@ -889,7 +888,6 @@ func TestPauseResume(t *testing.T) { execArgs := control.ExecArgs{ Filename: "/bin/bash", Argv: []string{"bash", "-c", script}, - Envv: []string{"PATH=" + os.Getenv("PATH")}, WorkingDirectory: "/", KUID: uid, } @@ -1070,7 +1068,6 @@ func TestCapabilities(t *testing.T) { execArgs := control.ExecArgs{ Filename: exePath, Argv: []string{exePath}, - Envv: []string{"PATH=" + os.Getenv("PATH")}, WorkingDirectory: "/", KUID: uid, KGID: gid, diff --git a/runsc/specutils/specutils.go b/runsc/specutils/specutils.go index 3234cc088..551718e9a 100644 --- a/runsc/specutils/specutils.go +++ b/runsc/specutils/specutils.go @@ -163,37 +163,6 @@ func ReadSpecFromFile(bundleDir string, specFile *os.File) (*specs.Spec, error) return &spec, nil } -// GetExecutablePath returns the absolute path to the executable, relative to -// the root. It searches the environment PATH for the first file that exists -// with the given name. -// TODO: Remove this in favor of finding executables via -// boot.GetExecutablePathInternal. -func GetExecutablePath(exec, root string, env []string) (string, error) { - exec = filepath.Clean(exec) - - // 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. - path := GetPath(env) - for _, p := range path { - abs := filepath.Join(root, p, exec) - // Do not follow symlink link because the target is in the container - // root filesystem. - if _, err := os.Lstat(abs); err == nil { - // We found it! Return the path relative to the root. - return filepath.Join("/", p, exec), nil - } - } - - // Could not find a suitable path, just return the original string. - log.Warningf("could not find executable %s in path %s", exec, path) - return exec, nil -} - // GetPath returns the PATH as a slice of strings given the environemnt // variables. func GetPath(env []string) []string {