exec: add optional --exec-wait-fifo

When you run exec with --detach you can already get the new process's
pidfd and pid before it runs, but nothing actually holds the process
there. The program can execve, and possibly exit, before whatever is
supervising it has finished adopting it. I didn't want to build out a
whole create/start style lifecycle for exec just to get that, when all
it really needs is a single wait point right before execve.

So this adds an opt-in --exec-wait-fifo <path>. The caller creates and
owns the fifo. runc passes it into the setns process, which does all of
its normal setup and then, just before execve, opens the fifo for
writing. That open blocks until something opens the read end, so a
supervisor can finish its handoff and then open the fifo to let the
program run. We write a byte and exec.

It's the same handshake create/start already use with the internal
exec.fifo, so this reuses that path (awaitExecFifo) instead of adding
another one. The O_PATH fd is closed before execve so the CVE-2016-9962
workaround still holds on old kernels. Nothing changes when the flag
isn't set.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

Author: cpuguy83

CHANGELOG.md

@@ -6,6 +6,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added ###
+- `runc exec` now accepts an optional `--exec-wait-fifo <path>` flag. When set,
+  the exec process opens the caller-owned FIFO for writing just before `execve`.
+  Opening a FIFO for writing blocks until a reader is present, so the process
+  pauses there until an external supervisor opens the read end. This lets the
+  supervisor register the detached exec process (e.g. one identified via
+  `--pidfd-socket`) before its program runs. (#5373)
+
 ### Fixed ###
 - The poststart hooks are now executed after starting the user-specified
   process, fixing a runtime-spec conformance issue. (#4347, #5186)

exec.go

@@ -44,6 +44,10 @@ following will output a list of processes running in the container:
 			Name:  "pidfd-socket",
 			Usage: "path to an AF_UNIX socket which will receive a file descriptor referencing the exec process",
 		},
+		&cli.StringFlag{
+			Name:  "exec-wait-fifo",
+			Usage: "path to a caller-owned FIFO; the exec process blocks opening it for writing just before execve, until an external reader opens the read end",
+		},
 		&cli.StringFlag{
 			Name:  "cwd",
 			Usage: "current working directory in the container",
@@ -196,6 +200,7 @@ func execProcess(cmd *cli.Command) (int, error) {
 		container:       container,
 		consoleSocket:   cmd.String("console-socket"),
 		pidfdSocket:     cmd.String("pidfd-socket"),
+		execWaitFifo:    cmd.String("exec-wait-fifo"),
 		detach:          cmd.Bool("detach"),
 		pidFile:         cmd.String("pid-file"),
 		action:          CT_ACT_RUN,

libcontainer/container_linux.go

@@ -619,6 +619,13 @@ func (c *Container) newParentProcess(p *Process) (parentProcess, error) {
 		)
 	}
 
+	if p.ExecWaitFifo != nil {
+		cmd.ExtraFiles = append(cmd.ExtraFiles, p.ExecWaitFifo)
+		cmd.Env = append(cmd.Env,
+			"_LIBCONTAINER_EXECWAITFD="+strconv.Itoa(stdioFdCount+len(cmd.ExtraFiles)-1),
+		)
+	}
+
 	// 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 {

libcontainer/init_linux.go

@@ -215,6 +215,19 @@ func startInitialization() (retErr error) {
 		defer pidfdSocket.Close()
 	}
 
+	// Only setns ("runc exec") processes may be given an exec-wait FIFO.
+	var execWaitFifo *os.File
+	if it == initSetns {
+		if envFd := os.Getenv("_LIBCONTAINER_EXECWAITFD"); envFd != "" {
+			fd, err := strconv.Atoi(envFd)
+			if err != nil {
+				return fmt.Errorf("unable to convert _LIBCONTAINER_EXECWAITFD: %w", err)
+			}
+			execWaitFifo = os.NewFile(uintptr(fd), "exec-wait-fifo")
+			defer execWaitFifo.Close()
+		}
+	}
+
 	// From here on, we don't need current process environment. It is not
 	// used directly anywhere below this point, but let's clear it anyway.
 	os.Clearenv()
@@ -235,10 +248,10 @@ func startInitialization() (retErr error) {
 	}
 
 	// If init succeeds, it will not return, hence none of the defers will be called.
-	return containerInit(it, &config, syncPipe, consoleSocket, pidfdSocket, fifoFile, logPipe)
+	return containerInit(it, &config, syncPipe, consoleSocket, pidfdSocket, fifoFile, logPipe, execWaitFifo)
 }
 
-func containerInit(t initType, config *initConfig, pipe *syncSocket, consoleSocket, pidfdSocket, fifoFile, logPipe *os.File) error {
+func containerInit(t initType, config *initConfig, pipe *syncSocket, consoleSocket, pidfdSocket, fifoFile, logPipe, execWaitFifo *os.File) error {
 	// Clean the RLIMIT_NOFILE cache in go runtime.
 	// Issue: https://github.com/opencontainers/runc/issues/4195
 	maybeClearRlimitNofileCache(config.Rlimits)
@@ -251,6 +264,7 @@ func containerInit(t initType, config *initConfig, pipe *syncSocket, consoleSock
 			pidfdSocket:   pidfdSocket,
 			config:        config,
 			logPipe:       logPipe,
+			execWaitFifo:  execWaitFifo,
 		}
 		return i.Init()
 	case initStandard:
@@ -735,3 +749,25 @@ func setupPidfd(socket *os.File, initType string) error {
 	}
 	return unix.Close(pidFd)
 }
+
+// awaitExecFifo blocks until a reader opens the other end of the exec FIFO
+// referenced by the given O_PATH fd, then writes a single byte to signal that
+// the process is about to execve. Both the reopened write end and the original
+// O_PATH fd are closed before returning; closing the O_PATH fd before execve
+// avoids re-triggering CVE-2016-9962 on older kernels.
+func awaitExecFifo(fifo *os.File) error {
+	// The fd we were handed is an O_PATH fd to the FIFO, which cannot be used
+	// for I/O. Re-open it for writing through /proc/self/fd; this blocks until
+	// an external reader opens the read end.
+	w, err := pathrs.Reopen(fifo, unix.O_WRONLY|unix.O_CLOEXEC)
+	if err != nil {
+		return fmt.Errorf("reopen exec fifo: %w", err)
+	}
+	defer w.Close()
+	if _, err := w.Write([]byte("0")); err != nil {
+		return &os.PathError{Op: "write exec fifo", Path: w.Name(), Err: err}
+	}
+	_ = w.Close()
+	_ = fifo.Close()
+	return nil
+}

libcontainer/process.go

@@ -93,6 +93,14 @@ type Process struct {
 	// PidfdSocket provides process file descriptor of it own.
 	PidfdSocket *os.File
 
+	// ExecWaitFifo is an optional FIFO that a setns ("runc exec") process
+	// opens for writing immediately before execve, blocking until an external
+	// reader opens the read end. This lets a supervisor register the process
+	// (e.g. via PidfdSocket) before its requested program starts. The caller
+	// owns creation and cleanup of the FIFO. It is only honored for exec
+	// (non-Init) processes.
+	ExecWaitFifo *os.File
+
 	// Init specifies whether the process is the first process in the container.
 	Init bool
 

libcontainer/setns_init_linux.go

@@ -26,6 +26,7 @@ type linuxSetnsInit struct {
 	pidfdSocket   *os.File
 	config        *initConfig
 	logPipe       *os.File
+	execWaitFifo  *os.File
 }
 
 func (l *linuxSetnsInit) getSessionRingName() string {
@@ -151,6 +152,16 @@ func (l *linuxSetnsInit) Init() error {
 		return fmt.Errorf("close log pipe: %w", err)
 	}
 
+	// If an exec-wait FIFO was provided, block until an external reader opens
+	// it before running the user process. This gives a supervisor a chance to
+	// register the (already-created) process before its program starts. See
+	// the --exec-wait-fifo flag of "runc exec".
+	if l.execWaitFifo != nil {
+		if err := awaitExecFifo(l.execWaitFifo); err != nil {
+			return err
+		}
+	}
+
 	// Close all file descriptors we are not passing to the container. This is
 	// necessary because the execve target could use internal runc fds as the
 	// execve path, potentially giving access to binary files from the host

libcontainer/standard_init_linux.go

@@ -12,7 +12,6 @@ import (
 	"golang.org/x/sys/unix"
 
 	"github.com/opencontainers/runc/internal/linux"
-	"github.com/opencontainers/runc/internal/pathrs"
 	"github.com/opencontainers/runc/internal/sys"
 	"github.com/opencontainers/runc/libcontainer/apparmor"
 	"github.com/opencontainers/runc/libcontainer/configs"
@@ -260,27 +259,12 @@ func (l *linuxStandardInit) Init() error {
 	}
 
 	// Wait for the FIFO to be opened on the other side before exec-ing the
-	// user process. We open it through /proc/self/fd/$fd, because the fd that
-	// was given to us was an O_PATH fd to the fifo itself. Linux allows us to
-	// re-open an O_PATH fd through /proc.
-	fifoFile, err := pathrs.Reopen(l.fifoFile, unix.O_WRONLY|unix.O_CLOEXEC)
-	if err != nil {
-		return fmt.Errorf("reopen exec fifo: %w", err)
-	}
-	defer fifoFile.Close()
-	if _, err := fifoFile.Write([]byte("0")); err != nil {
-		return &os.PathError{Op: "write exec fifo", Path: fifoFile.Name(), Err: err}
+	// user process. The fd we were given is an O_PATH fd to the fifo itself,
+	// which awaitExecFifo re-opens through /proc to signal readiness.
+	if err := awaitExecFifo(l.fifoFile); err != nil {
+		return err
 	}
 
-	// Close the O_PATH fifofd fd before exec because the kernel resets
-	// dumpable in the wrong order. This has been fixed in newer kernels, but
-	// we keep this to ensure CVE-2016-9962 doesn't re-emerge on older kernels.
-	// N.B. the core issue itself (passing dirfds to the host filesystem) has
-	// since been resolved.
-	// https://github.com/torvalds/linux/blob/v4.9/fs/exec.c#L1290-L1318
-	_ = fifoFile.Close()
-	_ = l.fifoFile.Close()
-
 	if s := l.config.SpecState; s != nil {
 		s.Pid = unix.Getpid()
 		s.Status = specs.StateCreated

man/runc-exec.8.md

@@ -42,6 +42,16 @@ specification as defined by the
 **--pid-file** _path_
 : Specify the file to write the container process' PID to.
 
+**--exec-wait-fifo** _path_
+: Path to a caller-created and caller-owned FIFO. When set, the exec process
+completes its normal setup but blocks opening the FIFO for writing, just before
+the final **execve**(2), until an external reader opens the read end. It then
+writes one byte and proceeds to execute the requested program. This is a single
+opt-in synchronization point — analogous to the FIFO handshake that gates the
+start of a **runc-create**(8)d container — that lets a supervisor register a detached
+exec process — for example one identified via **--pidfd-socket** — before its
+program runs. Intended for use with **--detach**.
+
 **--process-label** _label_
 : Set the asm process label for the process commonly used with **selinux**(7).
 

tests/integration/exec-wait-fifo.bats

@@ -0,0 +1,61 @@
+#!/usr/bin/env bats
+
+load helpers
+
+function setup() {
+	setup_busybox
+}
+
+function teardown() {
+	teardown_bundle
+}
+
+@test "runc exec [ --exec-wait-fifo ] does not run the program until the read end is opened" {
+	update_config '.root.readonly = false'
+	runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox
+	[ "$status" -eq 0 ]
+	testcontainer test_busybox running
+
+	local fifo="$PWD/exec-wait.fifo"
+	mkfifo "$fifo"
+
+	# Use __runc, not the run() helper: the exec process blocks before execve
+	# while holding the inherited stdio, so the run() helper would wait forever
+	# for those fds to reach EOF. The detached parent still writes the pid file
+	# and exits.
+	__runc exec -d --pid-file exec.pid --exec-wait-fifo "$fifo" test_busybox \
+		sh -c "echo ran > /exec-wait.marker" </dev/null
+	[ -f exec.pid ]
+
+	# The program has not run: the exec process is blocked before execve.
+	runc exec test_busybox test -e /exec-wait.marker
+	[ "$status" -ne 0 ]
+
+	# Opening the read end unblocks the process, which then runs its program.
+	timeout 10 cat "$fifo" >/dev/null
+
+	retry 10 1 __runc exec test_busybox test -e /exec-wait.marker
+}
+
+@test "runc exec [ --exec-wait-fifo ] rejects a path that is not a fifo" {
+	runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox
+	[ "$status" -eq 0 ]
+	testcontainer test_busybox running
+
+	local not_fifo="$PWD/not-a-fifo"
+	: >"$not_fifo"
+
+	runc exec --exec-wait-fifo "$not_fifo" test_busybox true
+	[ "$status" -ne 0 ]
+	[[ "$output" == *"is not a fifo"* ]]
+}
+
+@test "runc exec [ --exec-wait-fifo ] fails for a missing fifo path" {
+	runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox
+	[ "$status" -eq 0 ]
+	testcontainer test_busybox running
+
+	runc exec --exec-wait-fifo "$PWD/does-not-exist.fifo" test_busybox true
+	[ "$status" -ne 0 ]
+	[[ "$output" == *"open exec wait fifo"* ]]
+}

utils_linux.go

@@ -213,6 +213,7 @@ type runner struct {
 	pidFile         string
 	consoleSocket   string
 	pidfdSocket     string
+	execWaitFifo    string
 	container       *libcontainer.Container
 	action          CtAct
 	notifySocket    *notifySocket
@@ -285,6 +286,14 @@ func (r *runner) run(config *specs.Process) (_ int, retErr error) {
 		defer connClose()
 	}
 
+	if r.execWaitFifo != "" {
+		fifoClose, err := setupExecWaitFifo(process, r.execWaitFifo)
+		if err != nil {
+			return -1, err
+		}
+		defer fifoClose()
+	}
+
 	switch r.action {
 	case CT_ACT_CREATE:
 		err = r.container.Start(process)
@@ -458,6 +467,31 @@ func setupPidfdSocket(process *libcontainer.Process, sockpath string) (_clean fu
 	}, nil
 }
 
+// setupExecWaitFifo opens the caller-provided FIFO as an O_PATH fd and attaches
+// it to the process. The setns init process re-opens it for writing right
+// before execve, blocking until an external reader opens the read end. The fd
+// is O_PATH so opening it here neither blocks nor counts as a writer.
+func setupExecWaitFifo(process *libcontainer.Process, path string) (_clean func(), _ error) {
+	fifo, err := os.OpenFile(path, unix.O_PATH|unix.O_CLOEXEC, 0)
+	if err != nil {
+		return nil, fmt.Errorf("failed to open exec wait fifo: %w", err)
+	}
+	fi, err := fifo.Stat()
+	if err != nil {
+		fifo.Close()
+		return nil, err
+	}
+	if fi.Mode()&os.ModeNamedPipe == 0 {
+		fifo.Close()
+		return nil, fmt.Errorf("exec wait fifo %q is not a fifo", path)
+	}
+
+	process.ExecWaitFifo = fifo
+	return func() {
+		fifo.Close()
+	}, nil
+}
+
 func maybeLogCgroupWarning(op string, err error) {
 	if errors.Is(err, fs.ErrPermission) {
 		logrus.Warn("runc " + op + " failure might be caused by lack of full access to cgroups")