cgroup: rework how we validate/escape cgroups

Let's clean up validation/escaping of cgroup names. i.e. split out code
that tests if name needs escaping. Return proper error codes, and extend
test a bit.

2 files changed, 57 insertions, 45 deletions

diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c
index 90877c9fa1..21e2255dae 100644
--- a/src/basic/cgroup-util.c
+++ b/src/basic/cgroup-util.c
@@ -1554,52 +1554,64 @@ int cg_pid_get_user_slice(pid_t pid, char **slice) {
         return cg_path_get_user_slice(cgroup, slice);
 }
 
-char *cg_escape(const char *p) {
-        bool need_prefix = false;
-
-        /* This implements very minimal escaping for names to be used
-         * as file names in the cgroup tree: any name which might
-         * conflict with a kernel name or is prefixed with '_' is
-         * prefixed with a '_'. That way, when reading cgroup names it
-         * is sufficient to remove a single prefixing underscore if
-         * there is one. */
-
-        /* The return value of this function (unlike cg_unescape())
-         * needs free()! */
-
-        if (IN_SET(p[0], 0, '_', '.') ||
-            STR_IN_SET(p, "notify_on_release", "release_agent", "tasks") ||
-            startswith(p, "cgroup."))
-                need_prefix = true;
-        else {
-                const char *dot;
+bool cg_needs_escape(const char *p) {
 
-                dot = strrchr(p, '.');
-                if (dot) {
-                        CGroupController c;
-                        size_t l = dot - p;
+        /* Checks if the specified path is a valid cgroup name by our rules, or if it must be escaped. Note
+         * that we consider escaped cgroup names invalid here, as they need to be escaped a second time if
+         * they shall be used. Also note that various names cannot be made valid by escaping even if we
+         * return true here (because too long, or contain the forbidden character "/"). */
 
-                        for (c = 0; c < _CGROUP_CONTROLLER_MAX; c++) {
-                                const char *n;
+        if (!filename_is_valid(p))
+                return true;
 
-                                n = cgroup_controller_to_string(c);
+        if (IN_SET(p[0], '_', '.'))
+                return true;
 
-                                if (l != strlen(n))
-                                        continue;
+        if (STR_IN_SET(p, "notify_on_release", "release_agent", "tasks"))
+                return true;
 
-                                if (memcmp(p, n, l) != 0)
-                                        continue;
+        if (startswith(p, "cgroup."))
+                return true;
 
-                                need_prefix = true;
-                                break;
-                        }
-                }
+        for (CGroupController c = 0; c < _CGROUP_CONTROLLER_MAX; c++) {
+                const char *q;
+
+                q = startswith(p, cgroup_controller_to_string(c));
+                if (!q)
+                        continue;
+
+                if (q[0] == '.')
+                        return true;
         }
 
-        if (need_prefix)
-                return strjoin("_", p);
+        return false;
+}
+
+int cg_escape(const char *p, char **ret) {
+        _cleanup_free_ char *n = NULL;
+
+        /* This implements very minimal escaping for names to be used as file names in the cgroup tree: any
+         * name which might conflict with a kernel name or is prefixed with '_' is prefixed with a '_'. That
+         * way, when reading cgroup names it is sufficient to remove a single prefixing underscore if there
+         * is one. */
+
+        /* The return value of this function (unlike cg_unescape()) needs free()! */
 
-        return strdup(p);
+        if (cg_needs_escape(p)) {
+                n = strjoin("_", p);
+                if (!n)
+                        return -ENOMEM;
+
+                if (!filename_is_valid(n)) /* became invalid due to the prefixing? Or contained things like a slash that cannot be fixed by prefixing? */
+                        return -EINVAL;
+        } else {
+                n = strdup(p);
+                if (!n)
+                        return -ENOMEM;
+        }
+
+        *ret = TAKE_PTR(n);
+        return 0;
 }
 
 char *cg_unescape(const char *p) {
@@ -1698,9 +1710,9 @@ int cg_slice_to_path(const char *unit, char **ret) {
                 if (!unit_name_is_valid(n, UNIT_NAME_PLAIN))
                         return -EINVAL;
 
-                escaped = cg_escape(n);
-                if (!escaped)
-                        return -ENOMEM;
+                r = cg_escape(n, &escaped);
+                if (r < 0)
+                        return r;
 
                 if (!strextend(&s, escaped, "/"))
                         return -ENOMEM;
@@ -1708,15 +1720,14 @@ int cg_slice_to_path(const char *unit, char **ret) {
                 dash = strchr(dash+1, '-');
         }
 
-        e = cg_escape(unit);
-        if (!e)
-                return -ENOMEM;
+        r = cg_escape(unit, &e);
+        if (r < 0)
+                return r;
 
         if (!strextend(&s, e))
                 return -ENOMEM;
 
         *ret = TAKE_PTR(s);
-
         return 0;
 }
 
diff --git a/src/basic/cgroup-util.h b/src/basic/cgroup-util.h
index 5de000c4ce..9b30ae0396 100644
--- a/src/basic/cgroup-util.h
+++ b/src/basic/cgroup-util.h
@@ -279,7 +279,8 @@ int cg_pid_get_user_slice(pid_t pid, char **slice);
 
 int cg_path_decode_unit(const char *cgroup, char **unit);
 
-char *cg_escape(const char *p);
+bool cg_needs_escape(const char *p);
+int cg_escape(const char *p, char **ret);
 char *cg_unescape(const char *p) _pure_;
 
 bool cg_controller_is_valid(const char *p);