From 13271e2ddecabd334a0aebc2c8fdaf582a4718c0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 13 Sep 2022 04:42:38 +0900 Subject: [PATCH 1/2] udev-node: split out stack_directory_read_one() No functional changes, just refactoring. --- src/udev/udev-node.c | 126 ++++++++++++++++++++++++------------------- 1 file changed, 71 insertions(+), 55 deletions(-) diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c index 4e7dca06de..7a98b02176 100644 --- a/src/udev/udev-node.c +++ b/src/udev/udev-node.c @@ -122,6 +122,71 @@ static int node_symlink(sd_device *dev, const char *devnode, const char *slink) return 0; } +static int stack_directory_read_one(int dirfd, const char *id, bool is_symlink, char **devnode, int *priority) { + int tmp_prio, r; + + assert(dirfd >= 0); + assert(id); + assert(devnode); + assert(priority); + + if (is_symlink) { + _cleanup_free_ char *buf = NULL; + char *colon; + + /* New format. The devnode and priority can be obtained from symlink. */ + + r = readlinkat_malloc(dirfd, id, &buf); + if (r < 0) + return r; + + colon = strchr(buf, ':'); + if (!colon || colon == buf) + return -EINVAL; + + *colon = '\0'; + + r = safe_atoi(buf, &tmp_prio); + if (r < 0) + return r; + + if (*devnode && tmp_prio <= *priority) + return 0; /* Unchanged */ + + r = free_and_strdup(devnode, colon + 1); + if (r < 0) + return r; + + } else { + _cleanup_(sd_device_unrefp) sd_device *dev = NULL; + const char *val; + + /* Old format. The devnode and priority must be obtained from uevent and udev database. */ + + r = sd_device_new_from_device_id(&dev, id); + if (r < 0) + return r; + + r = device_get_devlink_priority(dev, &tmp_prio); + if (r < 0) + return r; + + if (*devnode && tmp_prio <= *priority) + return 0; /* Unchanged */ + + r = sd_device_get_devname(dev, &val); + if (r < 0) + return r; + + r = free_and_strdup(devnode, val); + if (r < 0) + return r; + } + + *priority = tmp_prio; + return 1; /* Updated */ +} + static int stack_directory_find_prioritized_devnode(sd_device *dev, const char *dirname, bool add, char **ret) { _cleanup_closedir_ DIR *dir = NULL; _cleanup_free_ char *devnode = NULL; @@ -160,8 +225,6 @@ static int stack_directory_find_prioritized_devnode(sd_device *dev, const char * return r; FOREACH_DIRENT_ALL(de, dir, break) { - int tmp_prio; - if (de->d_name[0] == '.') continue; @@ -169,61 +232,14 @@ static int stack_directory_find_prioritized_devnode(sd_device *dev, const char * if (streq(de->d_name, id)) continue; - if (de->d_type == DT_LNK) { - _cleanup_free_ char *buf = NULL; - char *colon; - - /* New format. The devnode and priority can be obtained from symlink. */ - - r = readlinkat_malloc(dirfd(dir), de->d_name, &buf); - if (r < 0) { - log_device_debug_errno(dev, r, "Failed to read symlink %s, ignoring: %m", de->d_name); - continue; - } - - colon = strchr(buf, ':'); - if (!colon || colon == buf) - continue; - - *colon = '\0'; - - if (safe_atoi(buf, &tmp_prio) < 0) - continue; - - if (devnode && tmp_prio <= priority) - continue; - - r = free_and_strdup(&devnode, colon + 1); - if (r < 0) - return r; - - } else if (de->d_type == DT_REG) { - _cleanup_(sd_device_unrefp) sd_device *tmp_dev = NULL; - const char *val; - - /* Old format. The devnode and priority must be obtained from uevent and - * udev database files. */ - - if (sd_device_new_from_device_id(&tmp_dev, de->d_name) < 0) - continue; - - if (device_get_devlink_priority(tmp_dev, &tmp_prio) < 0) - continue; - - if (devnode && tmp_prio <= priority) - continue; - - if (sd_device_get_devname(tmp_dev, &val) < 0) - continue; - - r = free_and_strdup(&devnode, val); - if (r < 0) - return r; - - } else + if (!IN_SET(de->d_type, DT_LNK, DT_REG)) continue; - priority = tmp_prio; + r = stack_directory_read_one(dirfd(dir), de->d_name, /* is_symlink = */ de->d_type == DT_LNK, &devnode, &priority); + if (r < 0) { + log_debug_errno(r, "Failed to read '%s/%s', ignoring: %m", dirname, de->d_name); + continue; + } } *ret = TAKE_PTR(devnode); From 69928b4f152d3a282a7103720ae89cdb2517ef77 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 14 Sep 2022 06:07:09 +0900 Subject: [PATCH 2/2] udev-node: do not create symlink to a non-existing device node Previously, the stack directory contains empty regular files named with device ID, and we create sd_device object from the device name. Hence, we implicitly checked the existence of the device node. However, now the files in the stack directory are symlink, and we retrieve the path to the device node and its priority from the symlink. Hence, the existence of the device node is not checked. Let's check if the device node is still exist. --- src/udev/udev-node.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c index 7a98b02176..f3da48862b 100644 --- a/src/udev/udev-node.c +++ b/src/udev/udev-node.c @@ -146,6 +146,13 @@ static int stack_directory_read_one(int dirfd, const char *id, bool is_symlink, *colon = '\0'; + /* Of course, this check is racy, but it is not necessary to be perfect. Even if the device + * node will be removed after this check, we will receive 'remove' uevent, and the invalid + * symlink will be removed during processing the event. The check is just for shortening the + * timespan that the symlink points to a non-existing device node. */ + if (access(colon + 1, F_OK) < 0) + return -errno; + r = safe_atoi(buf, &tmp_prio); if (r < 0) return r;