summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLennart Poettering <lennart@poettering.net>2013-04-24 19:01:29 -0300
committerLennart Poettering <lennart@poettering.net>2013-04-24 19:02:13 -0300
commit78edb35ab4f4227485cb9ec816b43c37e0d5e62a (patch)
tree20d78b824f47ce62a790c448799c471d4db9900e
parente10375f2c0f5dd0dc8508f3ca165eb8aa63c64fb (diff)
cgroup: always validate cgroup controller names
Let's better be safe than sorry.
-rw-r--r--TODO4
-rw-r--r--src/core/unit.c26
-rw-r--r--src/core/unit.h1
-rw-r--r--src/shared/cgroup-util.c77
-rw-r--r--src/shared/cgroup-util.h2
-rw-r--r--src/test/test-cgroup-util.c14
6 files changed, 89 insertions, 35 deletions
diff --git a/TODO b/TODO
index 88a3b2cb45..cfd42ce745 100644
--- a/TODO
+++ b/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
diff --git a/src/core/unit.c b/src/core/unit.c
index 4b9abf32dc..c0c3ce90a7 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -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;
diff --git a/src/core/unit.h b/src/core/unit.h
index 51a8364d63..6bfe58c8bf 100644
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -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);
diff --git a/src/shared/cgroup-util.c b/src/shared/cgroup-util.c
index 9ec4f40c88..b79a24a495 100644
--- a/src/shared/cgroup-util.c
+++ b/src/shared/cgroup-util.c
@@ -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;
+}
diff --git a/src/shared/cgroup-util.h b/src/shared/cgroup-util.h
index 2099f934b4..a2ee72d67f 100644
--- a/src/shared/cgroup-util.h
+++ b/src/shared/cgroup-util.h
@@ -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);
diff --git a/src/test/test-cgroup-util.c b/src/test/test-cgroup-util.c
index 95cede7a22..6726f8fb13 100644
--- a/src/test/test-cgroup-util.c
+++ b/src/test/test-cgroup-util.c
@@ -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;
}