SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

cgroup: rework how we validate/escape cgroups

Browse Source 
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.
This commit is contained in:
Lennart Poettering
2023-04-27 12:17:58 +02:00
parent 49c778e6bf
commit 1a56b0c05d
6 changed files with 103 additions and 75 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/basic/cgroup-util.c
+53 -42
View File
@@ -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;
bool cg_needs_escape(const char *p) {
/* 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. */
/* 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 "/"). */
/* The return value of this function (unlike cg_unescape())
* needs free()! */
if (!filename_is_valid(p))
return true;
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;
if (IN_SET(p[0], '_', '.'))
return true;
dot = strrchr(p, '.');
if (dot) {
CGroupController c;
size_t l = dot - p;
if (STR_IN_SET(p, "notify_on_release", "release_agent", "tasks"))
return true;
for (c = 0; c < _CGROUP_CONTROLLER_MAX; c++) {
const char *n;
if (startswith(p, "cgroup."))
return true;
n = cgroup_controller_to_string(c);
for (CGroupController c = 0; c < _CGROUP_CONTROLLER_MAX; c++) {
const char *q;
if (l != strlen(n))
continue;
q = startswith(p, cgroup_controller_to_string(c));
if (!q)
continue;
if (memcmp(p, n, l) != 0)
continue;
need_prefix = true;
break;
}
}
if (q[0] == '.')
return true;
}
if (need_prefix)
return strjoin("_", p);
return false;
}
return strdup(p);
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()! */
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;
}
src/basic/cgroup-util.h
+2 -1
View File
@@ -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);
src/core/cgroup.c
+25 -16
View File
@@ -2087,28 +2087,37 @@ static const char *migrate_callback(CGroupMask mask, void *userdata) {
return strempty(unit_get_realized_cgroup_path(userdata, mask));
}
char *unit_default_cgroup_path(const Unit *u) {
_cleanup_free_ char *escaped = NULL, *slice_path = NULL;
Unit *slice;
int unit_default_cgroup_path(const Unit *u, char **ret) {
_cleanup_free_ char *p = NULL;
int r;
assert(u);
assert(ret);
if (unit_has_name(u, SPECIAL_ROOT_SLICE))
return strdup(u->manager->cgroup_root);
p = strdup(u->manager->cgroup_root);
else {
_cleanup_free_ char *escaped = NULL, *slice_path = NULL;
Unit *slice;
slice = UNIT_GET_SLICE(u);
if (slice && !unit_has_name(slice, SPECIAL_ROOT_SLICE)) {
r = cg_slice_to_path(slice->id, &slice_path);
slice = UNIT_GET_SLICE(u);
if (slice && !unit_has_name(slice, SPECIAL_ROOT_SLICE)) {
r = cg_slice_to_path(slice->id, &slice_path);
if (r < 0)
return r;
}
r = cg_escape(u->id, &escaped);
if (r < 0)
return NULL;
return r;
p = path_join(empty_to_root(u->manager->cgroup_root), slice_path, escaped);
}
if (!p)
return -ENOMEM;
escaped = cg_escape(u->id);
if (!escaped)
return NULL;
return path_join(empty_to_root(u->manager->cgroup_root), slice_path, escaped);
*ret = TAKE_PTR(p);
return 0;
}
int unit_set_cgroup_path(Unit *u, const char *path) {
@@ -2264,9 +2273,9 @@ int unit_pick_cgroup_path(Unit *u) {
if (!UNIT_HAS_CGROUP_CONTEXT(u))
return -EINVAL;
path = unit_default_cgroup_path(u);
if (!path)
return log_oom();
r = unit_default_cgroup_path(u, &path);
if (r < 0)
return log_unit_error_errno(u, r, "Failed to generate default cgroup path: %m");
r = unit_set_cgroup_path(u, path);
if (r == -EEXIST)
src/core/cgroup.h
+1 -1
View File
@@ -290,7 +290,7 @@ void unit_invalidate_cgroup_members_masks(Unit *u);
void unit_add_family_to_cgroup_realize_queue(Unit *u);
const char *unit_get_realized_cgroup_path(Unit *u, CGroupMask mask);
char *unit_default_cgroup_path(const Unit *u);
int unit_default_cgroup_path(const Unit *u, char **ret);
int unit_set_cgroup_path(Unit *u, const char *path);
int unit_pick_cgroup_path(Unit *u);
src/core/unit-printf.c
+12 -10
View File
@@ -86,19 +86,21 @@ static void bad_specifier(const Unit *u, char specifier) {
static int specifier_cgroup(char specifier, const void *data, const char *root, const void *userdata, char **ret) {
const Unit *u = ASSERT_PTR(userdata);
char *n;
bad_specifier(u, specifier);
if (u->cgroup_path)
n = strdup(u->cgroup_path);
else
n = unit_default_cgroup_path(u);
if (!n)
return -ENOMEM;
if (u->cgroup_path) {
char *n;
*ret = n;
return 0;
n = strdup(u->cgroup_path);
if (!n)
return -ENOMEM;
*ret = n;
return 0;
}
return unit_default_cgroup_path(u, ret);
}
static int specifier_cgroup_root(char specifier, const void *data, const char *root, const void *userdata, char **ret) {
@@ -126,7 +128,7 @@ static int specifier_cgroup_slice(char specifier, const void *data, const char *
if (slice->cgroup_path)
n = strdup(slice->cgroup_path);
else
n = unit_default_cgroup_path(slice);
return unit_default_cgroup_path(slice, ret);
} else
n = strdup(u->manager->cgroup_root);
if (!n)
src/test/test-cgroup-util.c
+10 -5
View File
@@ -235,14 +235,19 @@ TEST(proc) {
}
}
static void test_escape_one(const char *s, const char *r) {
_cleanup_free_ char *b;
static void test_escape_one(const char *s, const char *expected) {
_cleanup_free_ char *b = NULL;
b = cg_escape(s);
assert_se(b);
assert_se(streq(b, r));
assert_se(s);
assert_se(expected);
assert_se(cg_escape(s, &b) >= 0);
assert_se(streq(b, expected));
assert_se(streq(cg_unescape(b), s));
assert_se(filename_is_valid(b));
assert_se(!cg_needs_escape(s) || b[0] == '_');
}
TEST(escape, .sd_booted = true) {