Merge pull request #22759 from msekletar/issue-18077-long-sysfs-paths-hashing

Create "hashed" unit names from long paths
2022-04-13 22:21:38 +02:00
parent bd1f75d5fb b26f4f0028
commit 1fde09b3b4
7 changed files with 147 additions and 18 deletions
@@ -10,17 +10,18 @@
 #include "string-util-fundamental.h"

 /* What is interpreted as whitespace? */
-#define WHITESPACE        " \t\n\r"
-#define NEWLINE           "\n\r"
-#define QUOTES            "\"\'"
-#define COMMENTS          "#;"
-#define GLOB_CHARS        "*?["
-#define DIGITS            "0123456789"
-#define LOWERCASE_LETTERS "abcdefghijklmnopqrstuvwxyz"
-#define UPPERCASE_LETTERS "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
-#define LETTERS           LOWERCASE_LETTERS UPPERCASE_LETTERS
-#define ALPHANUMERICAL    LETTERS DIGITS
-#define HEXDIGITS         DIGITS "abcdefABCDEF"
+#define WHITESPACE          " \t\n\r"
+#define NEWLINE             "\n\r"
+#define QUOTES              "\"\'"
+#define COMMENTS            "#;"
+#define GLOB_CHARS          "*?["
+#define DIGITS              "0123456789"
+#define LOWERCASE_LETTERS   "abcdefghijklmnopqrstuvwxyz"
+#define UPPERCASE_LETTERS   "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+#define LETTERS             LOWERCASE_LETTERS UPPERCASE_LETTERS
+#define ALPHANUMERICAL      LETTERS DIGITS
+#define HEXDIGITS           DIGITS "abcdefABCDEF"
+#define LOWERCASE_HEXDIGITS DIGITS "abcdef"

 static inline char* strstr_ptr(const char *haystack, const char *needle) {
        if (!haystack || !needle)
@@ -5,12 +5,17 @@
 #include <stdint.h>
 #include <stdlib.h>

+#include "sd-id128.h"
+
 #include "alloc-util.h"
 #include "glob-util.h"
 #include "hexdecoct.h"
 #include "memory-util.h"
 #include "path-util.h"
+#include "random-util.h"
+#include "sparse-endian.h"
 #include "special.h"
+#include "stdio-util.h"
 #include "string-util.h"
 #include "strv.h"
 #include "unit-name.h"
@@ -31,6 +36,9 @@
        VALID_CHARS_WITH_AT                     \
        "[]!-*?"

+#define LONG_UNIT_NAME_HASH_KEY SD_ID128_MAKE(ec,f2,37,fb,58,32,4a,32,84,9f,06,9b,0d,21,eb,9a)
+#define UNIT_NAME_HASH_LENGTH_CHARS 16
+
 bool unit_name_is_valid(const char *n, UnitNameFlags flags) {
        const char *e, *i, *at;

@@ -507,6 +515,68 @@ int unit_name_template(const char *f, char **ret) {
        return 0;
 }

+bool unit_name_is_hashed(const char *name) {
+        char *s;
+
+        if (!unit_name_is_valid(name, UNIT_NAME_PLAIN))
+                return false;
+
+        assert_se(s = strrchr(name, '.'));
+
+        if (s - name < UNIT_NAME_HASH_LENGTH_CHARS + 1)
+                return false;
+
+        s -= UNIT_NAME_HASH_LENGTH_CHARS;
+        if (s[-1] != '_')
+                return false;
+
+        for (size_t i = 0; i < UNIT_NAME_HASH_LENGTH_CHARS; i++)
+                if (!strchr(LOWERCASE_HEXDIGITS, s[i]))
+                        return false;
+
+        return true;
+}
+
+int unit_name_hash_long(const char *name, char **ret) {
+        _cleanup_free_ char *n = NULL, *hash = NULL;
+        char *suffix;
+        le64_t h;
+        size_t len;
+
+        if (strlen(name) < UNIT_NAME_MAX)
+                return -EMSGSIZE;
+
+        suffix = strrchr(name, '.');
+        if (!suffix)
+                return -EINVAL;
+
+        if (unit_type_from_string(suffix+1) < 0)
+                return -EINVAL;
+
+        h = htole64(siphash24_string(name, LONG_UNIT_NAME_HASH_KEY.bytes));
+
+        hash = hexmem(&h, sizeof(h));
+        if (!hash)
+                return -ENOMEM;
+
+        assert_se(strlen(hash) == UNIT_NAME_HASH_LENGTH_CHARS);
+
+        len = UNIT_NAME_MAX - 1 - strlen(suffix+1) - UNIT_NAME_HASH_LENGTH_CHARS - 2;
+        assert(len > 0 && len < UNIT_NAME_MAX);
+
+        n = strndup(name, len);
+        if (!n)
+                return -ENOMEM;
+
+        if (!strextend(&n, "_", hash, suffix))
+                return -ENOMEM;
+        assert_se(unit_name_is_valid(n, UNIT_NAME_PLAIN));
+
+        *ret = TAKE_PTR(n);
+
+        return 0;
+}
+
 int unit_name_from_path(const char *path, const char *suffix, char **ret) {
        _cleanup_free_ char *p = NULL, *s = NULL;
        int r;
@@ -526,8 +596,17 @@ int unit_name_from_path(const char *path, const char *suffix, char **ret) {
        if (!s)
                return -ENOMEM;

-        if (strlen(s) >= UNIT_NAME_MAX) /* Return a slightly more descriptive error for this specific condition */
-                return -ENAMETOOLONG;
+        if (strlen(s) >= UNIT_NAME_MAX) {
+                _cleanup_free_ char *n = NULL;
+
+                log_debug("Unit name \"%s\" too long, falling back to hashed unit name.", s);
+
+                r = unit_name_hash_long(s, &n);
+                if (r < 0)
+                        return r;
+
+                free_and_replace(s, n);
+        }

        /* Refuse if this for some other reason didn't result in a valid name */
        if (!unit_name_is_valid(s, UNIT_NAME_PLAIN))
@@ -581,6 +660,9 @@ int unit_name_to_path(const char *name, char **ret) {
        if (r < 0)
                return r;

+        if (unit_name_is_hashed(name))
+                return -ENAMETOOLONG;
+
        return unit_name_path_unescape(prefix, ret);
 }

@@ -44,6 +44,9 @@ int unit_name_replace_instance(const char *f, const char *i, char **ret);

 int unit_name_template(const char *f, char **ret);

+int unit_name_hash_long(const char *name, char **ret);
+bool unit_name_is_hashed(const char *name);
+
 int unit_name_from_path(const char *path, const char *suffix, char **ret);
 int unit_name_from_path_instance(const char *prefix, const char *path, const char *suffix, char **ret);
 int unit_name_to_path(const char *name, char **ret);
@@ -622,6 +622,9 @@ static int mount_add_extras(Mount *m) {

        if (!m->where) {
                r = unit_name_to_path(u->id, &m->where);
+                if (r == -ENAMETOOLONG)
+                        log_unit_error_errno(u, r, "Failed to derive mount point path from unit name, because unit name is hashed. "
+                                                   "Set \"Where=\" in the unit file explicitly.");
                if (r < 0)
                        return r;
        }
@@ -107,6 +107,7 @@ TEST(unit_name_replace_instance) {

 static void test_unit_name_from_path_one(const char *path, const char *suffix, const char *expected, int ret) {
        _cleanup_free_ char *t = NULL;
+        int r;

        assert_se(unit_name_from_path(path, suffix, &t) == ret);
        puts(strna(t));
@@ -114,12 +115,31 @@ static void test_unit_name_from_path_one(const char *path, const char *suffix, c

        if (t) {
                _cleanup_free_ char *k = NULL;
-                assert_se(unit_name_to_path(t, &k) == 0);
+
+                /* We don't support converting hashed unit names back to paths */
+                r = unit_name_to_path(t, &k);
+                if (r == -ENAMETOOLONG)
+                        return;
+                assert(r == 0);
+
                puts(strna(k));
                assert_se(path_equal(k, empty_to_root(path)));
        }
 }

+TEST(unit_name_is_hashed) {
+        assert_se(!unit_name_is_hashed(""));
+        assert_se(!unit_name_is_hashed("foo@bar.service"));
+        assert_se(!unit_name_is_hashed("foo@.service"));
+        assert_se(unit_name_is_hashed("waldoaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa_7736d9ed33c2ec55.mount"));
+        assert_se(!unit_name_is_hashed("waldoaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa_7736D9ED33C2EC55.mount"));
+        assert_se(!unit_name_is_hashed("waldoaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!7736d9ed33c2ec55.mount"));
+        assert_se(!unit_name_is_hashed("waldoaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa_7736d9gd33c2ec55.mount"));
+        assert_se(!unit_name_is_hashed("waldoaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa_.mount"));
+        assert_se(!unit_name_is_hashed("waldoaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa_2103e1466b87f7f7@waldo.mount"));
+        assert_se(!unit_name_is_hashed("waldoaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa_2103e1466b87f7f7@.mount"));
+}
+
 TEST(unit_name_from_path) {
        test_unit_name_from_path_one("/waldo", ".mount", "waldo.mount", 0);
        test_unit_name_from_path_one("/waldo/quuix", ".mount", "waldo-quuix.mount", 0);
@@ -129,7 +149,8 @@ TEST(unit_name_from_path) {
        test_unit_name_from_path_one("///", ".mount", "-.mount", 0);
        test_unit_name_from_path_one("/foo/../bar", ".mount", NULL, -EINVAL);
        test_unit_name_from_path_one("/foo/./bar", ".mount", "foo-bar.mount", 0);
-        test_unit_name_from_path_one("/waldoaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", ".mount", NULL, -ENAMETOOLONG);
+        test_unit_name_from_path_one("/waldoaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", ".mount",
+                                     "waldoaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa_7736d9ed33c2ec55.mount", 0);
 }

 static void test_unit_name_from_path_instance_one(const char *pattern, const char *path, const char *suffix, const char *expected, int ret) {
@@ -157,7 +178,6 @@ TEST(unit_name_from_path_instance) {
        test_unit_name_from_path_instance_one("waldo", "..", ".mount", NULL, -EINVAL);
        test_unit_name_from_path_instance_one("waldo", "/foo", ".waldi", NULL, -EINVAL);
        test_unit_name_from_path_instance_one("wa--ldo", "/--", ".mount", "wa--ldo@\\x2d\\x2d.mount", 0);
-        test_unit_name_from_path_instance_one("waldoaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "/waldo", ".mount", NULL, -ENAMETOOLONG);
 }

 static void test_unit_name_to_path_one(const char *unit, const char *path, int ret) {
@@ -8,6 +8,25 @@ systemd-analyze log-target journal

 NUM_DIRS=20

+# make sure we can handle mounts at very long paths such that mount unit name must be hashed to fall within our unit name limit
+LONGPATH="$(printf "/$(printf "x%0.s" {1..255})%0.s" {1..7})"
+LONGMNT="$(systemd-escape --suffix=mount --path "$LONGPATH")"
+TS="$(date '+%H:%M:%S')"
+
+mkdir -p "$LONGPATH"
+mount -t tmpfs tmpfs "$LONGPATH"
+systemctl daemon-reload
+
+# check that unit is active(mounted)
+systemctl --no-pager show -p SubState --value "$LONGPATH" | grep -q mounted
+
+# check that relevant part of journal doesn't contain any errors related to unit
+[ "$(journalctl -b --since="$TS" --priority=err | grep -c "$LONGMNT")" = "0" ]
+
+# check that we can successfully stop the mount unit
+systemctl stop "$LONGPATH"
+rm -rf "$LONGPATH"
+
 # mount/unmount enough times to trigger the /proc/self/mountinfo parsing rate limiting

 for ((i = 0; i < NUM_DIRS; i++)); do
@@ -515,6 +515,7 @@ testcase_long_sysfs_path() {
    echo "UUID=deadbeef-dead-dead-beef-222222222222 $mpoint ext4 defaults 0 0" >>/etc/fstab
    systemctl daemon-reload
    mount "$mpoint"
+    systemctl status "$mpoint"
    test -e "$mpoint/test"
    umount "$mpoint"

@@ -525,9 +526,9 @@ testcase_long_sysfs_path() {
    udevadm settle

    logfile="$(mktemp)"
-    journalctl -b -q --no-pager -o short-monotonic -p info --grep "Device path.*vda.?' too long to fit into unit name"
+    [[ "$(journalctl -b -q --no-pager -o short-monotonic -p info --grep "Device path.*vda.?' too long to fit into unit name" | wc -l)" -eq 0 ]]
    # Make sure we don't unnecessarily spam the log
-    journalctl -b -q --no-pager -o short-monotonic -p info --grep "/sys/devices/.+/vda[0-9]?" _PID=1 + UNIT=systemd-udevd.service | tee "$logfile"
+    { journalctl -b -q --no-pager -o short-monotonic -p info --grep "/sys/devices/.+/vda[0-9]?" _PID=1 + UNIT=systemd-udevd.service || :;} | tee "$logfile"
    [[ "$(wc -l <"$logfile")" -lt 10 ]]

    : >/etc/fstab