libct: setns recreate cmd object after the first call

Signed-off-by: Efim Verzakov <efimverzakov@gmail.com>

Author: everzakov

libcontainer/container_linux.go

@@ -479,52 +479,30 @@ func (c *Container) deleteExecFifo() {
 // container cannot access the statedir (and the FIFO itself remains
 // un-opened). It then adds the FifoFd to the given exec.Cmd as an inherited
 // fd, with _LIBCONTAINER_FIFOFD set to its fd number.
-func (c *Container) includeExecFifo(cmd *exec.Cmd) error {
+func (c *Container) includeExecFifo() error {
 	fifoName := filepath.Join(c.stateDir, execFifoFilename)
 	fifo, err := os.OpenFile(fifoName, unix.O_PATH|unix.O_CLOEXEC, 0)
 	if err != nil {
 		return err
 	}
 	c.fifo = fifo
 
-	cmd.ExtraFiles = append(cmd.ExtraFiles, fifo)
-	cmd.Env = append(cmd.Env,
-		"_LIBCONTAINER_FIFOFD="+strconv.Itoa(stdioFdCount+len(cmd.ExtraFiles)-1))
 	return nil
 }
 
-func (c *Container) newParentProcess(p *Process) (parentProcess, error) {
-	comm, err := newProcessComm()
-	if err != nil {
-		return nil, err
-	}
-
-	// Make sure we use a new safe copy of /proc/self/exe binary each time, this
-	// is called to make sure that if a container manages to overwrite the file,
-	// it cannot affect other containers on the system. For runc, this code will
-	// only ever be called once, but libcontainer users might call this more than
-	// once.
-	p.closeClonedExes()
+// After https://go-review.googlesource.com/c/go/+/728642 will be merged, we need to recreate
+// cmd Object to retry it.
+func (c *Container) createCmdObject(p *Process, comm *processComm, cgroupFD *os.File) *exec.Cmd {
 	var (
 		exePath string
 		safeExe *os.File
 	)
-	if exeseal.IsSelfExeCloned() {
-		// /proc/self/exe is already a cloned binary -- no need to do anything
-		logrus.Debug("skipping binary cloning -- /proc/self/exe is already cloned!")
-		// We don't need to use /proc/thread-self here because the exe mm of a
-		// thread-group is guaranteed to be the same for all threads by
-		// definition. This lets us avoid having to do runtime.LockOSThread.
-		exePath = "/proc/self/exe"
-	} else {
-		var err error
-		safeExe, err = exeseal.CloneSelfExe(c.stateDir)
-		if err != nil {
-			return nil, fmt.Errorf("unable to create safe /proc/self/exe clone for runc init: %w", err)
-		}
+
+	if len(p.clonedExes) > 0 {
+		safeExe = p.clonedExes[0]
 		exePath = "/proc/self/fd/" + strconv.Itoa(int(safeExe.Fd()))
-		p.clonedExes = append(p.clonedExes, safeExe)
-		logrus.Debug("runc exeseal: using /proc/self/exe clone") // used for tests
+	} else {
+		exePath = "/proc/self/exe"
 	}
 
 	cmd := exec.Command(exePath, "init")
@@ -602,22 +580,72 @@ func (c *Container) newParentProcess(p *Process) (parentProcess, error) {
 		cmd.SysProcAttr.Pdeathsig = unix.Signal(c.config.ParentDeathSignal)
 	}
 
+	if c.fifo != nil {
+		cmd.ExtraFiles = append(cmd.ExtraFiles, c.fifo)
+		cmd.Env = append(cmd.Env,
+			"_LIBCONTAINER_FIFOFD="+strconv.Itoa(stdioFdCount+len(cmd.ExtraFiles)-1))
+	}
+
+	if p.Init {
+		cmd.Env = append(cmd.Env, "_LIBCONTAINER_INITTYPE="+string(initStandard))
+	} else {
+		cmd.Env = append(cmd.Env, "_LIBCONTAINER_INITTYPE="+string(initSetns))
+	}
+
+	if cgroupFD != nil {
+		cmd.SysProcAttr.UseCgroupFD = true
+		cmd.SysProcAttr.CgroupFD = int(cgroupFD.Fd())
+	}
+
+	return cmd
+}
+
+func (c *Container) newParentProcess(p *Process) (parentProcess, error) {
+	comm, err := newProcessComm()
+	if err != nil {
+		return nil, err
+	}
+
+	// Make sure we use a new safe copy of /proc/self/exe binary each time, this
+	// is called to make sure that if a container manages to overwrite the file,
+	// it cannot affect other containers on the system. For runc, this code will
+	// only ever be called once, but libcontainer users might call this more than
+	// once.
+	p.closeClonedExes()
+	var (
+		safeExe *os.File
+	)
+	if exeseal.IsSelfExeCloned() {
+		// /proc/self/exe is already a cloned binary -- no need to do anything
+		logrus.Debug("skipping binary cloning -- /proc/self/exe is already cloned!")
+		// We don't need to use /proc/thread-self here because the exe mm of a
+		// thread-group is guaranteed to be the same for all threads by
+		// definition. This lets us avoid having to do runtime.LockOSThread.
+	} else {
+		var err error
+		safeExe, err = exeseal.CloneSelfExe(c.stateDir)
+		if err != nil {
+			return nil, fmt.Errorf("unable to create safe /proc/self/exe clone for runc init: %w", err)
+		}
+		p.clonedExes = append(p.clonedExes, safeExe)
+		logrus.Debug("runc exeseal: using /proc/self/exe clone") // used for tests
+	}
+
 	if p.Init {
 		// We only set up fifoFd if we're not doing a `runc exec`. The historic
 		// reason for this is that previously we would pass a dirfd that allowed
 		// for container rootfs escape (and not doing it in `runc exec` avoided
 		// that problem), but we no longer do that. However, there's no need to do
 		// this for `runc exec` so we just keep it this way to be safe.
-		if err := c.includeExecFifo(cmd); err != nil {
+		if err := c.includeExecFifo(); err != nil {
 			return nil, fmt.Errorf("unable to setup exec fifo: %w", err)
 		}
-		return c.newInitProcess(p, cmd, comm)
+		return c.newInitProcess(p, comm)
 	}
-	return c.newSetnsProcess(p, cmd, comm)
+	return c.newSetnsProcess(p, comm)
 }
 
-func (c *Container) newInitProcess(p *Process, cmd *exec.Cmd, comm *processComm) (*initProcess, error) {
-	cmd.Env = append(cmd.Env, "_LIBCONTAINER_INITTYPE="+string(initStandard))
+func (c *Container) newInitProcess(p *Process, comm *processComm) (*initProcess, error) {
 	nsMaps := make(map[configs.NamespaceType]string)
 	for _, ns := range c.config.Namespaces {
 		if ns.Path != "" {
@@ -629,6 +657,8 @@ func (c *Container) newInitProcess(p *Process, cmd *exec.Cmd, comm *processComm)
 		return nil, err
 	}
 
+	cmd := c.createCmdObject(p, comm, nil)
+
 	init := &initProcess{
 		containerProcess: containerProcess{
 			cmd:           cmd,
@@ -645,8 +675,7 @@ func (c *Container) newInitProcess(p *Process, cmd *exec.Cmd, comm *processComm)
 	return init, nil
 }
 
-func (c *Container) newSetnsProcess(p *Process, cmd *exec.Cmd, comm *processComm) (*setnsProcess, error) {
-	cmd.Env = append(cmd.Env, "_LIBCONTAINER_INITTYPE="+string(initSetns))
+func (c *Container) newSetnsProcess(p *Process, comm *processComm) (*setnsProcess, error) {
 	state := c.currentState()
 	// for setns process, we don't have to set cloneflags as the process namespaces
 	// will only be set via setns syscall
@@ -656,7 +685,6 @@ func (c *Container) newSetnsProcess(p *Process, cmd *exec.Cmd, comm *processComm
 	}
 	proc := &setnsProcess{
 		containerProcess: containerProcess{
-			cmd:           cmd,
 			comm:          comm,
 			manager:       c.cgroupManager,
 			config:        c.newInitConfig(p),

libcontainer/process_linux.go

@@ -15,7 +15,6 @@ import (
 	"runtime"
 	"strconv"
 	"strings"
-	"syscall"
 	"time"
 
 	"github.com/opencontainers/runtime-spec/specs-go"
@@ -357,12 +356,6 @@ func (p *setnsProcess) prepareCgroupFD() (*os.File, error) {
 	}
 
 	logrus.Debugf("using CLONE_INTO_CGROUP %q", cgroup)
-	if p.cmd.SysProcAttr == nil {
-		p.cmd.SysProcAttr = &syscall.SysProcAttr{}
-	}
-	p.cmd.SysProcAttr.UseCgroupFD = true
-	p.cmd.SysProcAttr.CgroupFD = int(fd.Fd())
-
 	return fd, nil
 }
 
@@ -380,11 +373,12 @@ func (p *setnsProcess) startWithCgroupFD() error {
 		defer fd.Close()
 	}
 
+	p.cmd = p.containerProcess.container.createCmdObject(p.process, p.comm, fd)
 	err = p.startWithCPUAffinity()
 	if err != nil && p.cmd.SysProcAttr.UseCgroupFD {
+		// We need to recreate the exec.Cmd object
 		logrus.Debugf("exec with CLONE_INTO_CGROUP failed: %v; retrying without", err)
-		// SysProcAttr.CgroupFD is never used when UseCgroupFD is unset.
-		p.cmd.SysProcAttr.UseCgroupFD = false
+		p.cmd = p.containerProcess.container.createCmdObject(p.process, p.comm, nil)
 		err = p.startWithCPUAffinity()
 	}