cgroup: always validate cgroup controller names
Let's better be safe than sorry.
This commit is contained in:
parent
e10375f2c0
commit
78edb35ab4
4
TODO
4
TODO
|
@ -57,10 +57,6 @@ Features:
|
|||
|
||||
* add s.th. like "systemctl set-log-level debug"
|
||||
|
||||
* sd-login: allow enumerating machines and add inotify iface
|
||||
|
||||
* cgroup-util: verify syntax of cgroup controllers
|
||||
|
||||
* cgtop: make cgtop useful in a container
|
||||
|
||||
* make sure cg_pid_get_path() works properly for co-mounted controllers
|
||||
|
|
|
@ -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;
|
||||
|
||||
|
|
|
@ -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);
|
||||
|
|
|
@ -510,6 +510,9 @@ int cg_get_path(const char *controller, const char *path, const char *suffix, ch
|
|||
|
||||
assert(fs);
|
||||
|
||||
if (controller && !cg_controller_is_valid(controller, true))
|
||||
return -EINVAL;
|
||||
|
||||
if (_unlikely_(!good)) {
|
||||
int r;
|
||||
|
||||
|
@ -546,7 +549,7 @@ int cg_get_path_and_check(const char *controller, const char *path, const char *
|
|||
|
||||
assert(fs);
|
||||
|
||||
if (isempty(controller))
|
||||
if (!cg_controller_is_valid(controller, true))
|
||||
return -EINVAL;
|
||||
|
||||
/* Normalize the controller syntax */
|
||||
|
@ -741,6 +744,9 @@ int cg_pid_get_path(const char *controller, pid_t pid, char **path) {
|
|||
assert(path);
|
||||
assert(pid >= 0);
|
||||
|
||||
if (controller && !cg_controller_is_valid(controller, true))
|
||||
return -EINVAL;
|
||||
|
||||
if (!controller)
|
||||
controller = SYSTEMD_CGROUP_CONTROLLER;
|
||||
|
||||
|
@ -933,7 +939,7 @@ int cg_split_spec(const char *spec, char **controller, char **path) {
|
|||
|
||||
e = strchr(spec, ':');
|
||||
if (!e) {
|
||||
if (!filename_is_safe(spec))
|
||||
if (!cg_controller_is_valid(spec, true))
|
||||
return -EINVAL;
|
||||
|
||||
if (controller) {
|
||||
|
@ -953,7 +959,7 @@ int cg_split_spec(const char *spec, char **controller, char **path) {
|
|||
t = strndup(spec, e-spec);
|
||||
if (!t)
|
||||
return -ENOMEM;
|
||||
if (!filename_is_safe(t)) {
|
||||
if (!cg_controller_is_valid(t, true)) {
|
||||
free(t);
|
||||
return -EINVAL;
|
||||
}
|
||||
|
@ -987,18 +993,19 @@ int cg_join_spec(const char *controller, const char *path, char **spec) {
|
|||
|
||||
assert(path);
|
||||
|
||||
|
||||
if (!controller)
|
||||
controller = "systemd";
|
||||
else if (controller[0] == 0 ||
|
||||
strchr(controller, ':') ||
|
||||
strchr(controller, '/'))
|
||||
return -EINVAL;
|
||||
else {
|
||||
if (!cg_controller_is_valid(controller, true))
|
||||
return -EINVAL;
|
||||
|
||||
controller = normalize_controller(controller);
|
||||
}
|
||||
|
||||
if (!path_is_absolute(path))
|
||||
return -EINVAL;
|
||||
|
||||
controller = normalize_controller(controller);
|
||||
|
||||
s = strjoin(controller, ":", path, NULL);
|
||||
if (!s)
|
||||
return -ENOMEM;
|
||||
|
@ -1008,7 +1015,8 @@ int cg_join_spec(const char *controller, const char *path, char **spec) {
|
|||
}
|
||||
|
||||
int cg_mangle_path(const char *path, char **result) {
|
||||
char *t, *c, *p;
|
||||
_cleanup_free_ char *c = NULL, *p = NULL;
|
||||
char *t;
|
||||
int r;
|
||||
|
||||
assert(path);
|
||||
|
@ -1030,11 +1038,7 @@ int cg_mangle_path(const char *path, char **result) {
|
|||
if (r < 0)
|
||||
return r;
|
||||
|
||||
r = cg_get_path(c ? c : SYSTEMD_CGROUP_CONTROLLER, p ? p : "/", NULL, result);
|
||||
free(c);
|
||||
free(p);
|
||||
|
||||
return r;
|
||||
return cg_get_path(c ? c : SYSTEMD_CGROUP_CONTROLLER, p ? p : "/", NULL, result);
|
||||
}
|
||||
|
||||
int cg_get_system_path(char **path) {
|
||||
|
@ -1138,14 +1142,20 @@ char **cg_shorten_controllers(char **controllers) {
|
|||
|
||||
p = normalize_controller(*f);
|
||||
|
||||
if (streq(*f, "systemd")) {
|
||||
if (streq(p, "systemd")) {
|
||||
free(*f);
|
||||
continue;
|
||||
}
|
||||
|
||||
if (!cg_controller_is_valid(p, true)) {
|
||||
log_warning("Controller %s is not valid, removing from controllers list.", p);
|
||||
free(*f);
|
||||
continue;
|
||||
}
|
||||
|
||||
r = check_hierarchy(p);
|
||||
if (r < 0) {
|
||||
log_debug("Controller %s is not available, removing from controllers list.", *f);
|
||||
log_debug("Controller %s is not available, removing from controllers list.", p);
|
||||
free(*f);
|
||||
continue;
|
||||
}
|
||||
|
@ -1457,7 +1467,7 @@ int cg_controller_from_attr(const char *attr, char **controller) {
|
|||
if (!c)
|
||||
return -ENOMEM;
|
||||
|
||||
if (!filename_is_safe(c)) {
|
||||
if (!cg_controller_is_valid(c, false)) {
|
||||
free(c);
|
||||
return -EINVAL;
|
||||
}
|
||||
|
@ -1517,3 +1527,34 @@ char *cg_unescape(const char *p) {
|
|||
|
||||
return (char*) p;
|
||||
}
|
||||
|
||||
#define CONTROLLER_VALID \
|
||||
"0123456789" \
|
||||
"abcdefghijklmnopqrstuvwxyz" \
|
||||
"ABCDEFGHIJKLMNOPQRSTUVWXYZ" \
|
||||
"_"
|
||||
|
||||
bool cg_controller_is_valid(const char *p, bool allow_named) {
|
||||
const char *t, *s;
|
||||
|
||||
if (!p)
|
||||
return false;
|
||||
|
||||
if (allow_named) {
|
||||
s = startswith(p, "name=");
|
||||
if (s)
|
||||
p = s;
|
||||
}
|
||||
|
||||
if (*p == 0 || *p == '_')
|
||||
return false;
|
||||
|
||||
for (t = p; *t; t++)
|
||||
if (!strchr(CONTROLLER_VALID, *t))
|
||||
return false;
|
||||
|
||||
if (t - p > FILENAME_MAX)
|
||||
return false;
|
||||
|
||||
return true;
|
||||
}
|
||||
|
|
|
@ -96,3 +96,5 @@ int cg_controller_from_attr(const char *attr, char **controller);
|
|||
|
||||
char *cg_escape(const char *p);
|
||||
char *cg_unescape(const char *p);
|
||||
|
||||
bool cg_controller_is_valid(const char *p, bool allow_named);
|
||||
|
|
|
@ -153,6 +153,19 @@ static void test_escape(void) {
|
|||
test_escape_one("_foobar", "__foobar");
|
||||
}
|
||||
|
||||
static void test_controller_is_valid(void) {
|
||||
assert_se(cg_controller_is_valid("foobar", false));
|
||||
assert_se(cg_controller_is_valid("foo_bar", false));
|
||||
assert_se(cg_controller_is_valid("name=foo", true));
|
||||
assert_se(!cg_controller_is_valid("", false));
|
||||
assert_se(!cg_controller_is_valid("name=", true));
|
||||
assert_se(!cg_controller_is_valid("=", false));
|
||||
assert_se(!cg_controller_is_valid("cpu,cpuacct", false));
|
||||
assert_se(!cg_controller_is_valid("_", false));
|
||||
assert_se(!cg_controller_is_valid("_foobar", false));
|
||||
assert_se(!cg_controller_is_valid("tatü", false));
|
||||
}
|
||||
|
||||
int main(void) {
|
||||
test_path_decode_unit();
|
||||
test_path_get_unit();
|
||||
|
@ -160,6 +173,7 @@ int main(void) {
|
|||
test_get_paths();
|
||||
test_proc();
|
||||
test_escape();
|
||||
test_controller_is_valid();
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue