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.

4 files changed, 172 insertions, 69 deletions

diff --git a/src/shared/cgroup-util.c b/src/shared/cgroup-util.c
index 7521b8b5f9..67e6f70e8e 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];
+
+                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;
 
-        return p;
+                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];
+
+                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 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;
-
-        s = strndupa(e, n - e);
-        s = cg_unescape(s);
-
-        x = startswith(s, "session-");
-        if (!x)
+        end = endswith(start, ".scope");
+        if (!end)
                 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 054c77503b..64650a9134 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 6a883d79d1..2c7254eeda 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 fff9ec22cf..759ca4498b 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());