diff --git a/src/libsystemd/sd-device/device-internal.h b/src/libsystemd/sd-device/device-internal.h index c7f9b59087..9844608157 100644 --- a/src/libsystemd/sd-device/device-internal.h +++ b/src/libsystemd/sd-device/device-internal.h @@ -67,7 +67,7 @@ struct sd_device { char *driver_subsystem; /* only set for the 'drivers' subsystem */ char *driver; - char *id_filename; + char *device_id; usec_t usec_initialized; @@ -84,9 +84,7 @@ struct sd_device { bool property_tags_outdated:1; /* need to update TAGS= or CURRENT_TAGS= property */ bool property_devlinks_outdated:1; /* need to update DEVLINKS= property */ bool properties_buf_outdated:1; /* need to reread hashmap */ - bool sysname_set:1; /* don't reread sysname */ bool subsystem_set:1; /* don't reread subsystem */ - bool driver_subsystem_set:1; /* don't reread subsystem */ bool driver_set:1; /* don't reread driver */ bool uevent_loaded:1; /* don't reread uevent */ bool db_loaded; /* don't reread db */ diff --git a/src/libsystemd/sd-device/device-private.c b/src/libsystemd/sd-device/device-private.c index 7a7204b145..e612dbe0a0 100644 --- a/src/libsystemd/sd-device/device-private.c +++ b/src/libsystemd/sd-device/device-private.c @@ -20,6 +20,7 @@ #include "parse-util.h" #include "path-util.h" #include "set.h" +#include "stdio-util.h" #include "string-table.h" #include "string-util.h" #include "strv.h" @@ -568,28 +569,142 @@ int device_get_devlink_priority(sd_device *device, int *priority) { return 0; } -int device_get_watch_handle(sd_device *device, int *handle) { +int device_get_watch_handle(sd_device *device) { + char path_wd[STRLEN("/run/udev/watch/") + DECIMAL_STR_MAX(int)]; + _cleanup_free_ char *buf = NULL; + const char *id, *path_id; + int wd, r; + + assert(device); + + if (device->watch_handle >= 0) + return device->watch_handle; + + r = device_get_device_id(device, &id); + if (r < 0) + return r; + + path_id = strjoina("/run/udev/watch/", id); + r = readlink_malloc(path_id, &buf); + if (r < 0) + return r; + + r = safe_atoi(buf, &wd); + if (r < 0) + return r; + + if (wd < 0) + return -EBADF; + + buf = mfree(buf); + xsprintf(path_wd, "/run/udev/watch/%d", wd); + r = readlink_malloc(path_wd, &buf); + if (r < 0) + return r; + + if (!streq(buf, id)) + return -EBADF; + + return device->watch_handle = wd; +} + +static void device_remove_watch_handle(sd_device *device) { + const char *id; + int wd; + + assert(device); + + /* First, remove the symlink from handle to device id. */ + wd = device_get_watch_handle(device); + if (wd >= 0) { + char path_wd[STRLEN("/run/udev/watch/") + DECIMAL_STR_MAX(int)]; + + xsprintf(path_wd, "/run/udev/watch/%d", wd); + if (unlink(path_wd) < 0 && errno != ENOENT) + log_device_debug_errno(device, errno, + "sd-device: failed to remove %s, ignoring: %m", + path_wd); + } + + /* Next, remove the symlink from device id to handle. */ + if (device_get_device_id(device, &id) >= 0) { + const char *path_id; + + path_id = strjoina("/run/udev/watch/", id); + if (unlink(path_id) < 0 && errno != ENOENT) + log_device_debug_errno(device, errno, + "sd-device: failed to remove %s, ignoring: %m", + path_id); + } + + device->watch_handle = -1; +} + +int device_set_watch_handle(sd_device *device, int wd) { + char path_wd[STRLEN("/run/udev/watch/") + DECIMAL_STR_MAX(int)]; + const char *id, *path_id; int r; assert(device); - r = device_read_db(device); + if (wd >= 0 && wd == device->watch_handle) + return 0; + + device_remove_watch_handle(device); + + if (wd < 0) + /* negative wd means that the caller requests to clear saved watch handle. */ + return 0; + + r = device_get_device_id(device, &id); if (r < 0) return r; - if (device->watch_handle < 0) - return -ENOENT; + path_id = strjoina("/run/udev/watch/", id); + xsprintf(path_wd, "/run/udev/watch/%d", wd); - if (handle) - *handle = device->watch_handle; + r = mkdir_parents(path_wd, 0755); + if (r < 0) + return r; + + if (symlink(id, path_wd) < 0) + return -errno; + + if (symlink(path_wd + STRLEN("/run/udev/watch/"), path_id) < 0) { + r = -errno; + if (unlink(path_wd) < 0 && errno != ENOENT) + log_device_debug_errno(device, errno, + "sd-device: failed to remove %s, ignoring: %m", + path_wd); + return r; + } + + device->watch_handle = wd; return 0; } -void device_set_watch_handle(sd_device *device, int handle) { - assert(device); +int device_new_from_watch_handle_at(sd_device **ret, int dirfd, int wd) { + char path_wd[STRLEN("/run/udev/watch/") + DECIMAL_STR_MAX(int)]; + _cleanup_free_ char *id = NULL; + int r; - device->watch_handle = handle; + assert(ret); + + if (wd < 0) + return -EBADF; + + if (dirfd >= 0) { + xsprintf(path_wd, "%d", wd); + r = readlinkat_malloc(dirfd, path_wd, &id); + } else { + xsprintf(path_wd, "/run/udev/watch/%d", wd); + r = readlink_malloc(path_wd, &id); + } + if (r < 0) + return r; + + return sd_device_new_from_device_id(ret, id); } int device_rename(sd_device *device, const char *name) { @@ -761,7 +876,7 @@ static int device_tag(sd_device *device, const char *tag, bool add) { assert(device); assert(tag); - r = device_get_id_filename(device, &id); + r = device_get_device_id(device, &id); if (r < 0) return r; @@ -822,9 +937,6 @@ static bool device_has_info(sd_device *device) { if (!set_isempty(device->current_tags)) return true; - if (device->watch_handle >= 0) - return true; - return false; } @@ -846,7 +958,7 @@ int device_update_db(sd_device *device) { has_info = device_has_info(device); - r = device_get_id_filename(device, &id); + r = device_get_device_id(device, &id); if (r < 0) return r; @@ -899,9 +1011,6 @@ int device_update_db(sd_device *device) { if (device->devlink_priority != 0) fprintf(f, "L:%i\n", device->devlink_priority); - - if (device->watch_handle >= 0) - fprintf(f, "W:%i\n", device->watch_handle); } if (device->usec_initialized > 0) @@ -950,7 +1059,7 @@ int device_delete_db(sd_device *device) { assert(device); - r = device_get_id_filename(device, &id); + r = device_get_device_id(device, &id); if (r < 0) return r; diff --git a/src/libsystemd/sd-device/device-private.h b/src/libsystemd/sd-device/device-private.h index ec76f772e5..fe268d7f2f 100644 --- a/src/libsystemd/sd-device/device-private.h +++ b/src/libsystemd/sd-device/device-private.h @@ -12,18 +12,22 @@ int device_new_from_nulstr(sd_device **ret, uint8_t *nulstr, size_t len); int device_new_from_strv(sd_device **ret, char **strv); +int device_new_from_watch_handle_at(sd_device **ret, int dirfd, int wd); +static inline int device_new_from_watch_handle(sd_device **ret, int wd) { + return device_new_from_watch_handle_at(ret, -1, wd); +} -int device_get_id_filename(sd_device *device, const char **ret); +int device_get_device_id(sd_device *device, const char **ret); int device_get_devlink_priority(sd_device *device, int *priority); -int device_get_watch_handle(sd_device *device, int *handle); +int device_get_watch_handle(sd_device *device); int device_get_devnode_mode(sd_device *device, mode_t *mode); int device_get_devnode_uid(sd_device *device, uid_t *uid); int device_get_devnode_gid(sd_device *device, gid_t *gid); void device_seal(sd_device *device); void device_set_is_initialized(sd_device *device); -void device_set_watch_handle(sd_device *device, int fd); +int device_set_watch_handle(sd_device *device, int wd); void device_set_db_persist(sd_device *device); void device_set_devlink_priority(sd_device *device, int priority); int device_ensure_usec_initialized(sd_device *device, sd_device *device_old); diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 4b7f22e207..f2e8c2112c 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -62,7 +62,7 @@ static sd_device *device_free(sd_device *device) { free(device->subsystem); free(device->driver_subsystem); free(device->driver); - free(device->id_filename); + free(device->device_id); free(device->properties_strv); free(device->properties_nulstr); @@ -623,7 +623,7 @@ _public_ int sd_device_new_from_device_id(sd_device **ret, const char *id) { struct ifreq ifr = {}; int ifindex; - r = ifr.ifr_ifindex = parse_ifindex(&id[1]); + r = ifr.ifr_ifindex = parse_ifindex(id + 1); if (r < 0) return r; @@ -652,15 +652,14 @@ _public_ int sd_device_new_from_device_id(sd_device **ret, const char *id) { } case '+': { - char subsys[PATH_MAX]; - char *sysname; + char subsys[NAME_MAX+1]; /* NAME_MAX does not include the trailing NUL. */ + const char *sysname; - (void) strscpy(subsys, sizeof(subsys), id + 1); - sysname = strchr(subsys, ':'); + sysname = strchr(id + 1, ':'); if (!sysname) return -EINVAL; - sysname[0] = '\0'; + (void) strnscpy(subsys, sizeof(subsys), id + 1, sysname - id - 1); sysname++; return sd_device_new_from_subsystem_sysname(ret, subsys, sysname); @@ -752,15 +751,34 @@ int device_set_subsystem(sd_device *device, const char *_subsystem) { return free_and_replace(device->subsystem, subsystem); } -static int device_set_drivers_subsystem(sd_device *device, const char *_subsystem) { +static int device_set_drivers_subsystem(sd_device *device) { _cleanup_free_ char *subsystem = NULL; + const char *syspath, *drivers, *p; int r; assert(device); - assert(_subsystem); - assert(*_subsystem); - subsystem = strdup(_subsystem); + r = sd_device_get_syspath(device, &syspath); + if (r < 0) + return r; + + drivers = strstr(syspath, "/drivers/"); + if (!drivers) + return -EINVAL; + + for (p = drivers - 1; p >= syspath; p--) + if (*p == '/') + break; + + if (p <= syspath) + /* syspath does not start with /sys/ ?? */ + return -EINVAL; + p++; + if (p >= drivers) + /* refuse duplicated slashes */ + return -EINVAL; + + subsystem = strndup(p, drivers - p); if (!subsystem) return -ENOMEM; @@ -772,60 +790,46 @@ static int device_set_drivers_subsystem(sd_device *device, const char *_subsyste } _public_ int sd_device_get_subsystem(sd_device *device, const char **ret) { - const char *syspath, *drivers = NULL; int r; assert_return(device, -EINVAL); - r = sd_device_get_syspath(device, &syspath); - if (r < 0) - return r; - if (!device->subsystem_set) { _cleanup_free_ char *subsystem = NULL; + const char *syspath; char *path; + r = sd_device_get_syspath(device, &syspath); + if (r < 0) + return r; + /* read 'subsystem' link */ path = strjoina(syspath, "/subsystem"); r = readlink_value(path, &subsystem); - if (r >= 0) + if (r < 0 && r != -ENOENT) + return log_device_debug_errno(device, r, + "sd-device: Failed to read subsystem for %s: %m", + device->devpath); + + if (subsystem) r = device_set_subsystem(device, subsystem); /* use implicit names */ else if (path_startswith(device->devpath, "/module/")) r = device_set_subsystem(device, "module"); - else if (!(drivers = strstr(syspath, "/drivers/")) && - PATH_STARTSWITH_SET(device->devpath, "/subsystem/", + else if (strstr(syspath, "/drivers/")) + r = device_set_drivers_subsystem(device); + else if (PATH_STARTSWITH_SET(device->devpath, "/subsystem/", "/class/", "/bus/")) r = device_set_subsystem(device, "subsystem"); - if (r < 0 && r != -ENOENT) - return log_device_debug_errno(device, r, "sd-device: Failed to set subsystem for %s: %m", device->devpath); - - device->subsystem_set = true; - } else if (!device->driver_subsystem_set) - drivers = strstr(syspath, "/drivers/"); - - if (!device->driver_subsystem_set) { - if (drivers) { - _cleanup_free_ char *subpath = NULL; - - subpath = strndup(syspath, drivers - syspath); - if (!subpath) - r = -ENOMEM; - else { - const char *subsys; - - subsys = strrchr(subpath, '/'); - if (!subsys) - r = -EINVAL; - else - r = device_set_drivers_subsystem(device, subsys + 1); - } - if (r < 0 && r != -ENOENT) - return log_device_debug_errno(device, r, "sd-device: Failed to set subsystem for driver %s: %m", device->devpath); + else { + device->subsystem_set = true; + r = 0; } - - device->driver_subsystem_set = true; + if (r < 0) + return log_device_debug_errno(device, r, + "sd-device: Failed to set subsystem for %s: %m", + device->devpath); } if (!device->subsystem) @@ -987,7 +991,7 @@ _public_ int sd_device_get_devname(sd_device *device, const char **devname) { return 0; } -static int device_set_sysname(sd_device *device) { +static int device_set_sysname_and_sysnum(sd_device *device) { _cleanup_free_ char *sysname = NULL; const char *sysnum = NULL; const char *pos; @@ -1024,7 +1028,6 @@ static int device_set_sysname(sd_device *device) { if (len == 0) sysnum = NULL; - device->sysname_set = true; device->sysnum = sysnum; return free_and_replace(device->sysname, sysname); } @@ -1034,14 +1037,12 @@ _public_ int sd_device_get_sysname(sd_device *device, const char **ret) { assert_return(device, -EINVAL); - if (!device->sysname_set) { - r = device_set_sysname(device); + if (!device->sysname) { + r = device_set_sysname_and_sysnum(device); if (r < 0) return r; } - assert_return(device->sysname, -ENOENT); - if (ret) *ret = device->sysname; return 0; @@ -1052,8 +1053,8 @@ _public_ int sd_device_get_sysnum(sd_device *device, const char **ret) { assert_return(device, -EINVAL); - if (!device->sysname_set) { - r = device_set_sysname(device); + if (!device->sysname) { + r = device_set_sysname_and_sysnum(device); if (r < 0) return r; } @@ -1237,10 +1238,10 @@ static int handle_db_line(sd_device *device, char key, const char *value) { break; case 'W': - r = safe_atoi(value, &device->watch_handle); - if (r < 0) - return r; - + /* Deprecated. Previously, watch handle is both saved in database and /run/udev/watch. + * However, the handle saved in database may not be updated when the handle is updated + * or removed. Moreover, it is not necessary to store the handle within the database, + * as its value becomes meaningless when udevd is restarted. */ break; case 'V': r = safe_atou(value, &device->database_version); @@ -1255,11 +1256,11 @@ static int handle_db_line(sd_device *device, char key, const char *value) { return 0; } -int device_get_id_filename(sd_device *device, const char **ret) { +int device_get_device_id(sd_device *device, const char **ret) { assert(device); assert(ret); - if (!device->id_filename) { + if (!device->device_id) { _cleanup_free_ char *id = NULL; const char *subsystem; dev_t devnum; @@ -1296,7 +1297,6 @@ int device_get_id_filename(sd_device *device, const char **ret) { if (!subsystem) return -EINVAL; - if (streq(subsystem, "drivers")) /* the 'drivers' pseudo-subsystem is special, and needs the real subsystem * encoded as well */ @@ -1307,10 +1307,13 @@ int device_get_id_filename(sd_device *device, const char **ret) { return -ENOMEM; } - device->id_filename = TAKE_PTR(id); + if (!filename_is_valid(id)) + return -EINVAL; + + device->device_id = TAKE_PTR(id); } - *ret = device->id_filename; + *ret = device->device_id; return 0; } @@ -1407,7 +1410,7 @@ int device_read_db_internal(sd_device *device, bool force) { if (device->db_loaded || (!force && device->sealed)) return 0; - r = device_get_id_filename(device, &id); + r = device_get_device_id(device, &id); if (r < 0) return r; diff --git a/src/test/test-udev.c b/src/test/test-udev.c index 6bb8a9e4fc..c2c63cc679 100644 --- a/src/test/test-udev.c +++ b/src/test/test-udev.c @@ -130,7 +130,7 @@ static int run(int argc, char *argv[]) { } } - udev_event_execute_rules(event, 3 * USEC_PER_SEC, SIGKILL, NULL, rules); + udev_event_execute_rules(event, -1, 3 * USEC_PER_SEC, SIGKILL, NULL, rules); udev_event_execute_run(event, 3 * USEC_PER_SEC, SIGKILL); return 0; diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index 12597194a9..f40b3120ca 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -911,8 +911,9 @@ static int update_devnode(UdevEvent *event) { return udev_node_add(dev, apply_mac, event->mode, event->uid, event->gid, event->seclabel_list); } -static void event_execute_rules_on_remove( +static int event_execute_rules_on_remove( UdevEvent *event, + int inotify_fd, usec_t timeout_usec, int timeout_signal, Hashmap *properties_list, @@ -933,13 +934,14 @@ static void event_execute_rules_on_remove( if (r < 0) log_device_debug_errno(dev, r, "Failed to delete database under /run/udev/data/, ignoring: %m"); - if (sd_device_get_devnum(dev, NULL) >= 0) - (void) udev_watch_end(dev); + (void) udev_watch_end(inotify_fd, dev); - (void) udev_rules_apply_to_event(rules, event, timeout_usec, timeout_signal, properties_list); + r = udev_rules_apply_to_event(rules, event, timeout_usec, timeout_signal, properties_list); if (sd_device_get_devnum(dev, NULL) >= 0) (void) udev_node_remove(dev); + + return r; } static int udev_event_on_move(sd_device *dev) { @@ -971,12 +973,14 @@ static int copy_all_tags(sd_device *d, sd_device *s) { return 0; } -int udev_event_execute_rules(UdevEvent *event, - usec_t timeout_usec, - int timeout_signal, - Hashmap *properties_list, - UdevRules *rules) { - const char *subsystem; +int udev_event_execute_rules( + UdevEvent *event, + int inotify_fd, /* This may be negative */ + usec_t timeout_usec, + int timeout_signal, + Hashmap *properties_list, + UdevRules *rules) { + sd_device_action_t action; sd_device *dev; int r; @@ -986,18 +990,15 @@ int udev_event_execute_rules(UdevEvent *event, dev = event->dev; - r = sd_device_get_subsystem(dev, &subsystem); - if (r < 0) - return log_device_error_errno(dev, r, "Failed to get subsystem: %m"); - r = sd_device_get_action(dev, &action); if (r < 0) return log_device_error_errno(dev, r, "Failed to get ACTION: %m"); - if (action == SD_DEVICE_REMOVE) { - event_execute_rules_on_remove(event, timeout_usec, timeout_signal, properties_list, rules); - return 0; - } + if (action == SD_DEVICE_REMOVE) + return event_execute_rules_on_remove(event, inotify_fd, timeout_usec, timeout_signal, properties_list, rules); + + /* Disable watch during event processing. */ + (void) udev_watch_end(inotify_fd, event->dev); r = device_clone_with_db(dev, &event->dev_db_clone); if (r < 0) @@ -1007,10 +1008,6 @@ int udev_event_execute_rules(UdevEvent *event, if (r < 0) log_device_warning_errno(dev, r, "Failed to copy all tags from old database entry, ignoring: %m"); - if (sd_device_get_devnum(dev, NULL) >= 0) - /* Disable watch during event processing. */ - (void) udev_watch_end(event->dev_db_clone); - if (action == SD_DEVICE_MOVE) { r = udev_event_on_move(event->dev); if (r < 0) @@ -1048,11 +1045,7 @@ int udev_event_execute_rules(UdevEvent *event, /* 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; - - return 0; + return update_devnode(event); } void udev_event_execute_run(UdevEvent *event, usec_t timeout_usec, int timeout_signal) { @@ -1087,3 +1080,24 @@ void udev_event_execute_run(UdevEvent *event, usec_t timeout_usec, int timeout_s } } } + +int udev_event_process_inotify_watch(UdevEvent *event, int inotify_fd) { + sd_device *dev; + + assert(event); + assert(inotify_fd >= 0); + + dev = event->dev; + + assert(dev); + + if (device_for_action(dev, SD_DEVICE_REMOVE)) + return 0; + + if (event->inotify_watch) + (void) udev_watch_begin(inotify_fd, dev); + else + (void) udev_watch_end(inotify_fd, dev); + + return 0; +} diff --git a/src/udev/udev-event.h b/src/udev/udev-event.h index ecbe957b4f..2067909fde 100644 --- a/src/udev/udev-event.h +++ b/src/udev/udev-event.h @@ -51,21 +51,30 @@ UdevEvent *udev_event_new(sd_device *dev, usec_t exec_delay_usec, sd_netlink *rt UdevEvent *udev_event_free(UdevEvent *event); DEFINE_TRIVIAL_CLEANUP_FUNC(UdevEvent*, udev_event_free); -size_t udev_event_apply_format(UdevEvent *event, - const char *src, char *dest, size_t size, - bool replace_whitespace); +size_t udev_event_apply_format( + UdevEvent *event, + const char *src, + char *dest, + size_t size, + bool replace_whitespace); int udev_check_format(const char *value, size_t *offset, const char **hint); -int udev_event_spawn(UdevEvent *event, - usec_t timeout_usec, - int timeout_signal, - bool accept_failure, - const char *cmd, char *result, size_t ressize); -int udev_event_execute_rules(UdevEvent *event, - usec_t timeout_usec, - int timeout_signal, - Hashmap *properties_list, - UdevRules *rules); +int udev_event_spawn( + UdevEvent *event, + usec_t timeout_usec, + int timeout_signal, + bool accept_failure, + const char *cmd, + char *result, + size_t ressize); +int udev_event_execute_rules( + UdevEvent *event, + int inotify_fd, + usec_t timeout_usec, + int timeout_signal, + Hashmap *properties_list, + UdevRules *rules); void udev_event_execute_run(UdevEvent *event, usec_t timeout_usec, int timeout_signal); +int udev_event_process_inotify_watch(UdevEvent *event, int inotify_fd); static inline usec_t udev_warn_timeout(usec_t timeout_usec) { return DIV_ROUND_UP(timeout_usec, 3); diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c index 96963a4343..5dc205e364 100644 --- a/src/udev/udev-node.c +++ b/src/udev/udev-node.c @@ -30,7 +30,7 @@ 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; + const char *id, *slink_tmp; struct stat stats; int r; @@ -81,10 +81,10 @@ static int node_symlink(sd_device *dev, const char *node, const char *slink) { } log_device_debug(dev, "Atomically replace '%s'", slink); - r = device_get_id_filename(dev, &id_filename); + r = device_get_device_id(dev, &id); if (r < 0) - return log_device_error_errno(dev, r, "Failed to get id_filename: %m"); - slink_tmp = strjoina(slink, ".tmp-", id_filename); + 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); @@ -147,7 +147,7 @@ static int link_find_prioritized(sd_device *dev, bool add, const char *stackdir, FOREACH_DIRENT_ALL(dent, dir, break) { _cleanup_(sd_device_unrefp) sd_device *dev_db = NULL; - const char *devnode, *id_filename; + const char *devnode, *id; int db_prio = 0; if (dent->d_name[0] == '\0') @@ -157,11 +157,11 @@ 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_id_filename(dev, &id_filename) < 0) + if (device_get_device_id(dev, &id) < 0) continue; /* did we find ourself? */ - if (streq(dent->d_name, id_filename)) + if (streq(dent->d_name, id)) continue; if (sd_device_new_from_device_id(&dev_db, dent->d_name) < 0) @@ -229,21 +229,21 @@ static size_t escape_path(const char *src, char *dest, size_t size) { static int link_update(sd_device *dev, const char *slink, bool add) { _cleanup_free_ char *filename = NULL, *dirname = NULL; char name_enc[PATH_MAX]; - const char *id_filename; + const char *id; int i, r, retries; assert(dev); assert(slink); - r = device_get_id_filename(dev, &id_filename); + r = device_get_device_id(dev, &id); if (r < 0) - return log_device_debug_errno(dev, r, "Failed to get id_filename: %m"); + return log_device_debug_errno(dev, r, "Failed to get device id: %m"); escape_path(slink + STRLEN("/dev"), name_enc, sizeof(name_enc)); dirname = path_join("/run/udev/links/", name_enc); if (!dirname) return log_oom(); - filename = path_join(dirname, id_filename); + filename = path_join(dirname, id); if (!filename) return log_oom(); @@ -348,7 +348,7 @@ int udev_node_update_old_links(sd_device *dev, sd_device *dev_old) { static int node_permissions_apply(sd_device *dev, bool apply_mac, mode_t mode, uid_t uid, gid_t gid, OrderedHashmap *seclabel_list) { - const char *devnode, *subsystem, *id_filename = NULL; + const char *devnode, *subsystem, *id = NULL; bool apply_mode, apply_uid, apply_gid; _cleanup_close_ int node_fd = -1; struct stat stats; @@ -366,7 +366,7 @@ static int node_permissions_apply(sd_device *dev, bool apply_mac, r = sd_device_get_devnum(dev, &devnum); if (r < 0) return log_device_debug_errno(dev, r, "Failed to get devnum: %m"); - (void) device_get_id_filename(dev, &id_filename); + (void) device_get_device_id(dev, &id); if (streq(subsystem, "block")) mode |= S_IFBLK; @@ -388,7 +388,7 @@ static int node_permissions_apply(sd_device *dev, bool apply_mac, if ((mode != MODE_INVALID && (stats.st_mode & S_IFMT) != (mode & S_IFMT)) || stats.st_rdev != devnum) { log_device_debug(dev, "Found node '%s' with non-matching devnum %s, skipping handling.", - devnode, id_filename); + devnode, strna(id)); return 0; /* We might process a device that already got replaced by the time we have a look * at it, handle this gracefully and step away. */ } @@ -509,10 +509,10 @@ int udev_node_add(sd_device *dev, bool apply, return log_device_debug_errno(dev, r, "Failed to get devnode: %m"); if (DEBUG_LOGGING) { - const char *id_filename = NULL; + const char *id = NULL; - (void) device_get_id_filename(dev, &id_filename); - log_device_debug(dev, "Handling device node '%s', devnum=%s", devnode, strnull(id_filename)); + (void) device_get_device_id(dev, &id); + log_device_debug(dev, "Handling device node '%s', devnum=%s", devnode, strna(id)); } r = node_permissions_apply(dev, apply, mode, uid, gid, seclabel_list); diff --git a/src/udev/udev-watch.c b/src/udev/udev-watch.c index 8656fb0076..7f15c87870 100644 --- a/src/udev/udev-watch.c +++ b/src/udev/udev-watch.c @@ -5,72 +5,60 @@ */ #include -#include #include "alloc-util.h" #include "device-private.h" #include "device-util.h" #include "dirent-util.h" #include "fs-util.h" -#include "mkdir.h" -#include "stdio-util.h" +#include "parse-util.h" #include "udev-watch.h" -static int inotify_fd = -1; - -/* inotify descriptor, will be shared with rules directory; - * set to cloexec since we need our children to be able to add - * watches for us. */ -int udev_watch_init(void) { - inotify_fd = inotify_init1(IN_CLOEXEC); - if (inotify_fd < 0) - return -errno; - - return inotify_fd; -} - -/* Move any old watches directory out of the way, and then restore the watches. */ -int udev_watch_restore(void) { +int udev_watch_restore(int inotify_fd) { struct dirent *ent; DIR *dir; int r; - if (inotify_fd < 0) - return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), - "Invalid inotify descriptor."); + /* Move any old watches directory out of the way, and then restore the watches. */ + + assert(inotify_fd >= 0); if (rename("/run/udev/watch", "/run/udev/watch.old") < 0) { if (errno != ENOENT) - return log_warning_errno(errno, "Failed to move watches directory /run/udev/watch. Old watches will not be restored: %m"); + return log_warning_errno(errno, "Failed to move watches directory /run/udev/watch. " + "Old watches will not be restored: %m"); return 0; } dir = opendir("/run/udev/watch.old"); if (!dir) - return log_warning_errno(errno, "Failed to open old watches directory /run/udev/watch.old. Old watches will not be restored: %m"); + return log_warning_errno(errno, "Failed to open old watches directory /run/udev/watch.old. " + "Old watches will not be restored: %m"); FOREACH_DIRENT_ALL(ent, dir, break) { _cleanup_(sd_device_unrefp) sd_device *dev = NULL; - _cleanup_free_ char *device = NULL; + int wd; if (ent->d_name[0] == '.') continue; - r = readlinkat_malloc(dirfd(dir), ent->d_name, &device); - if (r < 0) { - log_debug_errno(r, "Failed to read link '/run/udev/watch.old/%s', ignoring: %m", ent->d_name); - goto unlink; - } + /* For backward compatibility, read symlink from watch handle to device id, and ignore + * the opposite direction symlink. */ - r = sd_device_new_from_device_id(&dev, device); + if (safe_atoi(ent->d_name, &wd) < 0) + goto unlink; + + r = device_new_from_watch_handle_at(&dev, dirfd(dir), wd); if (r < 0) { - log_debug_errno(r, "Failed to create sd_device object for '%s', ignoring: %m", device); + log_full_errno(r == -ENODEV ? LOG_DEBUG : LOG_WARNING, r, + "Failed to create sd_device object from saved watch handle '%s', ignoring: %m", + ent->d_name); goto unlink; } log_device_debug(dev, "Restoring old watch"); - (void) udev_watch_begin(dev); + (void) udev_watch_begin(inotify_fd, dev); unlink: (void) unlinkat(dirfd(dir), ent->d_name, 0); } @@ -81,94 +69,54 @@ unlink: return 0; } -int udev_watch_begin(sd_device *dev) { - char filename[STRLEN("/run/udev/watch/") + DECIMAL_STR_MAX(int)]; - const char *devnode, *id_filename; +int udev_watch_begin(int inotify_fd, sd_device *dev) { + const char *devnode; int wd, r; - if (inotify_fd < 0) - return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), - "Invalid inotify descriptor."); + assert(inotify_fd >= 0); + assert(dev); r = sd_device_get_devname(dev, &devnode); if (r < 0) - return log_device_error_errno(dev, r, "Failed to get device name: %m"); + return log_device_debug_errno(dev, r, "Failed to get device name: %m"); log_device_debug(dev, "Adding watch on '%s'", devnode); wd = inotify_add_watch(inotify_fd, devnode, IN_CLOSE_WRITE); - if (wd < 0) - return log_device_full_errno(dev, errno == ENOENT ? LOG_DEBUG : LOG_ERR, errno, - "Failed to add device '%s' to watch: %m", devnode); + if (wd < 0) { + r = log_device_full_errno(dev, errno == ENOENT ? LOG_DEBUG : LOG_WARNING, + errno, "Failed to add device '%s' to watch: %m", devnode); - device_set_watch_handle(dev, wd); + (void) device_set_watch_handle(dev, -1); + return r; + } - xsprintf(filename, "/run/udev/watch/%d", wd); - r = mkdir_parents(filename, 0755); + r = device_set_watch_handle(dev, wd); if (r < 0) - return log_device_error_errno(dev, r, "Failed to create parent directory of '%s': %m", filename); - (void) unlink(filename); - - r = device_get_id_filename(dev, &id_filename); - if (r < 0) - return log_device_error_errno(dev, r, "Failed to get device id-filename: %m"); - - if (symlink(id_filename, filename) < 0) - return log_device_error_errno(dev, errno, "Failed to create symlink %s: %m", filename); + return log_device_warning_errno(dev, r, "Failed to save watch handle in /run/udev/watch: %m"); return 0; } -int udev_watch_end(sd_device *dev) { - char filename[STRLEN("/run/udev/watch/") + DECIMAL_STR_MAX(int)]; - int wd, r; +int udev_watch_end(int inotify_fd, sd_device *dev) { + int wd; + assert(dev); + + /* This may be called by 'udevadm test'. In that case, inotify_fd is not initialized. */ if (inotify_fd < 0) - return 0; /* Nothing to do. */ - - r = device_get_watch_handle(dev, &wd); - if (r == -ENOENT) return 0; - if (r < 0) - return log_device_debug_errno(dev, r, "Failed to get watch handle, ignoring: %m"); - log_device_debug(dev, "Removing watch"); - (void) inotify_rm_watch(inotify_fd, wd); + if (sd_device_get_devname(dev, NULL) < 0) + return 0; - xsprintf(filename, "/run/udev/watch/%d", wd); - (void) unlink(filename); - - device_set_watch_handle(dev, -1); + wd = device_get_watch_handle(dev); + if (wd < 0) + log_device_debug_errno(dev, wd, "Failed to get watch handle, ignoring: %m"); + else { + log_device_debug(dev, "Removing watch"); + (void) inotify_rm_watch(inotify_fd, wd); + } + (void) device_set_watch_handle(dev, -1); return 0; } - -int udev_watch_lookup(int wd, sd_device **ret) { - char filename[STRLEN("/run/udev/watch/") + DECIMAL_STR_MAX(int)]; - _cleanup_free_ char *device = NULL; - int r; - - assert(ret); - - if (inotify_fd < 0) - return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), - "Invalid inotify descriptor."); - - if (wd < 0) - return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), - "Invalid watch handle."); - - xsprintf(filename, "/run/udev/watch/%d", wd); - r = readlink_malloc(filename, &device); - if (r == -ENOENT) - return 0; - if (r < 0) - return log_debug_errno(r, "Failed to read link '%s': %m", filename); - - r = sd_device_new_from_device_id(ret, device); - if (r == -ENODEV) - return 0; - if (r < 0) - return log_debug_errno(r, "Failed to create sd_device object for '%s': %m", device); - - return 1; -} diff --git a/src/udev/udev-watch.h b/src/udev/udev-watch.h index a15fa2767e..d211d99f49 100644 --- a/src/udev/udev-watch.h +++ b/src/udev/udev-watch.h @@ -3,8 +3,6 @@ #include "sd-device.h" -int udev_watch_init(void); -int udev_watch_restore(void); -int udev_watch_begin(sd_device *dev); -int udev_watch_end(sd_device *dev); -int udev_watch_lookup(int wd, sd_device **ret); +int udev_watch_restore(int inotify_fd); +int udev_watch_begin(int inotify_fd, sd_device *dev); +int udev_watch_end(int inotify_fd, sd_device *dev); diff --git a/src/udev/udevadm-test.c b/src/udev/udevadm-test.c index 7a30e25135..fbac719fa0 100644 --- a/src/udev/udevadm-test.c +++ b/src/udev/udevadm-test.c @@ -141,7 +141,7 @@ int test_main(int argc, char *argv[], void *userdata) { assert_se(sigfillset(&mask) >= 0); assert_se(sigprocmask(SIG_SETMASK, &mask, &sigmask_orig) >= 0); - udev_event_execute_rules(event, 60 * USEC_PER_SEC, SIGKILL, NULL, rules); + udev_event_execute_rules(event, -1, 60 * USEC_PER_SEC, SIGKILL, NULL, rules); FOREACH_DEVICE_PROPERTY(dev, key, value) printf("%s=%s\n", key, value); diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 2c702d0388..5574abce9d 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -92,10 +92,12 @@ typedef struct Manager { sd_device_monitor *monitor; struct udev_ctrl *ctrl; - int fd_inotify; int worker_watch[2]; + /* used by udev-watch */ + int inotify_fd; sd_event_source *inotify_event; + sd_event_source *kill_workers_event; usec_t last_usec; @@ -304,7 +306,7 @@ static Manager* manager_free(Manager *manager) { hashmap_free_free_free(manager->properties); udev_rules_free(manager->rules); - safe_close(manager->fd_inotify); + safe_close(manager->inotify_fd); safe_close_pair(manager->worker_watch); return mfree(manager); @@ -473,7 +475,7 @@ static int worker_process_device(Manager *manager, sd_device *dev) { * degree — they go hand-in-hand after all. */ log_device_debug(dev, "Block device is currently locked, installing watch to wait until the lock is released."); - (void) udev_watch_begin(dev); + (void) udev_watch_begin(manager->inotify_fd, dev); /* Now the watch is installed, let's lock the device again, maybe in the meantime things changed */ r = worker_lock_block_device(dev, &fd_lock); @@ -484,7 +486,13 @@ static int worker_process_device(Manager *manager, sd_device *dev) { (void) worker_mark_block_device_read_only(dev); /* apply rules, create node, symlinks */ - r = udev_event_execute_rules(udev_event, arg_event_timeout_usec, arg_timeout_signal, manager->properties, manager->rules); + r = udev_event_execute_rules( + udev_event, + manager->inotify_fd, + arg_event_timeout_usec, + arg_timeout_signal, + manager->properties, + manager->rules); if (r < 0) return r; @@ -494,14 +502,9 @@ static int worker_process_device(Manager *manager, sd_device *dev) { /* in case rtnl was initialized */ manager->rtnl = sd_netlink_ref(udev_event->rtnl); - /* apply/restore/end inotify watch */ - if (udev_event->inotify_watch) { - (void) udev_watch_begin(dev); - r = device_update_db(dev); - if (r < 0) - return log_device_debug_errno(dev, r, "Failed to update database under /run/udev/data/: %m"); - } else - (void) udev_watch_end(dev); + r = udev_event_process_inotify_watch(udev_event, manager->inotify_fd); + if (r < 0) + return r; log_device_uevent(dev, "Device processed"); return 0; @@ -872,7 +875,7 @@ static void manager_exit(Manager *manager) { manager->ctrl = udev_ctrl_unref(manager->ctrl); manager->inotify_event = sd_event_source_unref(manager->inotify_event); - manager->fd_inotify = safe_close(manager->fd_inotify); + manager->inotify_fd = safe_close(manager->inotify_fd); manager->monitor = sd_device_monitor_unref(manager->monitor); @@ -1299,17 +1302,21 @@ static int on_inotify(sd_event_source *s, int fd, uint32_t revents, void *userda _cleanup_(sd_device_unrefp) sd_device *dev = NULL; const char *devnode; - if (udev_watch_lookup(e->wd, &dev) <= 0) + r = device_new_from_watch_handle(&dev, e->wd); + if (r < 0) { + log_debug_errno(r, "Failed to create sd_device object from watch handle, ignoring: %m"); continue; + } if (sd_device_get_devname(dev, &devnode) < 0) continue; log_device_debug(dev, "Inotify event: %x for %s", e->mask, devnode); if (e->mask & IN_CLOSE_WRITE) - synthesize_change(dev); - else if (e->mask & IN_IGNORED) - udev_watch_end(dev); + (void) synthesize_change(dev); + + /* Do not handle IN_IGNORED here. It should be handled by worker in 'remove' uevent; + * udev_event_execute_rules() -> event_execute_rules_on_remove() -> udev_watch_end(). */ } return 1; @@ -1669,7 +1676,7 @@ static int manager_new(Manager **ret, int fd_ctrl, int fd_uevent, const char *cg return log_oom(); *manager = (Manager) { - .fd_inotify = -1, + .inotify_fd = -1, .worker_watch = { -1, -1 }, .cgroup = cgroup, }; @@ -1722,12 +1729,11 @@ static int main_loop(Manager *manager) { if (r < 0) return log_error_errno(r, "Failed to enable SO_PASSCRED: %m"); - r = udev_watch_init(); - if (r < 0) - return log_error_errno(r, "Failed to create inotify descriptor: %m"); - manager->fd_inotify = r; + manager->inotify_fd = inotify_init1(IN_CLOEXEC); + if (manager->inotify_fd < 0) + return log_error_errno(errno, "Failed to create inotify descriptor: %m"); - udev_watch_restore(); + udev_watch_restore(manager->inotify_fd); /* block and listen to all signals on signalfd */ assert_se(sigprocmask_many(SIG_BLOCK, NULL, SIGTERM, SIGINT, SIGHUP, SIGCHLD, -1) >= 0); @@ -1772,7 +1778,7 @@ static int main_loop(Manager *manager) { if (r < 0) return log_error_errno(r, "Failed to set IDLE event priority for udev control event source: %m"); - r = sd_event_add_io(manager->event, &manager->inotify_event, manager->fd_inotify, EPOLLIN, on_inotify, manager); + r = sd_event_add_io(manager->event, &manager->inotify_event, manager->inotify_fd, EPOLLIN, on_inotify, manager); if (r < 0) return log_error_errno(r, "Failed to create inotify event source: %m"); diff --git a/test/TEST-17-UDEV/test.sh b/test/TEST-17-UDEV/test.sh index 43119d4c10..446f4a83db 100755 --- a/test/TEST-17-UDEV/test.sh +++ b/test/TEST-17-UDEV/test.sh @@ -8,7 +8,7 @@ TEST_NO_NSPAWN=1 # shellcheck source=test/test-functions . "${TEST_BASE_DIR:?}/test-functions" -QEMU_TIMEOUT=500 +QEMU_TIMEOUT=800 test_append_files() { ( diff --git a/test/units/testsuite-17.01.sh b/test/units/testsuite-17.01.sh index 8be27d9507..3b099fc2a4 100755 --- a/test/units/testsuite-17.01.sh +++ b/test/units/testsuite-17.01.sh @@ -20,6 +20,7 @@ while : ; do done cat >/run/udev/rules.d/50-testsuite.rules </run/udev/rules.d/50-testsuite.rules </run/udev/rules.d/50-testsuite.rules <"${test_rule}" + cat >"${test_rule}" <>/etc/udev/udev.conf echo "timeout_signal=SIGABRT" >>/etc/udev/udev.conf diff --git a/test/units/testsuite-17.04.sh b/test/units/testsuite-17.04.sh index b33d47ef6d..fbe8971732 100755 --- a/test/units/testsuite-17.04.sh +++ b/test/units/testsuite-17.04.sh @@ -9,6 +9,7 @@ test ! -f /run/udev/tags/changed/c1:3 udevadm info /dev/null | grep -E 'E: (TAGS|CURRENT_TAGS)=.*:(added|changed):' && exit 1 cat >/run/udev/rules.d/50-testsuite.rules </run/udev/rules.d/00-debug.rules </run/udev/rules.d/50-testsuite.rules </run/udev/rules.d/50-testsuite.rules </failed +udevadm settle + for t in "${0%.sh}".*.sh; do echo "Running $t"; ./"$t" done