Merge pull request #22993 from keszybz/refactor-manager_taint_string()

Refactor manager_taint_string()

src/basic/strv.c

@@ -351,7 +351,7 @@ int strv_split_colon_pairs(char ***t, const char *s) {
         return (int) n;
 }
 
-char* strv_join_full(char * const *l, const char *separator, const char *prefix, bool unescape_separators) {
+char* strv_join_full(char * const *l, const char *separator, const char *prefix, bool escape_separator) {
         char *r, *e;
         size_t n, k, m;
 
@@ -361,7 +361,7 @@ char* strv_join_full(char * const *l, const char *separator, const char *prefix,
         k = strlen(separator);
         m = strlen_ptr(prefix);
 
-        if (unescape_separators) /* If there separator is multi-char, we won't know how to escape it. */
+        if (escape_separator) /* If the separator was multi-char, we wouldn't know how to escape it. */
                 assert(k == 1);
 
         n = 0;
@@ -369,7 +369,7 @@ char* strv_join_full(char * const *l, const char *separator, const char *prefix,
                 if (s != l)
                         n += k;
 
-                bool needs_escaping = unescape_separators && strchr(*s, separator[0]);
+                bool needs_escaping = escape_separator && strchr(*s, *separator);
 
                 n += m + strlen(*s) * (1 + needs_escaping);
         }
@@ -386,11 +386,11 @@ char* strv_join_full(char * const *l, const char *separator, const char *prefix,
                 if (prefix)
                         e = stpcpy(e, prefix);
 
-                bool needs_escaping = unescape_separators && strchr(*s, separator[0]);
+                bool needs_escaping = escape_separator && strchr(*s, *separator);
 
                 if (needs_escaping)
                         for (size_t i = 0; (*s)[i]; i++) {
-                                if ((*s)[i] == separator[0])
+                                if ((*s)[i] == *separator)
                                         *(e++) = '\\';
                                 *(e++) = (*s)[i];
                         }

src/basic/strv.h

@@ -101,7 +101,7 @@ static inline char** strv_split_newlines(const char *s) {
  * string in the vector is an empty string. */
 int strv_split_colon_pairs(char ***t, const char *s);
 
-char* strv_join_full(char * const *l, const char *separator, const char *prefix, bool escape_separtor);
+char* strv_join_full(char * const *l, const char *separator, const char *prefix, bool escape_separator);
 static inline char *strv_join(char * const *l, const char *separator) {
         return strv_join_full(l, separator, NULL, false);
 }

src/core/manager.c

@@ -4370,71 +4370,54 @@ static int short_uid_range(const char *path) {
         return !uid_range_covers(p, n, 0, 65535);
 }
 
-char *manager_taint_string(Manager *m) {
-        _cleanup_free_ char *destination = NULL, *overflowuid = NULL, *overflowgid = NULL;
-        struct utsname uts;
-        char *buf, *e;
-        int r;
-
-        /* Returns a "taint string", e.g. "local-hwclock:var-run-bad".  Only things that are detected at
-         * runtime should be tagged here. For stuff that is set during compilation, emit a warning in the
+char* manager_taint_string(const Manager *m) {
+        /* Returns a "taint string", e.g. "local-hwclock:var-run-bad". Only things that are detected at
+         * runtime should be tagged here. For stuff that is known during compilation, emit a warning in the
          * configuration phase. */
 
         assert(m);
 
-        buf = new(char, sizeof("split-usr:"
-                               "cgroups-missing:"
-                               "cgroupsv1:"
-                               "local-hwclock:"
-                               "var-run-bad:"
-                               "overflowuid-not-65534:"
-                               "overflowgid-not-65534:"
-                               "old-kernel:"
-                               "short-uid-range:"
-                               "short-gid-range:"));
-        if (!buf)
-                return NULL;
-
-        e = buf;
-        buf[0] = 0;
+        const char* stage[11] = {};
+        size_t n = 0;
 
         if (m->taint_usr)
-                e = stpcpy(e, "split-usr:");
+                stage[n++] = "split-usr";
 
         if (access("/proc/cgroups", F_OK) < 0)
-                e = stpcpy(e, "cgroups-missing:");
+                stage[n++] = "cgroups-missing";
 
         if (cg_all_unified() == 0)
-                e = stpcpy(e, "cgroupsv1:");
+                stage[n++] = "cgroupsv1";
 
         if (clock_is_localtime(NULL) > 0)
-                e = stpcpy(e, "local-hwclock:");
+                stage[n++] = "local-hwclock";
 
-        r = readlink_malloc("/var/run", &destination);
-        if (r < 0 || !PATH_IN_SET(destination, "../run", "/run"))
-                e = stpcpy(e, "var-run-bad:");
+        _cleanup_free_ char *destination = NULL;
+        if (readlink_malloc("/var/run", &destination) < 0 ||
+            !PATH_IN_SET(destination, "../run", "/run"))
+                stage[n++] = "var-run-bad";
 
-        r = read_one_line_file("/proc/sys/kernel/overflowuid", &overflowuid);
-        if (r >= 0 && !streq(overflowuid, "65534"))
-                e = stpcpy(e, "overflowuid-not-65534:");
-        r = read_one_line_file("/proc/sys/kernel/overflowgid", &overflowgid);
-        if (r >= 0 && !streq(overflowgid, "65534"))
-                e = stpcpy(e, "overflowgid-not-65534:");
+        _cleanup_free_ char *overflowuid = NULL, *overflowgid = NULL;
+        if (read_one_line_file("/proc/sys/kernel/overflowuid", &overflowuid) >= 0 &&
+            !streq(overflowuid, "65534"))
+                stage[n++] = "overflowuid-not-65534";
+        if (read_one_line_file("/proc/sys/kernel/overflowgid", &overflowgid) >= 0 &&
+            !streq(overflowgid, "65534"))
+                stage[n++] = "overflowgid-not-65534";
 
+        struct utsname uts;
         assert_se(uname(&uts) >= 0);
         if (strverscmp_improved(uts.release, KERNEL_BASELINE_VERSION) < 0)
-                e = stpcpy(e, "old-kernel:");
+                stage[n++] = "old-kernel";
 
         if (short_uid_range("/proc/self/uid_map") > 0)
-                e = stpcpy(e, "short-uid-range:");
+                stage[n++] = "short-uid-range";
         if (short_uid_range("/proc/self/gid_map") > 0)
-                e = stpcpy(e, "short-gid-range:");
+                stage[n++] = "short-gid-range";
 
-        /* remove the last ':' */
-        if (e != buf)
-                e[-1] = 0;
+        assert(n < ELEMENTSOF(stage) - 1);  /* One extra for NULL terminator */
 
-        return buf;
+        return strv_join((char**) stage, ":");
 }
 
 void manager_ref_console(Manager *m) {

src/core/manager.h

@@ -551,7 +551,7 @@ int manager_ref_uid(Manager *m, uid_t uid, bool clean_ipc);
 void manager_unref_gid(Manager *m, gid_t gid, bool destroy_now);
 int manager_ref_gid(Manager *m, gid_t gid, bool clean_ipc);
 
-char *manager_taint_string(Manager *m);
+char* manager_taint_string(const Manager *m);
 
 void manager_ref_console(Manager *m);
 void manager_unref_console(Manager *m);

src/test/meson.build

@@ -56,6 +56,12 @@ tests += [
           libblkid],
          core_includes],
 
+        [files('test-manager.c'),
+         [libcore,
+          libshared],
+         [],
+         core_includes],
+
         [files('test-emergency-action.c'),
          [libcore,
           libshared],

src/test/test-manager.c

@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+
+#include "manager.h"
+#include "tests.h"
+
+TEST(manager_taint_string) {
+        Manager m = {};
+
+        _cleanup_free_ char *a = manager_taint_string(&m);
+        assert_se(a);
+        log_debug("taint string w/o split-usr: '%s'", a);
+        /* split-usr is the only one that is cached in Manager, so we know it's not present.
+         * The others are queried dynamically, so we'd need to duplicate the logic here
+         * to test for them. Let's do just one. */
+        assert_se(!strstr(a, "split-usr"));
+
+        if (cg_all_unified() == 0)
+                assert_se(strstr(a, "cgroupsv1"));
+        else
+                assert_se(!strstr(a, "cgroupsv1"));
+
+        m.taint_usr = true;
+        _cleanup_free_ char *b = manager_taint_string(&m);
+        assert_se(b);
+        log_debug("taint string w/ split-usr: '%s'", b);
+        assert_se(strstr(b, "split-usr"));
+}
+
+DEFINE_TEST_MAIN(LOG_DEBUG);