From dda0f77dcedceece0d67d24eb37bbcdca897ee6d Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Fri, 22 Sep 2017 19:58:24 +0200
Subject: [PATCH] cgroup: rework which files we chown() on delegation

On cgroupsv2 we should also chown()/chmod() the subtree_control file,
so that children can use controllers the way they like.

On cgroupsv1 we should also chown()/chmod() cgroups.clone_children, as
not setting this for new cgroups makes little sense, and hence delegated
clients should be able to write to it.

Note that error handling for both cases is different. subtree_control
matters so we check for errors, but the clone_children/tasks stuff
doesn't really, as it's legacy stuff. Hence we only log errors and
proceed.

Fixes: #6216
---
 src/basic/cgroup-util.c | 38 +++++++++++++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c
index 5fc59d2ef..7d64c6a2b 100644
--- a/src/basic/cgroup-util.c
+++ b/src/basic/cgroup-util.c
@@ -922,7 +922,7 @@ int cg_set_task_access(
                 uid_t uid,
                 gid_t gid) {
 
-        _cleanup_free_ char *fs = NULL, *procs = NULL;
+        _cleanup_free_ char *fs = NULL;
         int r;
 
         assert(path);
@@ -933,6 +933,7 @@ int cg_set_task_access(
         if (mode != MODE_INVALID)
                 mode &= 0666;
 
+        /* For both the legacy and unified hierarchies, "cgroup.procs" is the main entry point for PIDs */
         r = cg_get_path(controller, path, "cgroup.procs", &fs);
         if (r < 0)
                 return r;
@@ -945,10 +946,37 @@ int cg_set_task_access(
         if (r < 0)
                 return r;
         if (r == 0) {
-                /* Compatibility, Always keep values for "tasks" in sync with
-                 * "cgroup.procs" */
-                if (cg_get_path(controller, path, "tasks", &procs) >= 0)
-                        (void) chmod_and_chown(procs, mode, uid, gid);
+                const char *fn;
+
+                /* Compatibility: on cgroupsv1 always keep values for the legacy files "tasks" and
+                 * "cgroup.clone_children" in sync with "cgroup.procs". Since this is legacy stuff, we don't care if
+                 * this fails. */
+
+                FOREACH_STRING(fn,
+                               "tasks",
+                               "cgroup.clone_children") {
+
+                        fs = mfree(fs);
+
+                        r = cg_get_path(controller, path, fn, &fs);
+                        if (r < 0)
+                                log_debug_errno(r, "Failed to get path for %s of %s, ignoring: %m", fn, path);
+
+                        r = chmod_and_chown(fs, mode, uid, gid);
+                        if (r < 0)
+                                log_debug_errno(r, "Failed to to change ownership/access mode for %s of %s, ignoring: %m", fn, path);
+                }
+        } else {
+                /* On the unified controller, we want to permit subtree controllers too. */
+
+                fs = mfree(fs);
+                r = cg_get_path(controller, path, "cgroup.subtree_control", &fs);
+                if (r < 0)
+                        return r;
+
+                r = chmod_and_chown(fs, mode, uid, gid);
+                if (r < 0)
+                        return r;
         }
 
         r = cg_hybrid_unified();
-- 
2.30.2