From: Michael Biebl Date: Sat, 25 Sep 2021 19:08:36 +0000 (+0200) Subject: Revert "udev: make algorithm that selects highest priority devlink less susceptible... X-Git-Tag: archive/raspbian/247.3-7+rpi1^2~1 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=058128a267044ece04e549e7fef8ea062e19c560;p=systemd.git Revert "udev: make algorithm that selects highest priority devlink less susceptible to race conditions" This reverts commit 30f6dce62cb3a738b20253f2192270607c31b55b. Gbp-Pq: Topic debian Gbp-Pq: Name Revert-udev-make-algorithm-that-selects-highest-priority-.patch --- diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index 5159d19a..9cf5190f 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -1041,13 +1041,6 @@ int udev_event_execute_rules(UdevEvent *event, if (r < 0) return log_device_debug_errno(dev, r, "Failed to update database under /run/udev/data/: %m"); - /* Yes, we run update_devnode() twice, because in the first invocation, that is before update of udev database, - * it could happen that two contenders are replacing each other's symlink. Hence we run it again to make sure - * symlinks point to devices that claim them with the highest priority. */ - r = update_devnode(event); - if (r < 0) - return r; - device_set_is_initialized(dev); return 0; diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c index bde18f75..9d4b7d91 100644 --- a/src/udev/udev-node.c +++ b/src/udev/udev-node.c @@ -20,15 +20,12 @@ #include "path-util.h" #include "selinux-util.h" #include "smack-util.h" -#include "stat-util.h" #include "stdio-util.h" #include "string-util.h" #include "strxcpyx.h" #include "udev-node.h" #include "user-util.h" -#define LINK_UPDATE_MAX_RETRIES 128 - static int node_symlink(sd_device *dev, const char *node, const char *slink) { _cleanup_free_ char *slink_dirname = NULL, *target = NULL; const char *id_filename, *slink_tmp; @@ -102,9 +99,7 @@ static int node_symlink(sd_device *dev, const char *node, const char *slink) { 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; } @@ -197,7 +192,7 @@ static int link_update(sd_device *dev, const char *slink, bool add) { _cleanup_free_ char *target = NULL, *filename = NULL, *dirname = NULL; char name_enc[PATH_MAX]; const char *id_filename; - int i, r, retries; + int r; assert(dev); assert(slink); @@ -217,6 +212,14 @@ static int link_update(sd_device *dev, const char *slink, bool add) { if (!add && unlink(filename) == 0) (void) rmdir(dirname); + r = link_find_prioritized(dev, add, dirname, &target); + if (r < 0) { + log_device_debug(dev, "No reference left, removing '%s'", slink); + if (unlink(slink) == 0) + (void) rmdir_parents(slink, "/"); + } else + (void) node_symlink(dev, target, slink); + if (add) do { _cleanup_close_ int fd = -1; @@ -229,49 +232,7 @@ static int link_update(sd_device *dev, const char *slink, bool add) { r = -errno; } while (r == -ENOENT); - /* 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. */ - retries = sd_device_get_is_initialized(dev) > 0 ? LINK_UPDATE_MAX_RETRIES : 1; - - for (i = 0; i < retries; i++) { - struct stat st1 = {}, st2 = {}; - - r = stat(dirname, &st1); - if (r < 0 && errno != ENOENT) - return -errno; - - 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, "/"); - - break; - } else if (r < 0) - return log_device_error_errno(dev, r, "Failed to determine highest priority symlink: %m"); - - r = node_symlink(dev, target, slink); - if (r < 0) { - (void) unlink(filename); - break; - } else 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. */ - continue; - - /* Skip the second stat() if the first failed, stat_inode_unmodified() would return false regardless. */ - if ((st1.st_mode & S_IFMT) != 0) { - r = stat(dirname, &st2); - if (r < 0 && errno != ENOENT) - return -errno; - - if (stat_inode_unmodified(&st1, &st2)) - break; - } - } - - return i < LINK_UPDATE_MAX_RETRIES ? 0 : -ELOOP; + return r; } int udev_node_update_old_links(sd_device *dev, sd_device *dev_old) { @@ -490,11 +451,8 @@ int udev_node_add(sd_device *dev, bool apply, (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); - if (r < 0) - log_device_info_errno(dev, r, "Failed to update device symlinks: %m"); - } + FOREACH_DEVICE_DEVLINK(dev, devlink) + (void) link_update(dev, devlink, true); return 0; } @@ -507,11 +465,8 @@ int udev_node_remove(sd_device *dev) { assert(dev); /* remove/update symlinks, remove symlinks from name index */ - FOREACH_DEVICE_DEVLINK(dev, devlink) { - r = link_update(dev, devlink, false); - if (r < 0) - log_device_info_errno(dev, r, "Failed to update device symlinks: %m"); - } + FOREACH_DEVICE_DEVLINK(dev, devlink) + (void) link_update(dev, devlink, false); r = xsprintf_dev_num_path_from_sd_device(dev, &filename); if (r < 0)