cgroup: always validate cgroup controller names

Let's better be safe than sorry.

2 files changed, 14 insertions, 13 deletions

diff --git a/src/core/unit.c b/src/core/unit.c
index 4b9abf32dc..c0c3ce90a7 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -1938,7 +1938,7 @@ char *unit_dbus_path(Unit *u) {
         return unit_dbus_path_from_name(u->id);
 }
 
-int unit_add_cgroup(Unit *u, CGroupBonding *b) {
+static int unit_add_cgroup(Unit *u, CGroupBonding *b) {
         int r;
 
         assert(u);
@@ -2100,6 +2100,9 @@ static int unit_add_one_default_cgroup(Unit *u, const char *controller) {
 
         assert(u);
 
+        if (controller && !cg_controller_is_valid(controller, true))
+                return -EINVAL;
+
         if (!controller)
                 controller = SYSTEMD_CGROUP_CONTROLLER;
 
@@ -2202,13 +2205,15 @@ int unit_add_cgroup_attribute(
                 controller = c;
         }
 
-        if (!controller || streq(controller, SYSTEMD_CGROUP_CONTROLLER))
+        if (!controller ||
+            streq(controller, SYSTEMD_CGROUP_CONTROLLER) ||
+            streq(controller, "systemd"))
                 return -EINVAL;
 
         if (!filename_is_safe(name))
                 return -EINVAL;
 
-        if (!filename_is_safe(controller))
+        if (!cg_controller_is_valid(controller, false))
                 return -EINVAL;
 
         /* Check if this attribute already exists. Note that we will
@@ -2276,42 +2281,39 @@ int unit_add_cgroup_attribute(
 }
 
 int unit_load_related_unit(Unit *u, const char *type, Unit **_found) {
-        char *t;
+        _cleanup_free_ char *t = NULL;
         int r;
 
         assert(u);
         assert(type);
         assert(_found);
 
-        if (!(t = unit_name_change_suffix(u->id, type)))
+        t = unit_name_change_suffix(u->id, type);
+        if (!t)
                 return -ENOMEM;
 
         assert(!unit_has_name(u, t));
 
         r = manager_load_unit(u->manager, t, NULL, NULL, _found);
-        free(t);
-
         assert(r < 0 || *_found != u);
-
         return r;
 }
 
 int unit_get_related_unit(Unit *u, const char *type, Unit **_found) {
+        _cleanup_free_ char *t = NULL;
         Unit *found;
-        char *t;
 
         assert(u);
         assert(type);
         assert(_found);
 
-        if (!(t = unit_name_change_suffix(u->id, type)))
+        t = unit_name_change_suffix(u->id, type);
+        if (!t)
                 return -ENOMEM;
 
         assert(!unit_has_name(u, t));
 
         found = manager_get_unit(u->manager, t);
-        free(t);
-
         if (!found)
                 return -ENOENT;
 
diff --git a/src/core/unit.h b/src/core/unit.h
index 51a8364d63..6bfe58c8bf 100644
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -450,7 +450,6 @@ int unit_add_two_dependencies_by_name_inverse(Unit *u, UnitDependency d, UnitDep
 
 int unit_add_exec_dependencies(Unit *u, ExecContext *c);
 
-int unit_add_cgroup(Unit *u, CGroupBonding *b);
 int unit_add_cgroup_from_text(Unit *u, const char *name, bool overwrite, CGroupBonding **ret);
 int unit_add_default_cgroups(Unit *u);
 CGroupBonding* unit_get_default_cgroup(Unit *u);