From 8b0849e9710d721c5d0b775aaf0fd662eefa1449 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 30 Apr 2015 00:47:41 +0200 Subject: [PATCH] core: rework cgroup path parse logic Various cleanups, be stricter when parsing unit paths. Most importantly: return the root slice "-.slice" when asked for slice of paths that contain no slice component. --- src/shared/cgroup-util.c | 212 +++++++++++++++++++++++++----------- src/shared/login-shared.c | 6 +- src/shared/util.c | 5 +- src/test/test-cgroup-util.c | 18 +++ 4 files changed, 172 insertions(+), 69 deletions(-) diff --git a/src/shared/cgroup-util.c b/src/shared/cgroup-util.c index 7521b8b5f90..67e6f70e8e8 100644 --- a/src/shared/cgroup-util.c +++ b/src/shared/cgroup-util.c @@ -40,6 +40,7 @@ #include "fileio.h" #include "special.h" #include "mkdir.h" +#include "login-shared.h" int cg_enumerate_processes(const char *controller, const char *path, FILE **_f) { _cleanup_free_ char *fs = NULL; @@ -1140,13 +1141,17 @@ int cg_pid_get_path_shifted(pid_t pid, const char *root, char **cgroup) { } int cg_path_decode_unit(const char *cgroup, char **unit){ - char *e, *c, *s; + char *c, *s; + size_t n; assert(cgroup); assert(unit); - e = strchrnul(cgroup, '/'); - c = strndupa(cgroup, e - cgroup); + n = strcspn(cgroup, "/"); + if (n < 3) + return -ENXIO; + + c = strndupa(cgroup, n); c = cg_unescape(c); if (!unit_name_is_valid(c, TEMPLATE_INVALID)) @@ -1160,7 +1165,31 @@ int cg_path_decode_unit(const char *cgroup, char **unit){ return 0; } +static bool valid_slice_name(const char *p, size_t n) { + + if (!p) + return false; + + if (n < strlen("x.slice")) + return false; + + if (memcmp(p + n - 6, ".slice", 6) == 0) { + char buf[n+1], *c; + + memcpy(buf, p, n); + buf[n] = 0; + + c = cg_unescape(buf); + + return unit_name_is_valid(c, TEMPLATE_INVALID); + } + + return false; +} + static const char *skip_slices(const char *p) { + assert(p); + /* Skips over all slice assignments */ for (;;) { @@ -1169,22 +1198,35 @@ static const char *skip_slices(const char *p) { p += strspn(p, "/"); n = strcspn(p, "/"); - if (n <= 6 || memcmp(p + n - 6, ".slice", 6) != 0) + if (!valid_slice_name(p, n)) return p; p += n; } } -int cg_path_get_unit(const char *path, char **unit) { +int cg_path_get_unit(const char *path, char **ret) { const char *e; + char *unit; + int r; assert(path); - assert(unit); + assert(ret); e = skip_slices(path); - return cg_path_decode_unit(e, unit); + r = cg_path_decode_unit(e, &unit); + if (r < 0) + return r; + + /* We skipped over the slices, don't accept any now */ + if (endswith(unit, ".slice")) { + free(unit); + return -ENXIO; + } + + *ret = unit; + return 0; } int cg_pid_get_unit(pid_t pid, char **unit) { @@ -1206,18 +1248,35 @@ int cg_pid_get_unit(pid_t pid, char **unit) { static const char *skip_session(const char *p) { size_t n; - assert(p); + if (isempty(p)) + return NULL; p += strspn(p, "/"); n = strcspn(p, "/"); - if (n < strlen("session-x.scope") || memcmp(p, "session-", 8) != 0 || memcmp(p + n - 6, ".scope", 6) != 0) + if (n < strlen("session-x.scope")) return NULL; - p += n; - p += strspn(p, "/"); + if (memcmp(p, "session-", 8) == 0 && memcmp(p + n - 6, ".scope", 6) == 0) { + char buf[n - 8 - 6 + 1]; - return p; + memcpy(buf, p + 8, n - 8 - 6); + buf[n - 8 - 6] = 0; + + /* Note that session scopes never need unescaping, + * since they cannot conflict with the kernel's own + * names, hence we don't need to call cg_unescape() + * here. */ + + if (!session_id_valid(buf)) + return false; + + p += n; + p += strspn(p, "/"); + return p; + } + + return NULL; } /** @@ -1226,25 +1285,45 @@ static const char *skip_session(const char *p) { static const char *skip_user_manager(const char *p) { size_t n; - assert(p); + if (isempty(p)) + return NULL; p += strspn(p, "/"); n = strcspn(p, "/"); - if (n < strlen("user@x.service") || memcmp(p, "user@", 5) != 0 || memcmp(p + n - 8, ".service", 8) != 0) + if (n < strlen("user@x.service")) return NULL; - p += n; - p += strspn(p, "/"); + if (memcmp(p, "user@", 5) == 0 && memcmp(p + n - 8, ".service", 8) == 0) { + char buf[n - 5 - 8 + 1]; - return p; + memcpy(buf, p + 5, n - 5 - 8); + buf[n - 5 - 8] = 0; + + /* Note that user manager services never need unescaping, + * since they cannot conflict with the kernel's own + * names, hence we don't need to call cg_unescape() + * here. */ + + if (parse_uid(buf, NULL) < 0) + return NULL; + + p += n; + p += strspn(p, "/"); + + return p; + } + + return NULL; } -int cg_path_get_user_unit(const char *path, char **unit) { +int cg_path_get_user_unit(const char *path, char **ret) { const char *e, *t; + char *unit; + int r; assert(path); - assert(unit); + assert(ret); /* We always have to parse the path from the beginning as unit * cgroups might have arbitrary child cgroups and we shouldn't get @@ -1253,17 +1332,30 @@ int cg_path_get_user_unit(const char *path, char **unit) { /* Skip slices, if there are any */ e = skip_slices(path); - /* Skip the session scope or user manager... */ - t = skip_session(e); + /* Skip the user manager... */ + t = skip_user_manager(e); + + /* Alternatively skip the user session... */ if (!t) - t = skip_user_manager(e); + t = skip_session(e); if (!t) return -ENXIO; /* ... and skip more slices if there are any */ e = skip_slices(t); - return cg_path_decode_unit(e, unit); + r = cg_path_decode_unit(e, &unit); + if (r < 0) + return r; + + /* We skipped over the slices, don't accept any now */ + if (endswith(unit, ".slice")) { + free(unit); + return -ENXIO; + } + + *ret = unit; + return 0; } int cg_pid_get_user_unit(pid_t pid, char **unit) { @@ -1308,36 +1400,35 @@ int cg_pid_get_machine_name(pid_t pid, char **machine) { } int cg_path_get_session(const char *path, char **session) { - const char *e, *n, *x, *y; - char *s; + _cleanup_free_ char *unit = NULL; + char *start, *end; + int r; assert(path); - /* Skip slices, if there are any */ - e = skip_slices(path); + r = cg_path_get_unit(path, &unit); + if (r < 0) + return r; - n = strchrnul(e, '/'); - if (e == n) + start = startswith(unit, "session-"); + if (!start) + return -ENXIO; + end = endswith(start, ".scope"); + if (!end) return -ENXIO; - s = strndupa(e, n - e); - s = cg_unescape(s); - - x = startswith(s, "session-"); - if (!x) - return -ENXIO; - y = endswith(x, ".scope"); - if (!y || x == y) + *end = 0; + if (!session_id_valid(start)) return -ENXIO; if (session) { - char *r; + char *rr; - r = strndup(x, y - x); - if (!r) + rr = strdup(start); + if (!rr) return -ENOMEM; - *session = r; + *session = rr; } return 0; @@ -1356,9 +1447,7 @@ int cg_pid_get_session(pid_t pid, char **session) { int cg_path_get_owner_uid(const char *path, uid_t *uid) { _cleanup_free_ char *slice = NULL; - const char *start, *end; - char *s; - uid_t u; + char *start, *end; int r; assert(path); @@ -1370,20 +1459,14 @@ int cg_path_get_owner_uid(const char *path, uid_t *uid) { start = startswith(slice, "user-"); if (!start) return -ENXIO; - end = endswith(slice, ".slice"); + end = endswith(start, ".slice"); if (!end) return -ENXIO; - s = strndupa(start, end - start); - if (!s) + *end = 0; + if (parse_uid(start, uid) < 0) return -ENXIO; - if (parse_uid(s, &u) < 0) - return -ENXIO; - - if (uid) - *uid = u; - return 0; } @@ -1400,7 +1483,6 @@ int cg_pid_get_owner_uid(pid_t pid, uid_t *uid) { int cg_path_get_slice(const char *p, char **slice) { const char *e = NULL; - size_t m = 0; assert(p); assert(slice); @@ -1411,23 +1493,23 @@ int cg_path_get_slice(const char *p, char **slice) { p += strspn(p, "/"); n = strcspn(p, "/"); - if (n <= 6 || memcmp(p + n - 6, ".slice", 6) != 0) { - char *s; + if (!valid_slice_name(p, n)) { - if (!e) - return -ENXIO; + if (!e) { + char *s; - s = strndup(e, m); - if (!s) - return -ENOMEM; + s = strdup("-.slice"); + if (!s) + return -ENOMEM; - *slice = s; - return 0; + *slice = s; + return 0; + } + + return cg_path_decode_unit(e, slice); } e = p; - m = n; - p += n; } } diff --git a/src/shared/login-shared.c b/src/shared/login-shared.c index 054c77503be..64650a91341 100644 --- a/src/shared/login-shared.c +++ b/src/shared/login-shared.c @@ -23,7 +23,9 @@ #include "def.h" bool session_id_valid(const char *id) { - assert(id); - return id[0] && id[strspn(id, LETTERS DIGITS)] == '\0'; + if (isempty(id)) + return false; + + return id[strspn(id, LETTERS DIGITS)] == '\0'; } diff --git a/src/shared/util.c b/src/shared/util.c index 6a883d79d12..2c7254eedac 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -350,7 +350,6 @@ int parse_uid(const char *s, uid_t* ret_uid) { int r; assert(s); - assert(ret_uid); r = safe_atolu(s, &ul); if (r < 0) @@ -369,7 +368,9 @@ int parse_uid(const char *s, uid_t* ret_uid) { if (uid == (uid_t) 0xFFFF) return -ENXIO; - *ret_uid = uid; + if (ret_uid) + *ret_uid = uid; + return 0; } diff --git a/src/test/test-cgroup-util.c b/src/test/test-cgroup-util.c index fff9ec22cf5..759ca4498b0 100644 --- a/src/test/test-cgroup-util.c +++ b/src/test/test-cgroup-util.c @@ -125,6 +125,23 @@ static void test_path_get_owner_uid(void) { check_p_g_o_u("", -ENXIO, 0); } +static void check_p_g_slice(const char *path, int code, const char *result) { + _cleanup_free_ char *s = NULL; + + assert_se(cg_path_get_slice(path, &s) == code); + assert_se(streq_ptr(s, result)); +} + +static void test_path_get_slice(void) { + check_p_g_slice("/user.slice", 0, "user.slice"); + check_p_g_slice("/foobar", 0, "-.slice"); + check_p_g_slice("/user.slice/user-waldo.slice", 0, "user-waldo.slice"); + check_p_g_slice("", 0, "-.slice"); + check_p_g_slice("foobar", 0, "-.slice"); + check_p_g_slice("foobar.slice", 0, "foobar.slice"); + check_p_g_slice("foo.slice/foo-bar.slice/waldo.service", 0, "foo-bar.slice"); +} + static void test_get_paths(void) { _cleanup_free_ char *a = NULL; @@ -255,6 +272,7 @@ int main(void) { test_path_get_user_unit(); test_path_get_session(); test_path_get_owner_uid(); + test_path_get_slice(); TEST_REQ_RUNNING_SYSTEMD(test_get_paths()); test_proc(); TEST_REQ_RUNNING_SYSTEMD(test_escape());