From: Yu Watanabe Date: Sat, 30 Aug 2025 13:25:22 +0000 (+0900) Subject: cgroup-util: do not check validity of controller in cg_split_spec() X-Git-Tag: v259-rc1~18^2~3 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b9e612e070fd766c63b933c2a17c49b053c16111;p=thirdparty%2Fsystemd.git 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 90069684c4f..207f5428b70 100644 --- 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 59f27a72480..7be0de18bf0 100644 --- 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 bdc85adc895..0fd81284e8e 100644 --- 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) {