diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index 611b40ad1..9f0043fb0 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -296,21 +296,29 @@ func (c *linuxContainer) newParentProcess(p *Process, doInit bool) (parentProces if err != nil { return nil, newSystemErrorWithCause(err, "creating new init pipe") } - rootDir, err := os.Open(c.root) - if err != nil { - return nil, err - } - cmd, err := c.commandTemplate(p, childPipe, rootDir) + cmd, err := c.commandTemplate(p, childPipe) if err != nil { return nil, newSystemErrorWithCause(err, "creating new command template") } if !doInit { - return c.newSetnsProcess(p, cmd, parentPipe, childPipe, rootDir) + return c.newSetnsProcess(p, cmd, parentPipe, childPipe) } + + // We only set up rootDir if we're not doing a `runc exec`. The reason for + // this is to avoid cases where a racing, unprivileged process inside the + // container can get access to the statedir file descriptor (which would + // allow for container rootfs escape). + rootDir, err := os.Open(c.root) + if err != nil { + return nil, err + } + cmd.ExtraFiles = append(cmd.ExtraFiles, rootDir) + cmd.Env = append(cmd.Env, + fmt.Sprintf("_LIBCONTAINER_STATEDIR=%d", stdioFdCount+len(cmd.ExtraFiles)-1)) return c.newInitProcess(p, cmd, parentPipe, childPipe, rootDir) } -func (c *linuxContainer) commandTemplate(p *Process, childPipe, rootDir *os.File) (*exec.Cmd, error) { +func (c *linuxContainer) commandTemplate(p *Process, childPipe *os.File) (*exec.Cmd, error) { cmd := exec.Command(c.initArgs[0], c.initArgs[1:]...) cmd.Stdin = p.Stdin cmd.Stdout = p.Stdout @@ -319,10 +327,9 @@ func (c *linuxContainer) commandTemplate(p *Process, childPipe, rootDir *os.File if cmd.SysProcAttr == nil { cmd.SysProcAttr = &syscall.SysProcAttr{} } - cmd.ExtraFiles = append(p.ExtraFiles, childPipe, rootDir) + cmd.ExtraFiles = append(p.ExtraFiles, childPipe) cmd.Env = append(cmd.Env, - fmt.Sprintf("_LIBCONTAINER_INITPIPE=%d", stdioFdCount+len(cmd.ExtraFiles)-2), - fmt.Sprintf("_LIBCONTAINER_STATEDIR=%d", stdioFdCount+len(cmd.ExtraFiles)-1)) + fmt.Sprintf("_LIBCONTAINER_INITPIPE=%d", stdioFdCount+len(cmd.ExtraFiles)-1)) // NOTE: when running a container with no PID namespace and the parent process spawning the container is // PID1 the pdeathsig is being delivered to the container's init process by the kernel for some reason // even with the parent still running. @@ -360,7 +367,7 @@ func (c *linuxContainer) newInitProcess(p *Process, cmd *exec.Cmd, parentPipe, c }, nil } -func (c *linuxContainer) newSetnsProcess(p *Process, cmd *exec.Cmd, parentPipe, childPipe, rootDir *os.File) (*setnsProcess, error) { +func (c *linuxContainer) newSetnsProcess(p *Process, cmd *exec.Cmd, parentPipe, childPipe *os.File) (*setnsProcess, error) { cmd.Env = append(cmd.Env, "_LIBCONTAINER_INITTYPE="+string(initSetns)) state, err := c.currentState() if err != nil { @@ -382,7 +389,6 @@ func (c *linuxContainer) newSetnsProcess(p *Process, cmd *exec.Cmd, parentPipe, config: c.newInitConfig(p), process: p, bootstrapData: data, - rootDir: rootDir, }, nil } diff --git a/libcontainer/factory_linux.go b/libcontainer/factory_linux.go index e4b942adf..b90bc042d 100644 --- a/libcontainer/factory_linux.go +++ b/libcontainer/factory_linux.go @@ -222,29 +222,33 @@ func (l *LinuxFactory) Type() string { // StartInitialization loads a container by opening the pipe fd from the parent to read the configuration and state // This is a low level implementation detail of the reexec and should not be consumed externally func (l *LinuxFactory) StartInitialization() (err error) { - var pipefd, rootfd int - for _, pair := range []struct { - k string - v *int - }{ - {"_LIBCONTAINER_INITPIPE", &pipefd}, - {"_LIBCONTAINER_STATEDIR", &rootfd}, - } { + var ( + pipefd, rootfd int + envInitPipe = os.Getenv("_LIBCONTAINER_INITPIPE") + envStateDir = os.Getenv("_LIBCONTAINER_STATEDIR") + ) - s := os.Getenv(pair.k) - - i, err := strconv.Atoi(s) - if err != nil { - return fmt.Errorf("unable to convert %s=%s to int", pair.k, s) - } - *pair.v = i + // Get the INITPIPE. + pipefd, err = strconv.Atoi(envInitPipe) + if err != nil { + return fmt.Errorf("unable to convert _LIBCONTAINER_INITPIPE=%s to int: %s", envInitPipe, err) } + var ( pipe = os.NewFile(uintptr(pipefd), "pipe") it = initType(os.Getenv("_LIBCONTAINER_INITTYPE")) ) defer pipe.Close() + // Only init processes have STATEDIR. + rootfd = -1 + if it == initStandard { + rootfd, err = strconv.Atoi(envStateDir) + if err != nil { + return fmt.Errorf("unable to convert _LIBCONTAINER_STATEDIR=%s to int: %s", envStateDir, err) + } + } + // clear the current process's environment to clean any libcontainer // specific env vars. os.Clearenv() diff --git a/libcontainer/init_linux.go b/libcontainer/init_linux.go index 9d5f06802..01f32f506 100644 --- a/libcontainer/init_linux.go +++ b/libcontainer/init_linux.go @@ -77,9 +77,8 @@ func newContainerInit(t initType, pipe *os.File, stateDirFD int) (initer, error) switch t { case initSetns: return &linuxSetnsInit{ - pipe: pipe, - config: config, - stateDirFD: stateDirFD, + pipe: pipe, + config: config, }, nil case initStandard: return &linuxStandardInit{ diff --git a/libcontainer/process_linux.go b/libcontainer/process_linux.go index 3ff2000b2..50e90757e 100644 --- a/libcontainer/process_linux.go +++ b/libcontainer/process_linux.go @@ -51,7 +51,6 @@ type setnsProcess struct { fds []string process *Process bootstrapData io.Reader - rootDir *os.File } func (p *setnsProcess) startTime() (string, error) { @@ -70,7 +69,6 @@ func (p *setnsProcess) start() (err error) { defer p.parentPipe.Close() err = p.cmd.Start() p.childPipe.Close() - p.rootDir.Close() if err != nil { return newSystemErrorWithCause(err, "starting setns process") } diff --git a/libcontainer/setns_init_linux.go b/libcontainer/setns_init_linux.go index 372dfd357..2e80450d6 100644 --- a/libcontainer/setns_init_linux.go +++ b/libcontainer/setns_init_linux.go @@ -5,7 +5,6 @@ package libcontainer import ( "fmt" "os" - "syscall" "github.com/opencontainers/runc/libcontainer/apparmor" "github.com/opencontainers/runc/libcontainer/keys" @@ -17,9 +16,8 @@ import ( // linuxSetnsInit performs the container's initialization for running a new process // inside an existing container. type linuxSetnsInit struct { - pipe *os.File - config *initConfig - stateDirFD int + pipe *os.File + config *initConfig } func (l *linuxSetnsInit) getSessionRingName() string { @@ -60,8 +58,5 @@ func (l *linuxSetnsInit) Init() error { if err := label.SetProcessLabel(l.config.ProcessLabel); err != nil { return err } - // close the statedir fd before exec because the kernel resets dumpable in the wrong order - // https://github.com/torvalds/linux/blob/v4.9/fs/exec.c#L1290-L1318 - syscall.Close(l.stateDirFD) return system.Execv(l.config.Args[0], l.config.Args[0:], os.Environ()) }