From 99d14efe62106521e371cbfd752c4778db416d56 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 6 Nov 2023 15:54:37 -0800 Subject: [PATCH] runc delete: refuse to delete container with non-empty cgroup When we delete the container, we no longer have any knowledge about it. In case when the container does not have own private PID namespace, this knowledge (in particular, container's cgroup path) might be crucial, because otherwise we won't be able to kill the leftover container processes. Therefore, let's error out when runc destroy called on a stopped container without private PID namespace and there are processes in its cgroup, suggesting to use runc delete -f or runc kill. Add a test case. Signed-off-by: Kir Kolyshkin --- libcontainer/state_linux.go | 9 +++++++++ tests/integration/delete.bats | 8 +++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/libcontainer/state_linux.go b/libcontainer/state_linux.go index 0f629922b56..8608cf3dc2e 100644 --- a/libcontainer/state_linux.go +++ b/libcontainer/state_linux.go @@ -88,6 +88,15 @@ func (b *stoppedState) transition(s containerState) error { } func (b *stoppedState) destroy() error { + // Normally, when a container init is gone, all other processes in its + // cgroup are killed by the kernel. This is not the case for a shared + // PID namespace container, which may have some leftover processes. + if !b.c.config.Namespaces.IsPrivate(configs.NEWPID) { + // Get container pids on a best-effort basis. + if pids, _ := b.c.cgroupManager.GetAllPids(); len(pids) > 0 { + return fmt.Errorf("container is in stopped state (init is gone), but its cgroup still has %d processes; use runc delete -f or runc kill", len(pids)) + } + } return destroy(b.c) } diff --git a/tests/integration/delete.bats b/tests/integration/delete.bats index 8cc53c19e01..cc209071950 100644 --- a/tests/integration/delete.bats +++ b/tests/integration/delete.bats @@ -62,7 +62,7 @@ function teardown() { [ "$status" -eq 0 ] } -# Issue 4047, case "runc delete -f". +# Issue 4047, cases "runc delete" and "runc delete -f". # See also: "kill KILL [host pidns + init gone]" test in kill.bats. @test "runc delete --force [host pidns + init gone]" { requires cgroups_freezer @@ -113,6 +113,12 @@ function teardown() { kill -0 "$p" done + # runc delete must warn that container cgroup is not empty. + runc delete test_busybox + [ "$status" -ne 0 ] + [[ "$output" == *"container is in stopped state (init is gone), but its cgroup still has 5 processes"* ]] + + # runc delete -f must kill those processes and remove container. runc delete -f test_busybox [ "$status" -eq 0 ]