From 11fa6038b3afbcc9f93adc328e46f4505537c22f Mon Sep 17 00:00:00 2001 From: Nebojsa Jacovic Date: Mon, 10 Aug 2026 09:46:18 +0200 Subject: [PATCH] libcontainer: fix hang on dead child in sync If the container's init process dies mid-handshake (e.g. killed by a seccomp filter) while some other reference to its end of the sync socket is still held open elsewhere, the parent can block forever in recvfrom(), since the kernel never delivers EOF on a socket with a live reference. Add waitForSyncReady, which polls the sync socket alongside the child's own /proc state before each read in parseSync, so the parent can detect the child's death directly instead of relying on the socket's own EOF behavior. If the child is confirmed dead without its socket ever becoming ready, parseSync now returns a clear error instead of hanging or silently reporting success. Fixes #5087 Signed-off-by: Nebojsa Jacovic --- libcontainer/process_linux.go | 4 +- libcontainer/sync.go | 20 +++++++++- libcontainer/sync_linux.go | 42 +++++++++++++++++++ libcontainer/sync_linux_test.go | 71 +++++++++++++++++++++++++++++++++ 4 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 libcontainer/sync_linux.go create mode 100644 libcontainer/sync_linux_test.go diff --git a/libcontainer/process_linux.go b/libcontainer/process_linux.go index 62c89102d1b..2db7975d6db 100644 --- a/libcontainer/process_linux.go +++ b/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 { diff --git a/libcontainer/sync.go b/libcontainer/sync.go index 07befdbbc6a..a7f3a70041e 100644 --- a/libcontainer/sync.go +++ b/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) { diff --git a/libcontainer/sync_linux.go b/libcontainer/sync_linux.go new file mode 100644 index 00000000000..5a6278d301f --- /dev/null +++ b/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 + } + } +} diff --git a/libcontainer/sync_linux_test.go b/libcontainer/sync_linux_test.go new file mode 100644 index 00000000000..50e59db7d00 --- /dev/null +++ b/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)") + } +}