cgroup: always keep access mode of 'tasks' and 'cgroup.procs' files in cgroup directories in sync

4 files changed, 50 insertions, 26 deletions

diff --git a/src/core/cgroup.c b/src/core/cgroup.c
index 8fb2952c16..b07bd7e6be 100644
--- a/src/core/cgroup.c
+++ b/src/core/cgroup.c
@@ -111,7 +111,7 @@ void cgroup_bonding_trim_list(CGroupBonding *first, bool delete_root) {
 }
 
 int cgroup_bonding_install(CGroupBonding *b, pid_t pid, const char *cgroup_suffix) {
-        char *p = NULL;
+        _cleanup_free_ char *p = NULL;
         const char *path;
         int r;
 
@@ -128,8 +128,6 @@ int cgroup_bonding_install(CGroupBonding *b, pid_t pid, const char *cgroup_suffi
                 path = b->path;
 
         r = cg_create_and_attach(b->controller, path, pid);
-        free(p);
-
         if (r < 0)
                 return r;
 
diff --git a/src/core/execute.c b/src/core/execute.c
index 61369cdc93..dddef14514 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -1045,6 +1045,11 @@ int exec_spawn(ExecCommand *command,
         if (r < 0)
                 return r;
 
+        /* We must initialize the attributes in the parent, before we
+        fork, because we really need them initialized before making
+        the process a member of the group (which we do in both the
+        child and the parent), and we cannot really apply them twice
+        (due to 'append' style attributes) */
         cgroup_attribute_apply_list(cgroup_attributes, cgroup_bondings);
 
         if (context->private_tmp && !context->tmp_dir && !context->var_tmp_dir) {
@@ -1267,7 +1272,12 @@ int exec_spawn(ExecCommand *command,
                         if (cgroup_bondings && context->control_group_modify) {
                                 err = cgroup_bonding_set_group_access_list(cgroup_bondings, 0755, uid, gid);
                                 if (err >= 0)
-                                        err = cgroup_bonding_set_task_access_list(cgroup_bondings, 0644, uid, gid, context->control_group_persistent);
+                                        err = cgroup_bonding_set_task_access_list(
+                                                        cgroup_bondings,
+                                                        0644,
+                                                        uid,
+                                                        gid,
+                                                        context->control_group_persistent);
                                 if (err < 0) {
                                         r = EXIT_CGROUP;
                                         goto fail_child;
@@ -1278,7 +1288,12 @@ int exec_spawn(ExecCommand *command,
                 }
 
                 if (cgroup_bondings && !set_access && context->control_group_persistent >= 0)  {
-                        err = cgroup_bonding_set_task_access_list(cgroup_bondings, (mode_t) -1, (uid_t) -1, (uid_t) -1, context->control_group_persistent);
+                        err = cgroup_bonding_set_task_access_list(
+                                        cgroup_bondings,
+                                        (mode_t) -1,
+                                        (uid_t) -1,
+                                        (uid_t) -1,
+                                        context->control_group_persistent);
                         if (err < 0) {
                                 r = EXIT_CGROUP;
                                 goto fail_child;
diff --git a/src/shared/cgroup-label.c b/src/shared/cgroup-label.c
index beeeec5830..995e4c57cd 100644
--- a/src/shared/cgroup-label.c
+++ b/src/shared/cgroup-label.c
@@ -37,7 +37,7 @@
 #include "mkdir.h"
 
 int cg_create(const char *controller, const char *path) {
-        char *fs;
+        _cleanup_free_ char *fs = NULL;
         int r;
 
         assert(controller);
@@ -48,19 +48,18 @@ int cg_create(const char *controller, const char *path) {
                 return r;
 
         r = mkdir_parents_label(fs, 0755);
+        if (r < 0)
+                return r;
 
-        if (r >= 0) {
-                if (mkdir(fs, 0755) >= 0)
-                        r = 1;
-                else if (errno == EEXIST)
-                        r = 0;
-                else
-                        r = -errno;
-        }
+        if (mkdir(fs, 0755) < 0) {
 
-        free(fs);
+                if (errno == EEXIST)
+                        return 0;
 
-        return r;
+                return -errno;
+        }
+
+        return 1;
 }
 
 int cg_create_and_attach(const char *controller, const char *path, pid_t pid) {
@@ -70,13 +69,14 @@ int cg_create_and_attach(const char *controller, const char *path, pid_t pid) {
         assert(path);
         assert(pid >= 0);
 
-        if ((r = cg_create(controller, path)) < 0)
+        r = cg_create(controller, path);
+        if (r < 0)
                 return r;
 
-        if ((q = cg_attach(controller, path, pid)) < 0)
+        q = cg_attach(controller, path, pid);
+        if (q < 0)
                 return q;
 
         /* This does not remove the cgroup on failure */
-
         return r;
 }
diff --git a/src/shared/cgroup-util.c b/src/shared/cgroup-util.c
index 15e1b7c055..3738ca8eac 100644
--- a/src/shared/cgroup-util.c
+++ b/src/shared/cgroup-util.c
@@ -714,8 +714,15 @@ int cg_set_group_access(const char *controller, const char *path, mode_t mode, u
         return chmod_and_chown(fs, mode, uid, gid);
 }
 
-int cg_set_task_access(const char *controller, const char *path, mode_t mode, uid_t uid, gid_t gid, int sticky) {
-        char *fs;
+int cg_set_task_access(
+                const char *controller,
+                const char *path,
+                mode_t mode,
+                uid_t uid,
+                gid_t gid,
+                int sticky) {
+
+        _cleanup_free_ char *fs = NULL, *procs = NULL;
         int r;
 
         assert(controller);
@@ -742,10 +749,8 @@ int cg_set_task_access(const char *controller, const char *path, mode_t mode, ui
                  * mode from the file itself */
 
                 r = lstat(fs, &st);
-                if (r < 0) {
-                        free(fs);
+                if (r < 0)
                         return -errno;
-                }
 
                 if (mode == (mode_t) -1)
                         /* No mode set, we just shall set the sticky bit */
@@ -756,9 +761,15 @@ int cg_set_task_access(const char *controller, const char *path, mode_t mode, ui
         }
 
         r = chmod_and_chown(fs, mode, uid, gid);
-        free(fs);
+        if (r < 0)
+                return r;
 
-        return r;
+        /* Always keep values for "cgroup.procs" in sync with "tasks" */
+        r = cg_get_path(controller, path, "cgroup.procs", &procs);
+        if (r < 0)
+                return r;
+
+        return chmod_and_chown(procs, mode, uid, gid);
 }
 
 int cg_get_by_pid(const char *controller, pid_t pid, char **path) {