From 130298ba10bd1dfc0f5b22521d26ac0f2f0d4916 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 2 Jun 2021 22:46:47 +0900 Subject: [PATCH 01/17] util: expose urlsafe_base64char() --- src/basic/hexdecoct.c | 10 ++++++++++ src/basic/hexdecoct.h | 1 + src/nspawn/nspawn-network.c | 11 +---------- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/basic/hexdecoct.c b/src/basic/hexdecoct.c index a5edccad202..da1add7c76a 100644 --- a/src/basic/hexdecoct.c +++ b/src/basic/hexdecoct.c @@ -526,6 +526,16 @@ char base64char(int x) { return table[x & 63]; } +/* This is almost base64char(), but not entirely, as it uses the "url and filename safe" alphabet, + * since we don't want "/" appear in interface names (since interfaces appear in sysfs as filenames). + * See section #5 of RFC 4648. */ +char urlsafe_base64char(int x) { + static const char table[64] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "abcdefghijklmnopqrstuvwxyz" + "0123456789-_"; + return table[x & 63]; +} + int unbase64char(char c) { unsigned offset; diff --git a/src/basic/hexdecoct.h b/src/basic/hexdecoct.h index 7e2a6892c0a..4ace5b7a995 100644 --- a/src/basic/hexdecoct.h +++ b/src/basic/hexdecoct.h @@ -27,6 +27,7 @@ char base32hexchar(int x) _const_; int unbase32hexchar(char c) _const_; char base64char(int x) _const_; +char urlsafe_base64char(int x) _const_; int unbase64char(char c) _const_; char *base32hexmem(const void *p, size_t l, bool padding); diff --git a/src/nspawn/nspawn-network.c b/src/nspawn/nspawn-network.c index d6b7d8e1d89..95e4b0213b5 100644 --- a/src/nspawn/nspawn-network.c +++ b/src/nspawn/nspawn-network.c @@ -11,6 +11,7 @@ #include "alloc-util.h" #include "ether-addr-util.h" +#include "hexdecoct.h" #include "lockfile-util.h" #include "missing_network.h" #include "netif-naming-scheme.h" @@ -200,16 +201,6 @@ static int add_veth( return 0; } -/* This is almost base64char(), but not entirely, as it uses the "url and filename safe" alphabet, since we - * don't want "/" appear in interface names (since interfaces appear in sysfs as filenames). See section #5 - * of RFC 4648. */ -static char urlsafe_base64char(int x) { - static const char table[64] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" - "abcdefghijklmnopqrstuvwxyz" - "0123456789-_"; - return table[x & 63]; -} - static int shorten_ifname(char *ifname) { char new_ifname[IFNAMSIZ]; From e64943363a8dd8bd320c2b633478be8befd1af5c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 2 Jun 2021 22:33:34 +0900 Subject: [PATCH 02/17] udev: use hashed path as a filename to save devlink --- src/udev/udev-node.c | 44 +++++++++++++++++++++++++++++--------------- src/udev/udev-node.h | 2 ++ 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c index a6abaa1a9da..b3bbbaa4d81 100644 --- a/src/udev/udev-node.c +++ b/src/udev/udev-node.c @@ -7,6 +7,8 @@ #include #include +#include "sd-id128.h" + #include "alloc-util.h" #include "device-nodes.h" #include "device-private.h" @@ -15,6 +17,7 @@ #include "fd-util.h" #include "format-util.h" #include "fs-util.h" +#include "hexdecoct.h" #include "mkdir.h" #include "path-util.h" #include "selinux-util.h" @@ -27,6 +30,7 @@ #include "user-util.h" #define LINK_UPDATE_MAX_RETRIES 128 +#define UDEV_NODE_HASH_KEY SD_ID128_MAKE(b9,6a,f1,ce,40,31,44,1a,9e,19,ec,8b,ae,f3,e3,2f) static int node_symlink(sd_device *dev, const char *node, const char *slink) { _cleanup_free_ char *slink_dirname = NULL, *target = NULL; @@ -191,45 +195,54 @@ static int link_find_prioritized(sd_device *dev, bool add, const char *stackdir, return 0; } -static size_t escape_path(const char *src, char *dest, size_t size) { +size_t udev_node_escape_path(const char *src, char *dest, size_t size) { size_t i, j; + uint64_t h; assert(src); assert(dest); for (i = 0, j = 0; src[i] != '\0'; i++) { if (src[i] == '/') { - if (j+4 >= size) { - j = 0; - break; - } + if (j+4 >= size) + goto toolong; memcpy(&dest[j], "\\x2f", 4); j += 4; } else if (src[i] == '\\') { - if (j+4 >= size) { - j = 0; - break; - } + if (j+4 >= size) + goto toolong; memcpy(&dest[j], "\\x5c", 4); j += 4; } else { - if (j+1 >= size) { - j = 0; - break; - } + if (j+1 >= size) + goto toolong; dest[j] = src[i]; j++; } } dest[j] = '\0'; return j; + +toolong: + /* If the input path is too long to encode as a filename, then let's suffix with a string + * generated from the hash of the path. */ + + h = siphash24_string(src, UDEV_NODE_HASH_KEY.bytes); + + assert(size >= 12); + + for (unsigned k = 0; k <= 10; k++) + dest[size - k - 2] = urlsafe_base64char((h >> (k * 6)) & 63); + + dest[size - 1] = '\0'; + return size - 1; } /* manage "stack of names" with possibly specified device priorities */ static int link_update(sd_device *dev, const char *slink, bool add) { _cleanup_free_ char *filename = NULL, *dirname = NULL; const char *slink_name, *id; - char name_enc[PATH_MAX]; + char name_enc[NAME_MAX+1]; int i, r, retries; assert(dev); @@ -244,10 +257,11 @@ static int link_update(sd_device *dev, const char *slink, bool add) { if (r < 0) return log_device_debug_errno(dev, r, "Failed to get device id: %m"); - escape_path(slink_name, name_enc, sizeof(name_enc)); + (void) udev_node_escape_path(slink_name, name_enc, sizeof(name_enc)); dirname = path_join("/run/udev/links/", name_enc); if (!dirname) return log_oom(); + filename = path_join(dirname, id); if (!filename) return log_oom(); diff --git a/src/udev/udev-node.h b/src/udev/udev-node.h index 84c7e4567fa..2349f9c471f 100644 --- a/src/udev/udev-node.h +++ b/src/udev/udev-node.h @@ -13,3 +13,5 @@ int udev_node_add(sd_device *dev, bool apply, OrderedHashmap *seclabel_list); int udev_node_remove(sd_device *dev); int udev_node_update_old_links(sd_device *dev, sd_device *dev_old); + +size_t udev_node_escape_path(const char *src, char *dest, size_t size); From 52fde280144e829b02f170b80f8a6f5631be068a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 4 Jun 2021 03:09:08 +0900 Subject: [PATCH 03/17] test: add tests for udev_node_escape_path() --- src/udev/meson.build | 6 ++++++ src/udev/test-udev-node.c | 44 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 src/udev/test-udev-node.c diff --git a/src/udev/meson.build b/src/udev/meson.build index 53787fa1255..4e80f9bfd71 100644 --- a/src/udev/meson.build +++ b/src/udev/meson.build @@ -200,6 +200,12 @@ tests += [ [threads, libacl]], + [['src/udev/test-udev-node.c'], + [libudevd_core, + libshared], + [threads, + libacl]], + [['src/udev/test-udev-builtin.c'], [libudevd_core, libshared], diff --git a/src/udev/test-udev-node.c b/src/udev/test-udev-node.c new file mode 100644 index 00000000000..fe3dd437d5c --- /dev/null +++ b/src/udev/test-udev-node.c @@ -0,0 +1,44 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "tests.h" +#include "udev-node.h" + +static void test_udev_node_escape_path_one(const char *path, const char *expected) { + char buf[NAME_MAX+1]; + size_t r; + + r = udev_node_escape_path(path, buf, sizeof buf); + log_debug("udev_node_escape_path(%s) -> %s (expected: %s)", path, buf, expected); + assert_se(r == strlen(expected)); + assert_se(streq(buf, expected)); +} + +static void test_udev_node_escape_path(void) { + char a[NAME_MAX+1], b[NAME_MAX+1]; + + test_udev_node_escape_path_one("/disk/by-id/nvme-eui.1922908022470001001b448b44ccb9d6", "\\x2fdisk\\x2fby-id\\x2fnvme-eui.1922908022470001001b448b44ccb9d6"); + test_udev_node_escape_path_one("/disk/by-id/nvme-eui.1922908022470001001b448b44ccb9d6-part1", "\\x2fdisk\\x2fby-id\\x2fnvme-eui.1922908022470001001b448b44ccb9d6-part1"); + test_udev_node_escape_path_one("/disk/by-id/nvme-eui.1922908022470001001b448b44ccb9d6-part2", "\\x2fdisk\\x2fby-id\\x2fnvme-eui.1922908022470001001b448b44ccb9d6-part2"); + test_udev_node_escape_path_one("/disk/by-id/nvme-WDC_PC_SN720_SDAQNTW-512G-1001_192290802247", "\\x2fdisk\\x2fby-id\\x2fnvme-WDC_PC_SN720_SDAQNTW-512G-1001_192290802247"); + test_udev_node_escape_path_one("/disk/by-id/nvme-WDC_PC_SN720_SDAQNTW-512G-1001_192290802247-part1", "\\x2fdisk\\x2fby-id\\x2fnvme-WDC_PC_SN720_SDAQNTW-512G-1001_192290802247-part1"); + test_udev_node_escape_path_one("/disk/by-id/nvme-WDC_PC_SN720_SDAQNTW-512G-1001_192290802247-part2", "\\x2fdisk\\x2fby-id\\x2fnvme-WDC_PC_SN720_SDAQNTW-512G-1001_192290802247-part2"); + test_udev_node_escape_path_one("/disk/by-id/usb-Generic-_SD_MMC_20120501030900000-0:0", "\\x2fdisk\\x2fby-id\\x2fusb-Generic-_SD_MMC_20120501030900000-0:0"); + + memset(a, 'a', sizeof(a) - 1); + memcpy(a, "/disk/by-id/", strlen("/disk/by-id/")); + char_array_0(a); + + memset(b, 'a', sizeof(b) - 1); + memcpy(b, "\\x2fdisk\\x2fby-id\\x2f", strlen("\\x2fdisk\\x2fby-id\\x2f")); + strcpy(b + sizeof(b) - 12, "N3YhcCqFeID"); + + test_udev_node_escape_path_one(a, b); +} + +int main(int argc, char *argv[]) { + test_setup_logging(LOG_INFO); + + test_udev_node_escape_path(); + + return 0; +} From be322ecafbacc7d7cb7ae42932faf59f3790c36f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 2 Jun 2021 23:23:21 +0900 Subject: [PATCH 04/17] udev: refuse unsafe device symbolic link --- src/udev/udev-node.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c index b3bbbaa4d81..033ecc91f22 100644 --- a/src/udev/udev-node.c +++ b/src/udev/udev-node.c @@ -239,17 +239,25 @@ toolong: } /* manage "stack of names" with possibly specified device priorities */ -static int link_update(sd_device *dev, const char *slink, bool add) { - _cleanup_free_ char *filename = NULL, *dirname = NULL; +static int link_update(sd_device *dev, const char *slink_in, bool add) { + _cleanup_free_ char *slink = NULL, *filename = NULL, *dirname = NULL; const char *slink_name, *id; char name_enc[NAME_MAX+1]; int i, r, retries; assert(dev); - assert(slink); + assert(slink_in); + + slink = strdup(slink_in); + if (!slink) + return log_oom_debug(); + + path_simplify(slink); slink_name = path_startswith(slink, "/dev"); - if (!slink_name) + if (!slink_name || + empty_or_root(slink_name) || + !path_is_normalized(slink_name)) return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EINVAL), "Invalid symbolic link of device node: %s", slink); From 286bedd7a45b6dc82479351a2445e6b75263b47c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 2 Jun 2021 23:32:17 +0900 Subject: [PATCH 05/17] udev: logs when failed to remove saved info about devlink --- src/udev/udev-node.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c index 033ecc91f22..1702f489d1c 100644 --- a/src/udev/udev-node.c +++ b/src/udev/udev-node.c @@ -275,8 +275,10 @@ static int link_update(sd_device *dev, const char *slink_in, bool add) { return log_oom(); if (!add) { - if (unlink(filename) == 0) - (void) rmdir(dirname); + if (unlink(filename) < 0 && errno != ENOENT) + log_device_debug_errno(dev, errno, "Failed to remove %s, ignoring: %m", filename); + + (void) rmdir(dirname); } else for (;;) { _cleanup_close_ int fd = -1; From 5733bd4862aa5b9b20f2fdd967c0c0e8dd26deb7 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 2 Jun 2021 23:36:03 +0900 Subject: [PATCH 06/17] udev: use touch_file() and limit the number of trial --- src/udev/udev-node.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c index 1702f489d1c..5e556926f98 100644 --- a/src/udev/udev-node.c +++ b/src/udev/udev-node.c @@ -30,6 +30,7 @@ #include "user-util.h" #define LINK_UPDATE_MAX_RETRIES 128 +#define TOUCH_FILE_MAX_RETRIES 128 #define UDEV_NODE_HASH_KEY SD_ID128_MAKE(b9,6a,f1,ce,40,31,44,1a,9e,19,ec,8b,ae,f3,e3,2f) static int node_symlink(sd_device *dev, const char *node, const char *slink) { @@ -279,20 +280,17 @@ static int link_update(sd_device *dev, const char *slink_in, bool add) { log_device_debug_errno(dev, errno, "Failed to remove %s, ignoring: %m", filename); (void) rmdir(dirname); - } else - for (;;) { - _cleanup_close_ int fd = -1; - - r = mkdir_parents(filename, 0755); - if (!IN_SET(r, 0, -ENOENT)) - return r; - - fd = open(filename, O_WRONLY|O_CREAT|O_CLOEXEC|O_TRUNC|O_NOFOLLOW, 0444); - if (fd >= 0) + } else { + for (unsigned j = 0; j < TOUCH_FILE_MAX_RETRIES; j++) { + /* This may fail with -ENOENT when the parent directory is removed during + * creating the file by another udevd worker. */ + r = touch_file(filename, /* parents= */ true, USEC_INFINITY, UID_INVALID, GID_INVALID, 0444); + if (r != -ENOENT) break; - if (errno != ENOENT) - return -errno; } + if (r < 0) + return log_device_debug_errno(dev, r, "Failed to create %s: %m", filename); + } /* If the database entry is not written yet we will just do one iteration and possibly wrong symlink * will be fixed in the second invocation. */ From e91454231ba16caaad02e90be04b6bb15df24551 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 2 Jun 2021 23:52:46 +0900 Subject: [PATCH 07/17] udev: do not try to remove /dev --- src/udev/udev-node.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c index 5e556926f98..14a5e8126f4 100644 --- a/src/udev/udev-node.c +++ b/src/udev/udev-node.c @@ -308,7 +308,7 @@ static int link_update(sd_device *dev, const char *slink_in, bool add) { if (r == -ENOENT) { log_device_debug(dev, "No reference left, removing '%s'", slink); if (unlink(slink) == 0) - (void) rmdir_parents(slink, "/"); + (void) rmdir_parents(slink, "/dev"); break; } else if (r < 0) From a33dc87e420524da17575005023ab582cb6ecbc6 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 3 Jun 2021 00:13:55 +0900 Subject: [PATCH 08/17] udev: logs if failed to remove devlink --- src/udev/udev-node.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c index 14a5e8126f4..6c5694f55bd 100644 --- a/src/udev/udev-node.c +++ b/src/udev/udev-node.c @@ -306,10 +306,12 @@ static int link_update(sd_device *dev, const char *slink_in, bool add) { r = link_find_prioritized(dev, add, dirname, &target); if (r == -ENOENT) { - log_device_debug(dev, "No reference left, removing '%s'", slink); - if (unlink(slink) == 0) - (void) rmdir_parents(slink, "/dev"); + log_device_debug(dev, "No reference left for '%s', removing", slink); + if (unlink(slink) < 0 && errno != ENOENT) + log_device_debug_errno(dev, errno, "Failed to remove '%s', ignoring: %m", slink); + + (void) rmdir_parents(slink, "/dev"); break; } else if (r < 0) return log_device_error_errno(dev, r, "Failed to determine highest priority symlink: %m"); @@ -590,7 +592,8 @@ int udev_node_remove(sd_device *dev) { return log_device_debug_errno(dev, r, "Failed to get device path: %m"); /* remove /dev/{block,char}/$major:$minor */ - (void) unlink(filename); + if (unlink(filename) < 0 && errno != ENOENT) + return log_device_debug_errno(dev, errno, "Failed to remove '%s': %m", filename); return 0; } From e7f3b33e70b0ab21c298f15cee77a7da402262e8 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 2 Jun 2021 23:56:04 +0900 Subject: [PATCH 09/17] udev: slightly update log message and adjust log level --- src/udev/udev-node.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c index 6c5694f55bd..f06eadc7bc3 100644 --- a/src/udev/udev-node.c +++ b/src/udev/udev-node.c @@ -269,11 +269,11 @@ static int link_update(sd_device *dev, const char *slink_in, bool add) { (void) udev_node_escape_path(slink_name, name_enc, sizeof(name_enc)); dirname = path_join("/run/udev/links/", name_enc); if (!dirname) - return log_oom(); + return log_oom_debug(); filename = path_join(dirname, id); if (!filename) - return log_oom(); + return log_oom_debug(); if (!add) { if (unlink(filename) < 0 && errno != ENOENT) @@ -302,7 +302,7 @@ static int link_update(sd_device *dev, const char *slink_in, bool add) { r = stat(dirname, &st1); if (r < 0 && errno != ENOENT) - return -errno; + return log_device_debug_errno(dev, errno, "Failed to stat %s: %m", dirname); r = link_find_prioritized(dev, add, dirname, &target); if (r == -ENOENT) { @@ -314,7 +314,7 @@ static int link_update(sd_device *dev, const char *slink_in, bool add) { (void) rmdir_parents(slink, "/dev"); break; } else if (r < 0) - return log_device_error_errno(dev, r, "Failed to determine highest priority symlink: %m"); + return log_device_debug_errno(dev, r, "Failed to determine highest priority for symlink '%s': %m", slink); r = node_symlink(dev, target, slink); if (r < 0) { @@ -330,7 +330,7 @@ static int link_update(sd_device *dev, const char *slink_in, bool add) { if ((st1.st_mode & S_IFMT) != 0) { r = stat(dirname, &st2); if (r < 0 && errno != ENOENT) - return -errno; + return log_device_debug_errno(dev, errno, "Failed to stat %s: %m", dirname); if (stat_inode_unmodified(&st1, &st2)) break; @@ -341,16 +341,12 @@ static int link_update(sd_device *dev, const char *slink_in, bool add) { } int udev_node_update_old_links(sd_device *dev, sd_device *dev_old) { - const char *name, *devpath; + const char *name; int r; assert(dev); assert(dev_old); - r = sd_device_get_devpath(dev, &devpath); - if (r < 0) - return log_device_debug_errno(dev, r, "Failed to get devpath: %m"); - /* update possible left-over symlinks */ FOREACH_DEVICE_DEVLINK(dev_old, name) { const char *name_current; @@ -366,8 +362,10 @@ int udev_node_update_old_links(sd_device *dev, sd_device *dev_old) { if (found) continue; - log_device_debug(dev, "Updating old name, '%s' no longer belonging to '%s'", - name, devpath); + log_device_debug(dev, + "Updating old device symlink '%s', which is no longer belonging to this device.", + name); + r = link_update(dev, name, false); if (r < 0) log_device_warning_errno(dev, r, From d2b50631fbf9aeb79a0b3bc93793f7a925e320d3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 3 Jun 2021 00:10:52 +0900 Subject: [PATCH 10/17] udev: make link_find_prioritized() return 0, 1, or negative errno --- src/udev/udev-node.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c index f06eadc7bc3..2232b1dc8c1 100644 --- a/src/udev/udev-node.c +++ b/src/udev/udev-node.c @@ -113,17 +113,20 @@ static int node_symlink(sd_device *dev, const char *node, const char *slink) { return r; } -/* find device node of device with highest priority */ static int link_find_prioritized(sd_device *dev, bool add, const char *stackdir, char **ret) { _cleanup_closedir_ DIR *dir = NULL; _cleanup_free_ char *target = NULL; struct dirent *dent; int r, priority = 0; + const char *id; - assert(!add || dev); + assert(dev); assert(stackdir); assert(ret); + /* Find device node of device with highest priority. This returns 1 if a device found, 0 if no + * device found, or a negative errno. */ + if (add) { const char *devnode; @@ -142,17 +145,21 @@ static int link_find_prioritized(sd_device *dev, bool add, const char *stackdir, dir = opendir(stackdir); if (!dir) { - if (target) { + if (errno == ENOENT) { *ret = TAKE_PTR(target); - return 0; + return !!*ret; } return -errno; } + r = device_get_device_id(dev, &id); + if (r < 0) + return r; + FOREACH_DIRENT_ALL(dent, dir, break) { _cleanup_(sd_device_unrefp) sd_device *dev_db = NULL; - const char *devnode, *id; + const char *devnode; int db_prio = 0; if (dent->d_name[0] == '\0') @@ -162,9 +169,6 @@ static int link_find_prioritized(sd_device *dev, bool add, const char *stackdir, log_device_debug(dev, "Found '%s' claiming '%s'", dent->d_name, stackdir); - if (device_get_device_id(dev, &id) < 0) - continue; - /* did we find ourself? */ if (streq(dent->d_name, id)) continue; @@ -189,11 +193,8 @@ static int link_find_prioritized(sd_device *dev, bool add, const char *stackdir, priority = db_prio; } - if (!target) - return -ENOENT; - *ret = TAKE_PTR(target); - return 0; + return !!*ret; } size_t udev_node_escape_path(const char *src, char *dest, size_t size) { @@ -305,7 +306,9 @@ static int link_update(sd_device *dev, const char *slink_in, bool add) { return log_device_debug_errno(dev, errno, "Failed to stat %s: %m", dirname); r = link_find_prioritized(dev, add, dirname, &target); - if (r == -ENOENT) { + if (r < 0) + return log_device_debug_errno(dev, r, "Failed to determine highest priority for symlink '%s': %m", slink); + if (r == 0) { log_device_debug(dev, "No reference left for '%s', removing", slink); if (unlink(slink) < 0 && errno != ENOENT) @@ -313,8 +316,7 @@ static int link_update(sd_device *dev, const char *slink_in, bool add) { (void) rmdir_parents(slink, "/dev"); break; - } else if (r < 0) - return log_device_debug_errno(dev, r, "Failed to determine highest priority for symlink '%s': %m", slink); + } r = node_symlink(dev, target, slink); if (r < 0) { From f3b393e951450a5983ec91f47451dc561a0d977d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 3 Jun 2021 00:44:39 +0900 Subject: [PATCH 11/17] udev: refuse to create device symlink when a non-symlink file already exists --- src/udev/udev-node.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c index 2232b1dc8c1..d5a341baaac 100644 --- a/src/udev/udev-node.c +++ b/src/udev/udev-node.c @@ -52,21 +52,22 @@ static int node_symlink(sd_device *dev, const char *node, const char *slink) { if (r < 0) return log_device_error_errno(dev, r, "Failed to get relative path from '%s' to '%s': %m", slink, node); - /* preserve link with correct target, do not replace node of other device */ - if (lstat(slink, &stats) == 0) { - if (S_ISBLK(stats.st_mode) || S_ISCHR(stats.st_mode)) - return log_device_error_errno(dev, SYNTHETIC_ERRNO(EOPNOTSUPP), - "Conflicting device node '%s' found, link to '%s' will not be created.", slink, node); - else if (S_ISLNK(stats.st_mode)) { - _cleanup_free_ char *buf = NULL; + if (lstat(slink, &stats) >= 0) { + _cleanup_free_ char *buf = NULL; - if (readlink_malloc(slink, &buf) >= 0 && - streq(target, buf)) { - log_device_debug(dev, "Preserve already existing symlink '%s' to '%s'", slink, target); - (void) label_fix(slink, LABEL_IGNORE_ENOENT); - (void) utimensat(AT_FDCWD, slink, NULL, AT_SYMLINK_NOFOLLOW); - return 0; - } + if (!S_ISLNK(stats.st_mode)) + return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EEXIST), + "Conflicting inode '%s' found, link to '%s' will not be created.", slink, node); + + if (readlink_malloc(slink, &buf) >= 0 && + streq(target, buf)) { + /* preserve link with correct target, do not replace node of other device */ + log_device_debug(dev, "Preserve already existing symlink '%s' to '%s'", slink, target); + + (void) label_fix(slink, LABEL_IGNORE_ENOENT); + (void) utimensat(AT_FDCWD, slink, NULL, AT_SYMLINK_NOFOLLOW); + + return 0; } } else { log_device_debug(dev, "Creating symlink '%s' to '%s'", slink, target); From 1ddfb6cf29ad7bd4d2ebcc5049c7226579a8e2fb Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 3 Jun 2021 00:53:58 +0900 Subject: [PATCH 12/17] udev: use path_extract_directory() and path_equal() --- src/udev/udev-node.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c index d5a341baaac..b4f2a896686 100644 --- a/src/udev/udev-node.c +++ b/src/udev/udev-node.c @@ -43,9 +43,9 @@ static int node_symlink(sd_device *dev, const char *node, const char *slink) { assert(node); assert(slink); - slink_dirname = dirname_malloc(slink); - if (!slink_dirname) - return log_oom(); + r = path_extract_directory(slink, &slink_dirname); + if (r < 0) + return log_device_debug_errno(dev, r, "Failed to get parent directory of '%s': %m", slink); /* use relative link */ r = path_make_relative(slink_dirname, node, &target); @@ -60,7 +60,7 @@ static int node_symlink(sd_device *dev, const char *node, const char *slink) { "Conflicting inode '%s' found, link to '%s' will not be created.", slink, node); if (readlink_malloc(slink, &buf) >= 0 && - streq(target, buf)) { + path_equal(target, buf)) { /* preserve link with correct target, do not replace node of other device */ log_device_debug(dev, "Preserve already existing symlink '%s' to '%s'", slink, target); From 5802d4ea03e9f58a93dc862a848e9e38f800ef85 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 3 Jun 2021 01:07:45 +0900 Subject: [PATCH 13/17] udev: extract same logic of creating device symlink This also limits the number of trial. --- src/udev/udev-node.c | 67 +++++++++++++++++++++++++++----------------- 1 file changed, 41 insertions(+), 26 deletions(-) diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c index b4f2a896686..b9471397148 100644 --- a/src/udev/udev-node.c +++ b/src/udev/udev-node.c @@ -29,10 +29,37 @@ #include "udev-node.h" #include "user-util.h" +#define CREATE_LINK_MAX_RETRIES 128 #define LINK_UPDATE_MAX_RETRIES 128 #define TOUCH_FILE_MAX_RETRIES 128 #define UDEV_NODE_HASH_KEY SD_ID128_MAKE(b9,6a,f1,ce,40,31,44,1a,9e,19,ec,8b,ae,f3,e3,2f) +static int create_symlink(const char *target, const char *slink) { + int r; + + assert(target); + assert(slink); + + for (unsigned i = 0; i < CREATE_LINK_MAX_RETRIES; i++) { + r = mkdir_parents_label(slink, 0755); + if (r == -ENOENT) + continue; + if (r < 0) + return r; + + mac_selinux_create_file_prepare(slink, S_IFLNK); + if (symlink(target, slink) < 0) + r = -errno; + else + r = 0; + mac_selinux_create_file_clear(); + if (r != -ENOENT) + return r; + } + + return r; +} + static int node_symlink(sd_device *dev, const char *node, const char *slink) { _cleanup_free_ char *slink_dirname = NULL, *target = NULL; const char *id, *slink_tmp; @@ -71,47 +98,35 @@ static int node_symlink(sd_device *dev, const char *node, const char *slink) { } } else { log_device_debug(dev, "Creating symlink '%s' to '%s'", slink, target); - do { - r = mkdir_parents_label(slink, 0755); - if (!IN_SET(r, 0, -ENOENT)) - break; - mac_selinux_create_file_prepare(slink, S_IFLNK); - if (symlink(target, slink) < 0) - r = -errno; - mac_selinux_create_file_clear(); - } while (r == -ENOENT); - if (r == 0) + + r = create_symlink(target, slink); + if (r >= 0) return 0; - if (r < 0) - log_device_debug_errno(dev, r, "Failed to create symlink '%s' to '%s', trying to replace '%s': %m", slink, target, slink); + + log_device_debug_errno(dev, r, "Failed to create symlink '%s' to '%s', trying to replace '%s': %m", slink, target, slink); } log_device_debug(dev, "Atomically replace '%s'", slink); + r = device_get_device_id(dev, &id); if (r < 0) return log_device_error_errno(dev, r, "Failed to get device id: %m"); slink_tmp = strjoina(slink, ".tmp-", id); + (void) unlink(slink_tmp); - do { - r = mkdir_parents_label(slink_tmp, 0755); - if (!IN_SET(r, 0, -ENOENT)) - break; - mac_selinux_create_file_prepare(slink_tmp, S_IFLNK); - if (symlink(target, slink_tmp) < 0) - r = -errno; - mac_selinux_create_file_clear(); - } while (r == -ENOENT); + + r = create_symlink(target, slink_tmp); if (r < 0) - return log_device_error_errno(dev, r, "Failed to create symlink '%s' to '%s': %m", slink_tmp, target); + return log_device_debug_errno(dev, r, "Failed to create symlink '%s' to '%s': %m", slink_tmp, target); if (rename(slink_tmp, slink) < 0) { r = log_device_error_errno(dev, errno, "Failed to rename '%s' to '%s': %m", slink_tmp, slink); (void) unlink(slink_tmp); - } else - /* Tell caller that we replaced already existing symlink. */ - r = 1; + return r; + } - return r; + /* Tell caller that we replaced already existing symlink. */ + return 1; } static int link_find_prioritized(sd_device *dev, bool add, const char *stackdir, char **ret) { From c891389a163302a5e6dbe087ba13817933958a37 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 3 Jun 2021 01:16:44 +0900 Subject: [PATCH 14/17] udev: try to create device symlink directly only when the link does not exist yet --- src/udev/udev-node.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c index b9471397148..6276a543d28 100644 --- a/src/udev/udev-node.c +++ b/src/udev/udev-node.c @@ -96,7 +96,7 @@ static int node_symlink(sd_device *dev, const char *node, const char *slink) { return 0; } - } else { + } else if (errno == ENOENT) { log_device_debug(dev, "Creating symlink '%s' to '%s'", slink, target); r = create_symlink(target, slink); @@ -104,7 +104,8 @@ static int node_symlink(sd_device *dev, const char *node, const char *slink) { return 0; log_device_debug_errno(dev, r, "Failed to create symlink '%s' to '%s', trying to replace '%s': %m", slink, target, slink); - } + } else + return log_device_debug_errno(dev, errno, "Failed to lstat() '%s': %m", slink); log_device_debug(dev, "Atomically replace '%s'", slink); From ebb0a0155d8bd722b439a947f11e24114d024909 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 3 Jun 2021 01:27:39 +0900 Subject: [PATCH 15/17] udev: warn and propagate error in creating device symlink Also, this makes the file in /run/udev/links/ is kept on failure, as the target of the symbolic link may be belonging to another device. --- src/udev/udev-node.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c index 6276a543d28..d90c933988c 100644 --- a/src/udev/udev-node.c +++ b/src/udev/udev-node.c @@ -77,7 +77,7 @@ static int node_symlink(sd_device *dev, const char *node, const char *slink) { /* use relative link */ r = path_make_relative(slink_dirname, node, &target); if (r < 0) - return log_device_error_errno(dev, r, "Failed to get relative path from '%s' to '%s': %m", slink, node); + return log_device_debug_errno(dev, r, "Failed to get relative path from '%s' to '%s': %m", slink, node); if (lstat(slink, &stats) >= 0) { _cleanup_free_ char *buf = NULL; @@ -111,7 +111,7 @@ static int node_symlink(sd_device *dev, const char *node, const char *slink) { r = device_get_device_id(dev, &id); if (r < 0) - return log_device_error_errno(dev, r, "Failed to get device id: %m"); + return log_device_debug_errno(dev, r, "Failed to get device id: %m"); slink_tmp = strjoina(slink, ".tmp-", id); (void) unlink(slink_tmp); @@ -121,7 +121,7 @@ static int node_symlink(sd_device *dev, const char *node, const char *slink) { return log_device_debug_errno(dev, r, "Failed to create symlink '%s' to '%s': %m", slink_tmp, target); if (rename(slink_tmp, slink) < 0) { - r = log_device_error_errno(dev, errno, "Failed to rename '%s' to '%s': %m", slink_tmp, slink); + r = log_device_debug_errno(dev, errno, "Failed to rename '%s' to '%s': %m", slink_tmp, slink); (void) unlink(slink_tmp); return r; } @@ -336,10 +336,9 @@ static int link_update(sd_device *dev, const char *slink_in, bool add) { } r = node_symlink(dev, target, slink); - if (r < 0) { - (void) unlink(filename); - break; - } else if (r == 1) + if (r < 0) + return r; + if (r == 1) /* We have replaced already existing symlink, possibly there is some other device trying * to claim the same symlink. Let's do one more iteration to give us a chance to fix * the error if other device actually claims the symlink with higher priority. */ @@ -569,13 +568,6 @@ int udev_node_add(sd_device *dev, bool apply, if (r < 0) return r; - r = xsprintf_dev_num_path_from_sd_device(dev, &filename); - if (r < 0) - return log_device_debug_errno(dev, r, "Failed to get device path: %m"); - - /* always add /dev/{block,char}/$major:$minor */ - (void) node_symlink(dev, devnode, filename); - /* create/update symlinks, add symlinks to name index */ FOREACH_DEVICE_DEVLINK(dev, devlink) { r = link_update(dev, devlink, true); @@ -585,6 +577,15 @@ int udev_node_add(sd_device *dev, bool apply, devlink); } + r = xsprintf_dev_num_path_from_sd_device(dev, &filename); + if (r < 0) + return log_device_debug_errno(dev, r, "Failed to get device path: %m"); + + /* always add /dev/{block,char}/$major:$minor */ + r = node_symlink(dev, devnode, filename); + if (r < 0) + return log_device_warning_errno(dev, r, "Failed to create device symlink '%s': %m", filename); + return 0; } From 902b4c677eb8a15df7e3a7ca0968812ad1b99827 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 3 Jun 2021 03:12:53 +0900 Subject: [PATCH 16/17] util: move device-node.[ch] to shared --- src/basic/btrfs-util.c | 1 - src/basic/meson.build | 2 -- src/{basic => shared}/device-nodes.c | 0 src/{basic => shared}/device-nodes.h | 0 src/shared/meson.build | 2 ++ 5 files changed, 2 insertions(+), 3 deletions(-) rename src/{basic => shared}/device-nodes.c (100%) rename src/{basic => shared}/device-nodes.h (100%) diff --git a/src/basic/btrfs-util.c b/src/basic/btrfs-util.c index bc1fb8d29d0..adf9d03eb90 100644 --- a/src/basic/btrfs-util.c +++ b/src/basic/btrfs-util.c @@ -19,7 +19,6 @@ #include "btrfs-util.h" #include "chattr-util.h" #include "copy.h" -#include "device-nodes.h" #include "fd-util.h" #include "fileio.h" #include "fs-util.h" diff --git a/src/basic/meson.build b/src/basic/meson.build index 18084875bdf..034f30093d7 100644 --- a/src/basic/meson.build +++ b/src/basic/meson.build @@ -38,8 +38,6 @@ basic_sources = files(''' creds-util.c creds-util.h def.h - device-nodes.c - device-nodes.h dirent-util.c dirent-util.h dlfcn-util.c diff --git a/src/basic/device-nodes.c b/src/shared/device-nodes.c similarity index 100% rename from src/basic/device-nodes.c rename to src/shared/device-nodes.c diff --git a/src/basic/device-nodes.h b/src/shared/device-nodes.h similarity index 100% rename from src/basic/device-nodes.h rename to src/shared/device-nodes.h diff --git a/src/shared/meson.build b/src/shared/meson.build index 6ecdf576ab8..8d9cb323559 100644 --- a/src/shared/meson.build +++ b/src/shared/meson.build @@ -80,6 +80,8 @@ shared_sources = files(''' daemon-util.h dev-setup.c dev-setup.h + device-nodes.c + device-nodes.h devnode-acl.h discover-image.c discover-image.h From 78d8eae9a58c72ee6dfad13ceb4698c38a07764f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 3 Jun 2021 03:22:16 +0900 Subject: [PATCH 17/17] util: drop DEV_NUM_PATH_MAX and xsprintf_dev_num_path() --- src/shared/device-nodes.h | 9 --------- src/test/test-device-nodes.c | 2 +- src/udev/udev-node.c | 13 +------------ 3 files changed, 2 insertions(+), 22 deletions(-) diff --git a/src/shared/device-nodes.h b/src/shared/device-nodes.h index 9e5c79f67d2..a8b25643149 100644 --- a/src/shared/device-nodes.h +++ b/src/shared/device-nodes.h @@ -2,15 +2,6 @@ #pragma once #include -#include - -#include "macro.h" -#include "stdio-util.h" int encode_devnode_name(const char *str, char *str_enc, size_t len); int allow_listed_char_for_devnode(char c, const char *additional); - -#define DEV_NUM_PATH_MAX \ - (STRLEN("/dev/block/") + DECIMAL_STR_MAX(dev_t) + 1 + DECIMAL_STR_MAX(dev_t)) -#define xsprintf_dev_num_path(buf, type, devno) \ - xsprintf(buf, "/dev/%s/%u:%u", type, major(devno), minor(devno)) diff --git a/src/test/test-device-nodes.c b/src/test/test-device-nodes.c index c914d78324f..7ba05b53e97 100644 --- a/src/test/test-device-nodes.c +++ b/src/test/test-device-nodes.c @@ -1,11 +1,11 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ +#include #include #include "alloc-util.h" #include "device-nodes.h" #include "string-util.h" -#include "util.h" /* helpers for test_encode_devnode_name */ static char *do_encode_string(const char *in) { diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c index d90c933988c..a56084de7eb 100644 --- a/src/udev/udev-node.c +++ b/src/udev/udev-node.c @@ -10,7 +10,6 @@ #include "sd-id128.h" #include "alloc-util.h" -#include "device-nodes.h" #include "device-private.h" #include "device-util.h" #include "dirent-util.h" @@ -517,7 +516,6 @@ static int node_permissions_apply(sd_device *dev, bool apply_mac, } static int xsprintf_dev_num_path_from_sd_device(sd_device *dev, char **ret) { - char filename[DEV_NUM_PATH_MAX], *s; const char *subsystem; dev_t devnum; int r; @@ -532,16 +530,7 @@ static int xsprintf_dev_num_path_from_sd_device(sd_device *dev, char **ret) { if (r < 0) return r; - xsprintf_dev_num_path(filename, - streq(subsystem, "block") ? "block" : "char", - devnum); - - s = strdup(filename); - if (!s) - return -ENOMEM; - - *ret = s; - return 0; + return device_path_make_major_minor(streq(subsystem, "block") ? S_IFBLK : S_IFCHR, devnum, ret); } int udev_node_add(sd_device *dev, bool apply,