cgroup-util: do not check validity of controller in cg_split_spec()

Now the controller part is always ignored, hence let's skip check for
the controller part of the spec. This also make it acceppt unnormalized
path. Previously paths were checked by path_is_normalized(), but now
checked by path_is_safe(). Also, now this mapps an empty path to NULL.

diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c
index 90069684c4facc08c8130b6b9e1edfbac3da9a0e..207f5428b70ae5c6d39b7f9d146c4c339802c5db 100644 (file)
--- a/src/basic/cgroup-util.c
+++ b/src/basic/cgroup-util.c
@@ -630,60 +630,64 @@ int cg_is_empty(const char *path) {
 }
 
 int cg_split_spec(const char *spec, char **ret_controller, char **ret_path) {
-        _cleanup_free_ char *controller = NULL, *path = NULL;
+        _cleanup_free_ char *controller = NULL;
+        const char *path;
         int r;
 
         assert(spec);
 
-        if (*spec == '/') {
-                if (!path_is_normalized(spec))
-                        return -EINVAL;
+        /* This extracts the path part from the deprecated controller:path spec. The path must be absolute or
+         * an empty string. No validation is done for the controller part. */
 
-                if (ret_path) {
-                        r = path_simplify_alloc(spec, &path);
-                        if (r < 0)
-                                return r;
+        if (isempty(spec) || path_is_absolute(spec)) {
+                /* Assume this does not contain controller. */
+                path = spec;
+                goto finalize;
+        }
+
+        const char *e = strchr(spec, ':');
+        if (!e) {
+                /* Controller only. */
+                if (ret_controller) {
+                        controller = strdup(spec);
+                        if (!controller)
+                                return -ENOMEM;
                 }
 
+                path = NULL;
         } else {
-                const char *e;
-
-                e = strchr(spec, ':');
-                if (e) {
-                        controller = strndup(spec, e-spec);
+                /* Both controller and path. */
+                if (ret_controller) {
+                        controller = strndup(spec, e - spec);
                         if (!controller)
                                 return -ENOMEM;
-                        if (!cg_controller_is_valid(controller))
-                                return -EINVAL;
+                }
 
-                        if (!isempty(e + 1)) {
-                                path = strdup(e+1);
-                                if (!path)
-                                        return -ENOMEM;
+                path = e + 1;
+        }
 
-                                if (!path_is_normalized(path) ||
-                                    !path_is_absolute(path))
-                                        return -EINVAL;
+finalize:
+        path = empty_to_null(path);
 
-                                path_simplify(path);
-                        }
+        if (path) {
+                /* Non-empty path must be absolute. */
+                if (!path_is_absolute(path))
+                        return -EINVAL;
 
-                } else {
-                        if (!cg_controller_is_valid(spec))
-                                return -EINVAL;
+                /* Path must not contain dot-dot. */
+                if (!path_is_safe(path))
+                        return -EINVAL;
+        }
 
-                        if (ret_controller) {
-                                controller = strdup(spec);
-                                if (!controller)
-                                        return -ENOMEM;
-                        }
-                }
+        if (ret_path) {
+                r = path_simplify_alloc(path, ret_path);
+                if (r < 0)
+                        return r;
         }
 
         if (ret_controller)
                 *ret_controller = TAKE_PTR(controller);
-        if (ret_path)
-                *ret_path = TAKE_PTR(path);
+
         return 0;
 }
 
@@ -1314,36 +1318,6 @@ char* cg_unescape(const char *p) {
         return (char*) p;
 }
 
-#define CONTROLLER_VALID                        \
-        DIGITS LETTERS                          \
-        "_"
-
-bool cg_controller_is_valid(const char *p) {
-        const char *t, *s;
-
-        if (!p)
-                return false;
-
-        if (streq(p, SYSTEMD_CGROUP_CONTROLLER))
-                return true;
-
-        s = startswith(p, "name=");
-        if (s)
-                p = s;
-
-        if (IN_SET(*p, 0, '_'))
-                return false;
-
-        for (t = p; *t; t++)
-                if (!strchr(CONTROLLER_VALID, *t))
-                        return false;
-
-        if (t - p > NAME_MAX)
-                return false;
-
-        return true;
-}
-
 int cg_slice_to_path(const char *unit, char **ret) {
         _cleanup_free_ char *p = NULL, *s = NULL, *e = NULL;
         const char *dash;

diff --git a/src/basic/cgroup-util.h b/src/basic/cgroup-util.h
index 59f27a72480dcc05352c68c3f022f0d8c20426c2..7be0de18bf0649d33668e90199a15e5f281cd76f 100644 (file)
--- a/src/basic/cgroup-util.h
+++ b/src/basic/cgroup-util.h
@@ -242,8 +242,6 @@ bool cg_needs_escape(const char *p) _pure_;
 int cg_escape(const char *p, char **ret);
 char* cg_unescape(const char *p) _pure_;
 
-bool cg_controller_is_valid(const char *p);
-
 int cg_slice_to_path(const char *unit, char **ret);
 
 int cg_mask_supported(CGroupMask *ret);

diff --git a/src/test/test-cgroup-util.c b/src/test/test-cgroup-util.c
index bdc85adc8952aecacd235f9c1811d56623d04b62..0fd81284e8e3abd36f2a1bf150ac55155c376060 100644 (file)
--- a/src/test/test-cgroup-util.c
+++ b/src/test/test-cgroup-util.c
@@ -322,47 +322,52 @@ TEST(escape, .sd_booted = true) {
         test_escape_one(".", "_.");
 }
 
-TEST(controller_is_valid) {
-        assert_se(cg_controller_is_valid("foobar"));
-        assert_se(cg_controller_is_valid("foo_bar"));
-        assert_se(cg_controller_is_valid("name=foo"));
-        assert_se(!cg_controller_is_valid(""));
-        assert_se(!cg_controller_is_valid("name="));
-        assert_se(!cg_controller_is_valid("="));
-        assert_se(!cg_controller_is_valid("cpu,cpuacct"));
-        assert_se(!cg_controller_is_valid("_"));
-        assert_se(!cg_controller_is_valid("_foobar"));
-        assert_se(!cg_controller_is_valid("tatü"));
-}
-
 TEST(cg_split_spec) {
         char *c, *p;
 
-        ASSERT_OK_ZERO(cg_split_spec("foobar:/", &c, &p));
+        ASSERT_OK(cg_split_spec("foobar:/", &c, &p));
         ASSERT_STREQ(c, "foobar");
         ASSERT_STREQ(p, "/");
         c = mfree(c);
         p = mfree(p);
 
-        ASSERT_OK_ZERO(cg_split_spec("foobar:", &c, &p));
+        ASSERT_OK(cg_split_spec("foobar:", &c, &p));
+        ASSERT_STREQ(c, "foobar");
+        ASSERT_NULL(p);
         c = mfree(c);
-        p = mfree(p);
 
-        ASSERT_FAIL(cg_split_spec("foobar:asdfd", &c, &p));
-        ASSERT_FAIL(cg_split_spec(":///", &c, &p));
-        ASSERT_FAIL(cg_split_spec(":", &c, &p));
-        ASSERT_FAIL(cg_split_spec("", &c, &p));
-        ASSERT_FAIL(cg_split_spec("fo/obar:/", &c, &p));
+        ASSERT_OK(cg_split_spec("foobar", &c, &p));
+        ASSERT_STREQ(c, "foobar");
+        ASSERT_NULL(p);
+        c = mfree(c);
 
-        ASSERT_OK(cg_split_spec("/", &c, &p));
-        ASSERT_NULL(c);
+        ASSERT_OK(cg_split_spec(":///", &c, &p));
+        ASSERT_STREQ(c, "");
         ASSERT_STREQ(p, "/");
+        c = mfree(c);
         p = mfree(p);
 
-        ASSERT_OK(cg_split_spec("foo", &c, &p));
-        ASSERT_STREQ(c, "foo");
+        ASSERT_OK(cg_split_spec(":", &c, &p));
+        ASSERT_STREQ(c, "");
+        ASSERT_NULL(p);
+        c = mfree(c);
+
+        ASSERT_OK(cg_split_spec("", &c, &p));
+        ASSERT_NULL(c);
         ASSERT_NULL(p);
+
+        ASSERT_OK(cg_split_spec("fo/obar:/", &c, &p));
+        ASSERT_STREQ(c, "fo/obar");
+        ASSERT_STREQ(p, "/");
         c = mfree(c);
+        p = mfree(p);
+
+        ASSERT_ERROR(cg_split_spec("foobar:asdfd", &c, &p), EINVAL);
+
+        ASSERT_OK(cg_split_spec("/", &c, &p));
+        ASSERT_NULL(c);
+        ASSERT_STREQ(p, "/");
+        p = mfree(p);
 }
 
 static void test_slice_to_path_one(const char *unit, const char *path, int error) {