Fix indefinite parent hang when child dies mid-handshake

libcontainer/process_linux.go

@@ -507,7 +507,7 @@ func (p *setnsProcess) start() (retErr error) {
 	}
 
 	var seenProcReady bool
-	ierr := parseSync(p.comm.syncSockParent, func(sync *syncT) error {
+	ierr := parseSync(p.comm.syncSockParent, p.pid(), func(sync *syncT) error {
 		switch sync.Type {
 		case procReady:
 			seenProcReady = true
@@ -904,7 +904,7 @@ func (p *initProcess) start() (retErr error) {
 	}
 
 	var seenProcReady bool
-	ierr := parseSync(p.comm.syncSockParent, func(sync *syncT) error {
+	ierr := parseSync(p.comm.syncSockParent, p.pid(), func(sync *syncT) error {
 		switch sync.Type {
 		case procMountPlease:
 			if mountRequest == nil {

libcontainer/sync.go

@@ -186,8 +186,26 @@ func readSync(pipe *syncSocket, expected syncType) error {
 
 // parseSync runs the given callback function on each syncT received from the
 // child. It will return once io.EOF is returned from the given pipe.
-func parseSync(pipe *syncSocket, fn func(*syncT) error) error {
+//
+// pid is the pid of the child process on the other end of pipe. Before each
+// read, parseSync waits until the pipe is actually ready or the child has
+// exited (see waitForSyncReady), so a child that dies mid-handshake (e.g.
+// killed by seccomp) without cleanly closing its socket can't leave us
+// blocked forever.
+func parseSync(pipe *syncSocket, pid int, fn func(*syncT) error) error {
 	for {
+		ready, err := waitForSyncReady(pipe.File(), pid)
+		if err != nil {
+			return fmt.Errorf("waiting for sync socket: %w", err)
+		}
+		if !ready {
+			// A successful run always ends via the child voluntarily
+			// closing its own end of the socket (a real io.EOF below), so
+			// this always means the child died mid-protocol -- regardless
+			// of which sync stages we'd already seen -- and must be
+			// reported as an error rather than treated as a clean exit.
+			return fmt.Errorf("sync socket: process %d exited before completing sync handshake", pid)
+		}
 		sync, err := doReadSync(pipe)
 		if err != nil {
 			if errors.Is(err, io.EOF) {

libcontainer/sync_linux.go

@@ -0,0 +1,42 @@
+package libcontainer
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"time"
+
+	"golang.org/x/sys/unix"
+
+	"github.com/opencontainers/runc/libcontainer/system"
+)
+
+// waitForSyncReady blocks until either the sync socket is ready to read
+// (data available, or cleanly closed) or the given process has exited.
+// ready=false means the process is gone without its socket becoming ready;
+// the caller should treat that the same as io.EOF rather than attempting a
+// read, since a stray reference to the child's end of the socket elsewhere
+// can otherwise leave a plain read blocked indefinitely.
+func waitForSyncReady(f *os.File, pid int) (ready bool, err error) {
+	pfd := []unix.PollFd{{Fd: int32(f.Fd()), Events: unix.POLLIN}}
+	const pollIntervalMs = 100
+	for {
+		n, err := unix.Poll(pfd, pollIntervalMs)
+		if errors.Is(err, unix.EINTR) {
+			// Avoid a tight spin if signals are arriving rapidly (e.g. Go's
+			// runtime async-preemption SIGURG).
+			time.Sleep(time.Millisecond)
+			continue
+		}
+		if err != nil {
+			return false, fmt.Errorf("poll sync socket: %w", err)
+		}
+		if n > 0 && pfd[0].Revents&(unix.POLLIN|unix.POLLHUP) != 0 {
+			return true, nil
+		}
+		stat, err := system.Stat(pid)
+		if err != nil || stat.State == system.Zombie {
+			return false, nil
+		}
+	}
+}

libcontainer/sync_linux_test.go

@@ -0,0 +1,71 @@
+package libcontainer
+
+import (
+	"os"
+	"os/exec"
+	"testing"
+	"time"
+
+	"golang.org/x/sys/unix"
+)
+
+// TestParseSyncDeadChildDoesNotHang reproduces opencontainers/runc#5087:
+// parseSync must not block forever if the child dies mid-handshake (e.g.
+// killed by seccomp) while its end of the socket is still held open
+// elsewhere.
+func TestParseSyncDeadChildDoesNotHang(t *testing.T) {
+	// Real SOCK_SEQPACKET pair, same as runc's actual sync socket.
+	fds, err := unix.Socketpair(unix.AF_LOCAL, unix.SOCK_SEQPACKET|unix.SOCK_CLOEXEC, 0)
+	if err != nil {
+		t.Fatalf("socketpair: %v", err)
+	}
+	// Keep a duplicate of the child's end open for the test's lifetime, so
+	// the kernel never delivers EOF on fds[0] no matter what happens to
+	// the child process below (simulating a leaked reference elsewhere).
+	leaked, err := unix.Dup(fds[1])
+	if err != nil {
+		t.Fatalf("dup: %v", err)
+	}
+	defer unix.Close(leaked)
+	if err := unix.Close(fds[1]); err != nil {
+		t.Fatalf("close: %v", err)
+	}
+
+	parentFile := os.NewFile(uintptr(fds[0]), "sync-p")
+	defer parentFile.Close()
+	pipe := newSyncSocket(parentFile)
+
+	// A real child process we can kill, standing in for the container init
+	// that gets killed by seccomp mid-handshake.
+	cmd := exec.Command("sleep", "100")
+	if err := cmd.Start(); err != nil {
+		t.Fatalf("start child: %v", err)
+	}
+	defer func() {
+		_ = cmd.Process.Kill()
+		_ = cmd.Wait()
+	}()
+
+	go func() {
+		time.Sleep(300 * time.Millisecond)
+		_ = cmd.Process.Kill()
+		_ = cmd.Wait()
+	}()
+
+	done := make(chan error, 1)
+	go func() {
+		done <- parseSync(pipe, cmd.Process.Pid, func(sync *syncT) error {
+			return nil
+		})
+	}()
+
+	select {
+	case err := <-done:
+		if err == nil {
+			t.Fatal("expected parseSync to return an error when the child dies mid-handshake, got nil")
+		}
+		t.Logf("parseSync correctly reported the dead child: %v", err)
+	case <-time.After(5 * time.Second):
+		t.Fatal("parseSync hung indefinitely after child died with a leaked socket reference (runc#5087)")
+	}
+}