From 2dab75076e87a3b2c19008023c3d7b3fde1e59fe Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Wed, 22 Apr 2020 00:19:36 +0900 Subject: [PATCH] fs2: fix cgroup.subtree_control EPERM on rootless + add CI Fix #2339 (regression in 813cb3eb94659971c69c0dd19b2d5a9737891f15) Signed-off-by: Akihiro Suda --- .travis.yml | 2 ++ Vagrantfile | 2 ++ libcontainer/cgroups/fs2/create.go | 13 ++++++++++--- libcontainer/cgroups/fs2/fs2.go | 30 +++++++++++++++++++++++++----- libcontainer/cgroups/systemd/v2.go | 2 +- tests/integration/cgroups.bats | 2 +- tests/rootless.sh | 8 ++++++++ 7 files changed, 49 insertions(+), 10 deletions(-) diff --git a/.travis.yml b/.travis.yml index 3c2e39b751b..4df240df638 100644 --- a/.travis.yml +++ b/.travis.yml @@ -38,6 +38,8 @@ matrix: - sudo ssh default -t 'cd /vagrant && sudo make localintegration RUNC_USE_SYSTEMD=yes' # same setup but with fs2 driver instead of systemd - sudo ssh default -t 'cd /vagrant && sudo make localintegration' + # rootless + - sudo ssh default -t 'cd /vagrant && sudo make localrootlessintegration' allow_failures: - go: tip diff --git a/Vagrantfile b/Vagrantfile index 444d077077e..22d06091112 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -19,5 +19,7 @@ install podman make golang-go libseccomp-devel bats jq ts run EOF dnf clean all + # Add a user for rootless tests + useradd -u2000 -m -d/home/rootless -s/bin/bash rootless SHELL end diff --git a/libcontainer/cgroups/fs2/create.go b/libcontainer/cgroups/fs2/create.go index 0cc2c1f61f2..d2fa8cb15b2 100644 --- a/libcontainer/cgroups/fs2/create.go +++ b/libcontainer/cgroups/fs2/create.go @@ -65,7 +65,7 @@ func neededControllers(cgroup *configs.Cgroup) ([]string, error) { // CreateCgroupPath creates cgroupv2 path, enabling all the // needed controllers in the process. -func CreateCgroupPath(path string, c *configs.Cgroup) (Err error) { +func CreateCgroupPath(path string, c *configs.Cgroup, rootless bool) (Err error) { if !strings.HasPrefix(path, UnifiedMountpoint) { return fmt.Errorf("invalid cgroup path %s", path) } @@ -79,6 +79,7 @@ func CreateCgroupPath(path string, c *configs.Cgroup) (Err error) { elements := strings.Split(path, "/") elements = elements[3:] current := "/sys/fs" +elementLoop: for i, e := range elements { current = filepath.Join(current, e) if i > 0 { @@ -100,8 +101,14 @@ func CreateCgroupPath(path string, c *configs.Cgroup) (Err error) { if i < len(elements)-1 { file := filepath.Join(current, "cgroup.subtree_control") if err := ioutil.WriteFile(file, []byte(allCtrs), 0755); err != nil { - // XXX: we can enable _some_ controllers doing it one-by one - // instead of erroring out -- does it makes sense to do so? + if rootless { + // try write one by one + for _, ctr := range ctrs { + _ = ioutil.WriteFile(file, []byte(ctr), 0755) + } + // ignore unwritable files + continue elementLoop + } return err } } diff --git a/libcontainer/cgroups/fs2/fs2.go b/libcontainer/cgroups/fs2/fs2.go index 93a2aa59d1a..6e402611241 100644 --- a/libcontainer/cgroups/fs2/fs2.go +++ b/libcontainer/cgroups/fs2/fs2.go @@ -53,7 +53,10 @@ func (m *manager) getControllers() error { file := filepath.Join(m.dirPath, "cgroup.controllers") data, err := ioutil.ReadFile(file) - if err != nil && !m.rootless { + if err != nil { + if m.rootless && m.config.Path == "" { + return nil + } return err } fields := strings.Fields(string(data)) @@ -66,10 +69,23 @@ func (m *manager) getControllers() error { } func (m *manager) Apply(pid int) error { - if err := CreateCgroupPath(m.dirPath, m.config); err != nil { + if err := CreateCgroupPath(m.dirPath, m.config, m.rootless); err != nil { + // Related tests: + // - "runc create (no limits + no cgrouppath + no permission) succeeds" + // - "runc create (rootless + no limits + cgrouppath + no permission) fails with permission error" + // - "runc create (rootless + limits + no cgrouppath + no permission) fails with informative error" + if m.rootless { + if m.config.Path == "" { + cl, clErr := neededControllers(m.config) + if clErr == nil && len(cl) == 0 { + return nil + } + return errors.Wrap(err, "rootless needs no limits + no cgrouppath when no permission is granted for cgroups") + } + } return err } - if err := cgroups.WriteCgroupProc(m.dirPath, pid); err != nil && !m.rootless { + if err := cgroups.WriteCgroupProc(m.dirPath, pid); err != nil { return err } return nil @@ -199,7 +215,11 @@ func (m *manager) Set(container *configs.Config) error { } } // devices (since kernel 4.15, pseudo-controller) - if err := setDevices(m.dirPath, container.Cgroups); err != nil { + // + // When m.Rootless is true, errors from the device subsystem are ignored because it is really not expected to work. + // However, errors from other subsystems are not ignored. + // see @test "runc create (rootless + limits + no cgrouppath + no permission) fails with informative error" + if err := setDevices(m.dirPath, container.Cgroups); err != nil && !m.rootless { errs = append(errs, err) } // cpuset (since kernel 5.0) @@ -218,7 +238,7 @@ func (m *manager) Set(container *configs.Config) error { if err := setFreezer(m.dirPath, container.Cgroups.Freezer); err != nil { errs = append(errs, err) } - if len(errs) > 0 && !m.rootless { + if len(errs) > 0 { return errors.Errorf("error while setting cgroup v2: %+v", errs) } m.config = container.Cgroups diff --git a/libcontainer/cgroups/systemd/v2.go b/libcontainer/cgroups/systemd/v2.go index 130fefb645e..1572b39dadf 100644 --- a/libcontainer/cgroups/systemd/v2.go +++ b/libcontainer/cgroups/systemd/v2.go @@ -151,7 +151,7 @@ func (m *unifiedManager) Apply(pid int) error { if err != nil { return err } - if err := fs2.CreateCgroupPath(m.path, m.cgroups); err != nil { + if err := fs2.CreateCgroupPath(m.path, m.cgroups, m.rootless); err != nil { return err } return nil diff --git a/tests/integration/cgroups.bats b/tests/integration/cgroups.bats index 6abb2ed6b02..31a82336c61 100644 --- a/tests/integration/cgroups.bats +++ b/tests/integration/cgroups.bats @@ -95,7 +95,7 @@ EOF runc run -d --console-socket $CONSOLE_SOCKET test_cgroups_permissions [ "$status" -eq 1 ] - [[ ${lines[1]} == *"cannot set pids limit: container could not join or create cgroup"* ]] + [[ ${lines[1]} == *"rootless needs no limits + no cgrouppath when no permission is granted for cgroups"* ]] || [[ ${lines[1]} == *"cannot set pids limit: container could not join or create cgroup"* ]] } @test "runc create (limits + cgrouppath + permission on the cgroup dir) succeeds" { diff --git a/tests/rootless.sh b/tests/rootless.sh index d6092d042cb..ce2f193073f 100755 --- a/tests/rootless.sh +++ b/tests/rootless.sh @@ -82,6 +82,14 @@ function enable_cgroup() { # handling. [[ "$cg" == "cpuset" ]] && chown rootless:rootless "$CGROUP_MOUNT/$cg$CGROUP_PATH/cpuset."{cpus,mems} done + # cgroup v2 + # https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html#delegation-containment + if [[ -e "$CGROUP_MOUNT/cgroup.controllers" ]]; then + for f in $(cat cgroup.controllers); do echo +$f > "$CGROUP_MOUNT/cgroup.controllers"; done + mkdir -p "$CGROUP_MOUNT/$CGROUP_PATH" + chown root:rootless "$CGROUP_MOUNT/$CGROUP_PATH" "$CGROUP_MOUNT/$CGROUP_PATH/cgroup.procs" "$CGROUP_MOUNT/cgroup.procs" + chmod g+rwx "$CGROUP_MOUNT/$CGROUP_PATH" "$CGROUP_MOUNT/$CGROUP_PATH/cgroup.procs" "$CGROUP_MOUNT/cgroup.procs" + fi } function disable_cgroup() {