Skip to content

Commit

Permalink
runc create/run: warn on cgroup v1 + shared pidns + rootless
Browse files Browse the repository at this point in the history
Shared pid namespace means `runc kill` (or `runc delete -f`) have to
kill all container process, not just init. To do that, it needs access
to container's cgroup, which is impossible for rootless cgroup v1
containers.

Therefore, such configuration is bad and should not be allowed. To keep
backward compatibility, though, let's merely warn about this for now.

Signed-off-by: Kir Kolyshkin <[email protected]>
  • Loading branch information
kolyshkin committed Sep 12, 2024
1 parent cf5fa19 commit 0980635
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 0 deletions.
11 changes: 11 additions & 0 deletions libcontainer/configs/validate/rootless.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,14 @@ func rootlessEUIDMount(config *configs.Config) error {

return nil
}

func rootlessSharedPidns(config *configs.Config) error {
if !config.RootlessCgroups {
return nil
}
if config.Namespaces.IsPrivate(configs.NEWPID) {
return nil
}
return errors.New("runc won't be able to kill all processes in a rootless container with no cgroup access and shared pidns; " +
"such configuration is strongly discouraged and will result in an error in a future runc version")
}
1 change: 1 addition & 0 deletions libcontainer/configs/validate/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ func Validate(config *configs.Config) error {
// Relaxed validation rules for backward compatibility
warns := []check{
mountsWarn,
rootlessSharedPidns, // TODO: make it an error in runc 1.3.
}
for _, c := range warns {
if err := c(config); err != nil {
Expand Down
23 changes: 23 additions & 0 deletions tests/integration/create.bats
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,26 @@ function teardown() {

testcontainer test_busybox running
}

# https://github.com/opencontainers/runc/issues/4394#issuecomment-2334926257
@test "runc create [shared pidns + rootless]" {
update_config ' .linux.namespaces -= [{"type": "pid"}]'
if [ $EUID -ne 0 ]; then
# Can't mount real /proc when rootless + no pidns,
# so change it to a bind-mounted one from the host.
update_config ' .mounts |= map((select(.type == "proc")
| .type = "none"
| .source = "/proc"
| .options = ["rbind", "nosuid", "nodev", "noexec"]
) // .)'
fi

exp="won't be able to kill all processes in a rootless container with no cgroup access"
runc create --console-socket "$CONSOLE_SOCKET" test
[ "$status" -eq 0 ]
if [ -v CGROUP_V1 ] && [ $EUID -ne 0 ]; then
[[ "$output" = *"$exp"* ]]
else
[[ "$output" != *"$exp"* ]]
fi
}

0 comments on commit 0980635

Please sign in to comment.