cgroup: rework which files we chown() on delegation
authorLennart Poettering <lennart@poettering.net>
Fri, 22 Sep 2017 17:58:24 +0000 (19:58 +0200)
committerLennart Poettering <lennart@poettering.net>
Fri, 22 Sep 2017 18:00:53 +0000 (20:00 +0200)
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

index 2c94350..e008d00 100644 (file)
@@ -915,7 +915,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);
@@ -926,6 +926,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;
@@ -938,10 +939,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();