Repository: opencontainers/runc
File: libcontainer/container_linux.go (Go)

Code before:
	}

	if safeExe != nil {
		// Due to a Go stdlib bug, we need to add safeExe to the set of
		// ExtraFiles otherwise it is possible for the stdlib to clobber the fd
		// during forkAndExecInChild1 and replace it with some other file that
		// might be malicious. This is less than ideal (because the descriptor
		// will be non-O_CLOEXEC) however we have protections in "runc init" to
		// stop us from leaking extra file descriptors.
		//
		// See <https://github.com/golang/go/issues/61751>.
		cmd.ExtraFiles = append(cmd.ExtraFiles, safeExe)
	}

	// 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.
	if c.config.ParentDeathSignal > 0 {
		cmd.SysProcAttr.Pdeathsig = unix.Signal(c.config.ParentDeathSignal)
	}

	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 {
			return nil, fmt.Errorf("unable to setup exec fifo: %w", err)
		}
		return c.newInitProcess(p, cmd, comm)
	}
	return c.newSetnsProcess(p, cmd, comm)
}

func (c *Container) newInitProcess(p *Process, cmd *exec.Cmd, comm *processComm) (*initProcess, error) {
	cmd.Env = append(cmd.Env, "_LIBCONTAINER_INITTYPE="+string(initStandard))
	nsMaps := make(map[configs.NamespaceType]string)
	for _, ns := range c.config.Namespaces {
		if ns.Path != "" {
			nsMaps[ns.Type] = ns.Path
		}
	}
	data, err := c.bootstrapData(c.config.Namespaces.CloneFlags(), nsMaps)
	if err != nil {
		return nil, err
	}

	init := &initProcess{
		cmd:             cmd,

Review comment (bug):
Super nit: I think it would be slightly more clear if we add a comment here: https://github.com/opencontainers/runc/blob/main/libcontainer/container_linux.go#L563 saying we will override it later.

Also, IMHO having this in a function and just calling it from https://github.com/opencontainers/runc/blob/main/libcontainer/container_linux.go#L563 seems slightly better. But I don't know if it feels safer, due to this nasty bug, to do it way later when we won't have a small fd number (to be extra cautious).

In any case, this is very subjective and if the code as is feels better, I'm completely fine. And even if we want to add it, this can be a follow-up PR.

Code after:
	}

	// TODO: After https://go-review.googlesource.com/c/go/+/515799 included
	// in go versions supported by us, we can remove this logic.
	if safeExe != nil {
		// Due to a Go stdlib bug, we need to add safeExe to the set of
		// ExtraFiles otherwise it is possible for the stdlib to clobber the fd
		// during forkAndExecInChild1 and replace it with some other file that
		// might be malicious. This is less than ideal (because the descriptor
		// will be non-O_CLOEXEC) however we have protections in "runc init" to
		// stop us from leaking extra file descriptors.
		//
		// See <https://github.com/golang/go/issues/61751>.
		cmd.ExtraFiles = append(cmd.ExtraFiles, safeExe)

		// There is a race situation when we are opening a file, if there is a
		// small fd was closed at that time, maybe it will be reused by safeExe.
		// Because of Go stdlib fds shuffling bug, if the fd of safeExe is too
		// small, go stdlib will dup3 it to another fd, or dup3 a other fd to this
		// fd, then it will cause the fd type cmd.Path refers to a random path,
		// and it can lead to an error "permission denied" when starting the process.
		// Please see #4294.
		// So we should not use the original fd of safeExe, but use the fd after
		// shuffled by Go stdlib. Because Go stdlib will guarantee this fd refers to
		// the correct file.
		cmd.Path = "/proc/self/fd/" + strconv.Itoa(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.
	if c.config.ParentDeathSignal > 0 {
		cmd.SysProcAttr.Pdeathsig = unix.Signal(c.config.ParentDeathSignal)
	}

	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 {
			return nil, fmt.Errorf("unable to setup exec fifo: %w", err)
		}
		return c.newInitProcess(p, cmd, comm)
	}
	return c.newSetnsProcess(p, cmd, comm)
}

func (c *Container) newInitProcess(p *Process, cmd *exec.Cmd, comm *processComm) (*initProcess, error) {