cgroup: do not allow manipulating the cgroup path of units within the systemd:/system...

diff --git a/TODO b/TODO
index bb1802813fc93a2cc15a9ea88bdd34c28f6bbd32..1a35703d1a3342ad348da256e5e61c05868682dd 100644 (file)
--- a/TODO
+++ b/TODO
@@ -26,6 +26,10 @@ Fedora 19:
 
 Features:
 
 
 Features:
 

+* handle named vs controller hierarchies correctly in cg_pid_get_path()
+
+* add nspawn@.service
+

 * investigate endianess issues of UUID vs. GUID
 
 * see if we can fix https://bugs.freedesktop.org/show_bug.cgi?id=63672
 * investigate endianess issues of UUID vs. GUID
 
 * see if we can fix https://bugs.freedesktop.org/show_bug.cgi?id=63672


@@ -57,9 +61,6 @@ Features:
 
 * make sure cg_pid_get_path() works properly for co-mounted controllers
 
 
 * make sure cg_pid_get_path() works properly for co-mounted controllers
 

-* explicitly disallow changing the cgroup path of units in the
-  name=systemd hierarchy, unless it is outside of /system
-

 * test/:
   - add 'set -e' to scripts in test/
   - make stuff in test/ work with separate output dir
 * test/:
   - add 'set -e' to scripts in test/
   - make stuff in test/ work with separate output dir

diff --git a/src/core/unit.c b/src/core/unit.c
index 282852fed33d02257be8fe95dc1e8417e6f6db0a..c0f156c928e5526fa62823d38a98c6d2e9fdbd64 100644 (file)
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -1977,10 +1977,16 @@ static int unit_add_cgroup(Unit *u, CGroupBonding *b) {
 }
 
 char *unit_default_cgroup_path(Unit *u) {
 }
 
 char *unit_default_cgroup_path(Unit *u) {

+        _cleanup_free_ char *escaped_instance = NULL;
+

         assert(u);
 
         assert(u);
 

+        escaped_instance = cg_escape(u->id);
+        if (!escaped_instance)
+                return NULL;
+

         if (u->instance) {
         if (u->instance) {

-                _cleanup_free_ char *t = NULL, *escaped_template = NULL, *escaped_instance = NULL;
+                _cleanup_free_ char *t = NULL, *escaped_template = NULL;

 
                 t = unit_name_template(u->id);
                 if (!t)
 
                 t = unit_name_template(u->id);
                 if (!t)


@@ -1990,20 +1996,9 @@ char *unit_default_cgroup_path(Unit *u) {
                 if (!escaped_template)
                         return NULL;
 
                 if (!escaped_template)
                         return NULL;
 

-                escaped_instance = cg_escape(u->id);
-                if (!escaped_instance)
-                        return NULL;
-

                 return strjoin(u->manager->cgroup_hierarchy, "/", escaped_template, "/", escaped_instance, NULL);
                 return strjoin(u->manager->cgroup_hierarchy, "/", escaped_template, "/", escaped_instance, NULL);

-        } else {
-                _cleanup_free_ char *escaped = NULL;
-
-                escaped = cg_escape(u->id);
-                if (!escaped)
-                        return NULL;
-
-                return strjoin(u->manager->cgroup_hierarchy, "/", escaped, NULL);
-        }
+        } else
+                return strjoin(u->manager->cgroup_hierarchy, "/", escaped_instance, NULL);

 }
 
 int unit_add_cgroup_from_text(Unit *u, const char *name, bool overwrite, CGroupBonding **ret) {
 }
 
 int unit_add_cgroup_from_text(Unit *u, const char *name, bool overwrite, CGroupBonding **ret) {


@@ -2025,7 +2020,7 @@ int unit_add_cgroup_from_text(Unit *u, const char *name, bool overwrite, CGroupB
         }
 
         if (!controller) {
         }
 
         if (!controller) {

-                controller = strdup(SYSTEMD_CGROUP_CONTROLLER);
+                controller = strdup("systemd");

                 ours = true;
         }
 
                 ours = true;
         }
 


@@ -2035,6 +2030,16 @@ int unit_add_cgroup_from_text(Unit *u, const char *name, bool overwrite, CGroupB
                 return log_oom();
         }
 
                 return log_oom();
         }
 

+        if (streq(controller, "systemd")) {
+                /* Within the systemd unit hierarchy we do not allow changes. */
+                if (path_startswith(path, "/system")) {
+                        log_warning_unit(u->id, "Manipulating the systemd:/system cgroup hierarchy is not permitted.");
+                        free(path);
+                        free(controller);
+                        return -EPERM;
+                }
+        }
+

         b = cgroup_bonding_find_list(u->cgroup_bondings, controller);
         if (b) {
                 if (streq(path, b->path)) {
         b = cgroup_bonding_find_list(u->cgroup_bondings, controller);
         if (b) {
                 if (streq(path, b->path)) {

diff --git a/src/shared/cgroup-util.c b/src/shared/cgroup-util.c
index 46a8128eb4030d8373da43a54b2b419b1c7a9e8c..016080f65b902b49eae26e2195ff5a8f7d77a159 100644 (file)
--- a/src/shared/cgroup-util.c
+++ b/src/shared/cgroup-util.c
@@ -916,6 +916,7 @@ int cg_is_empty_recursive(const char *controller, const char *path, bool ignore_
 int cg_split_spec(const char *spec, char **controller, char **path) {
         const char *e;
         char *t = NULL, *u = NULL;
 int cg_split_spec(const char *spec, char **controller, char **path) {
         const char *e;
         char *t = NULL, *u = NULL;

+        _cleanup_free_ char *v = NULL;

 
         assert(spec);
 
 
         assert(spec);
 


@@ -928,6 +929,7 @@ int cg_split_spec(const char *spec, char **controller, char **path) {
                         if (!t)
                                 return -ENOMEM;
 
                         if (!t)
                                 return -ENOMEM;
 

+                        path_kill_slashes(t);

                         *path = t;
                 }
 
                         *path = t;
                 }
 


@@ -943,7 +945,7 @@ int cg_split_spec(const char *spec, char **controller, char **path) {
                         return -EINVAL;
 
                 if (controller) {
                         return -EINVAL;
 
                 if (controller) {

-                        t = strdup(spec);
+                        t = strdup(normalize_controller(spec));

                         if (!t)
                                 return -ENOMEM;
 
                         if (!t)
                                 return -ENOMEM;
 


@@ -956,7 +958,10 @@ int cg_split_spec(const char *spec, char **controller, char **path) {
                 return 0;
         }
 
                 return 0;
         }
 

-        t = strndup(spec, e-spec);
+        v = strndup(spec, e-spec);
+        if (!v)
+                return -ENOMEM;
+        t = strdup(normalize_controller(v));

         if (!t)
                 return -ENOMEM;
         if (!cg_controller_is_valid(t, true)) {
         if (!t)
                 return -ENOMEM;
         if (!cg_controller_is_valid(t, true)) {


@@ -969,12 +974,15 @@ int cg_split_spec(const char *spec, char **controller, char **path) {
                 free(t);
                 return -ENOMEM;
         }
                 free(t);
                 return -ENOMEM;
         }

-        if (!path_is_safe(u)) {
+        if (!path_is_safe(u) ||
+            !path_is_absolute(u)) {

                 free(t);
                 free(u);
                 return -EINVAL;
         }
 
                 free(t);
                 free(u);
                 return -EINVAL;
         }
 

+        path_kill_slashes(u);
+

         if (controller)
                 *controller = t;
         else
         if (controller)
                 *controller = t;
         else


@@ -993,7 +1001,6 @@ int cg_join_spec(const char *controller, const char *path, char **spec) {
 
         assert(path);
 
 
         assert(path);
 

-

         if (!controller)
                 controller = "systemd";
         else {
         if (!controller)
                 controller = "systemd";
         else {


@@ -1010,6 +1017,8 @@ int cg_join_spec(const char *controller, const char *path, char **spec) {
         if (!s)
                 return -ENOMEM;
 
         if (!s)
                 return -ENOMEM;
 

+        path_kill_slashes(s + strlen(controller) + 1);
+

         *spec = s;
         return 0;
 }
         *spec = s;
         return 0;
 }


@@ -1029,6 +1038,7 @@ int cg_mangle_path(const char *path, char **result) {
                 if (!t)
                         return -ENOMEM;
 
                 if (!t)
                         return -ENOMEM;
 

+                path_kill_slashes(t);

                 *result = t;
                 return 0;
         }
                 *result = t;
                 return 0;
         }

diff --git a/src/shared/cgroup-util.h b/src/shared/cgroup-util.h
index a2ee72d67ffa48201e56b2c7ce6bb0db625f720a..7bd02c10089f4e7784af203fa8c9c8f48b25f729 100644 (file)
--- a/src/shared/cgroup-util.h
+++ b/src/shared/cgroup-util.h
@@ -28,6 +28,21 @@
 #include "set.h"
 #include "def.h"
 
 #include "set.h"
 #include "def.h"
 

+/*
+ * General rules:
+ *
+ * We accept named hierarchies in the syntax "foo" and "name=foo".
+ *
+ * We expect that named hierarchies do not conflict in name with a
+ * kernel hierarchy, modulo the "name=" prefix.
+ *
+ * We always generate "normalized" controller names, i.e. without the
+ * "name=" prefix.
+ *
+ * We require absolute cgroup paths. When returning, we will always
+ * generate paths with multiple adjacent / removed.
+ */
+

 int cg_enumerate_processes(const char *controller, const char *path, FILE **_f);
 int cg_enumerate_tasks(const char *controller, const char *path, FILE **_f);
 int cg_read_pid(FILE *f, pid_t *_pid);
 int cg_enumerate_processes(const char *controller, const char *path, FILE **_f);
 int cg_enumerate_tasks(const char *controller, const char *path, FILE **_f);
 int cg_read_pid(FILE *f, pid_t *_pid);