From 0aa0fae3932c497f3bfa2b3001d97930d930547b Mon Sep 17 00:00:00 2001 From: Chaitanya Bandi Date: Mon, 5 Oct 2020 14:31:53 -0700 Subject: [PATCH] Kill all processes in cgroup even if init process Wait fails If the cgroup's init process doesn't complete successfully, Wait returns a non-nil error. We should still kill all the process in the cgroup if process namespace is shared. Otherwise, it may result in process leak. Fixes #2632 Signed-off-by: Chaitanya Bandi --- libcontainer/integration/exec_test.go | 54 +++++++++++++++++++++++++++ libcontainer/process_linux.go | 5 +-- 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/libcontainer/integration/exec_test.go b/libcontainer/integration/exec_test.go index 9c8652336..855699540 100644 --- a/libcontainer/integration/exec_test.go +++ b/libcontainer/integration/exec_test.go @@ -11,6 +11,7 @@ import ( "reflect" "strconv" "strings" + "syscall" "testing" "github.com/opencontainers/runc/libcontainer" @@ -1660,6 +1661,59 @@ func TestPIDHost(t *testing.T) { } } +func TestPIDHostInitProcessWait(t *testing.T) { + if testing.Short() { + return + } + + rootfs, err := newRootfs() + ok(t, err) + defer remove(rootfs) + + pidns := "/proc/1/ns/pid" + + // Run a container with two long-running processes. + config := newTemplateConfig(rootfs) + config.Namespaces.Add(configs.NEWPID, pidns) + container, err := newContainerWithName("test", config) + ok(t, err) + defer func() { + _ = container.Destroy() + }() + + process1 := &libcontainer.Process{ + Cwd: "/", + Args: []string{"sleep", "100"}, + Env: standardEnvironment, + Init: true, + } + err = container.Run(process1) + ok(t, err) + + process2 := &libcontainer.Process{ + Cwd: "/", + Args: []string{"sleep", "100"}, + Env: standardEnvironment, + Init: false, + } + err = container.Run(process2) + ok(t, err) + + // Kill the init process and Wait for it. + err = process1.Signal(syscall.SIGKILL) + ok(t, err) + _, err = process1.Wait() + if err == nil { + t.Fatal("expected Wait to indicate failure") + } + + // The non-init process must've been killed. + err = process2.Signal(syscall.Signal(0)) + if err == nil || err.Error() != "no such process" { + t.Fatalf("expected process to have been killed: %v", err) + } +} + func TestInitJoinPID(t *testing.T) { if testing.Short() { return diff --git a/libcontainer/process_linux.go b/libcontainer/process_linux.go index 1402b516d..834268b99 100644 --- a/libcontainer/process_linux.go +++ b/libcontainer/process_linux.go @@ -511,14 +511,11 @@ func (p *initProcess) start() (retErr error) { func (p *initProcess) wait() (*os.ProcessState, error) { err := p.cmd.Wait() - if err != nil { - return p.cmd.ProcessState, err - } // we should kill all processes in cgroup when init is died if we use host PID namespace if p.sharePidns { signalAllProcesses(p.manager, unix.SIGKILL) } - return p.cmd.ProcessState, nil + return p.cmd.ProcessState, err } func (p *initProcess) terminate() error {