diff --git a/src/libsystemd/sd-device/device-private.c b/src/libsystemd/sd-device/device-private.c index de12ad7e00a..4a5a110364c 100644 --- a/src/libsystemd/sd-device/device-private.c +++ b/src/libsystemd/sd-device/device-private.c @@ -111,7 +111,7 @@ uint64_t device_get_devlinks_generation(sd_device *device) { return device->devlinks_generation; } -int device_get_devnode_mode(sd_device *device, mode_t *mode) { +int device_get_devnode_mode(sd_device *device, mode_t *ret) { int r; assert(device); @@ -123,13 +123,13 @@ int device_get_devnode_mode(sd_device *device, mode_t *mode) { if (device->devmode == MODE_INVALID) return -ENOENT; - if (mode) - *mode = device->devmode; + if (ret) + *ret = device->devmode; return 0; } -int device_get_devnode_uid(sd_device *device, uid_t *uid) { +int device_get_devnode_uid(sd_device *device, uid_t *ret) { int r; assert(device); @@ -141,20 +141,20 @@ int device_get_devnode_uid(sd_device *device, uid_t *uid) { if (device->devuid == UID_INVALID) return -ENOENT; - if (uid) - *uid = device->devuid; + if (ret) + *ret = device->devuid; return 0; } static int device_set_devuid(sd_device *device, const char *uid) { - unsigned u; + uid_t u; int r; assert(device); assert(uid); - r = safe_atou(uid, &u); + r = parse_uid(uid, &u); if (r < 0) return r; @@ -167,7 +167,7 @@ static int device_set_devuid(sd_device *device, const char *uid) { return 0; } -int device_get_devnode_gid(sd_device *device, gid_t *gid) { +int device_get_devnode_gid(sd_device *device, gid_t *ret) { int r; assert(device); @@ -179,20 +179,20 @@ int device_get_devnode_gid(sd_device *device, gid_t *gid) { if (device->devgid == GID_INVALID) return -ENOENT; - if (gid) - *gid = device->devgid; + if (ret) + *ret = device->devgid; return 0; } static int device_set_devgid(sd_device *device, const char *gid) { - unsigned g; + gid_t g; int r; assert(device); assert(gid); - r = safe_atou(gid, &g); + r = parse_gid(gid, &g); if (r < 0) return r; @@ -593,17 +593,17 @@ int device_get_properties_strv(sd_device *device, char ***strv) { return 0; } -int device_get_devlink_priority(sd_device *device, int *priority) { +int device_get_devlink_priority(sd_device *device, int *ret) { int r; assert(device); - assert(priority); r = device_read_db(device); if (r < 0) return r; - *priority = device->devlink_priority; + if (ret) + *ret = device->devlink_priority; return 0; } @@ -747,62 +747,72 @@ int device_new_from_watch_handle_at(sd_device **ret, int dirfd, int wd) { } int device_rename(sd_device *device, const char *name) { - _cleanup_free_ char *dirname = NULL; - const char *new_syspath, *interface; + _cleanup_free_ char *new_syspath = NULL; + const char *interface; int r; assert(device); assert(name); - dirname = dirname_malloc(device->syspath); - if (!dirname) + if (!filename_is_valid(name)) + return -EINVAL; + + r = path_extract_directory(device->syspath, &new_syspath); + if (r < 0) + return r; + + if (!path_extend(&new_syspath, name)) return -ENOMEM; - new_syspath = prefix_roota(dirname, name); + if (!path_is_safe(new_syspath)) + return -EINVAL; - /* the user must trust that the new name is correct */ + /* At the time this is called, the renamed device may not exist yet. Hence, we cannot validate + * the new syspath. */ r = device_set_syspath(device, new_syspath, false); if (r < 0) return r; - r = sd_device_get_property_value(device, "INTERFACE", &interface); - if (r >= 0) { - /* like DEVPATH_OLD, INTERFACE_OLD is not saved to the db, but only stays around for the current event */ - r = device_add_property_internal(device, "INTERFACE_OLD", interface); - if (r < 0) - return r; + /* Here, only clear the sysname and sysnum. They will be set when requested. */ + device->sysnum = NULL; + device->sysname = mfree(device->sysname); - r = device_add_property_internal(device, "INTERFACE", name); - if (r < 0) - return r; - } else if (r != -ENOENT) + r = sd_device_get_property_value(device, "INTERFACE", &interface); + if (r == -ENOENT) + return 0; + if (r < 0) return r; - return 0; + /* like DEVPATH_OLD, INTERFACE_OLD is not saved to the db, but only stays around for the current event */ + r = device_add_property_internal(device, "INTERFACE_OLD", interface); + if (r < 0) + return r; + + return device_add_property_internal(device, "INTERFACE", name); } -int device_shallow_clone(sd_device *old_device, sd_device **new_device) { - _cleanup_(sd_device_unrefp) sd_device *ret = NULL; - const char *val; +int device_shallow_clone(sd_device *device, sd_device **ret) { + _cleanup_(sd_device_unrefp) sd_device *dest = NULL; + const char *val = NULL; int r; - assert(old_device); - assert(new_device); + assert(device); + assert(ret); - r = device_new_aux(&ret); + r = device_new_aux(&dest); if (r < 0) return r; - r = device_set_syspath(ret, old_device->syspath, false); + r = device_set_syspath(dest, device->syspath, false); if (r < 0) return r; - (void) sd_device_get_subsystem(old_device, &val); - r = device_set_subsystem(ret, val); + (void) sd_device_get_subsystem(device, &val); + r = device_set_subsystem(dest, val); if (r < 0) return r; if (streq_ptr(val, "drivers")) { - r = free_and_strdup(&ret->driver_subsystem, old_device->driver_subsystem); + r = free_and_strdup(&dest->driver_subsystem, device->driver_subsystem); if (r < 0) return r; } @@ -810,49 +820,47 @@ int device_shallow_clone(sd_device *old_device, sd_device **new_device) { /* The device may be already removed. Let's copy minimal set of information to make * device_get_device_id() work without uevent file. */ - if (sd_device_get_property_value(old_device, "IFINDEX", &val) >= 0) { - r = device_set_ifindex(ret, val); + if (sd_device_get_property_value(device, "IFINDEX", &val) >= 0) { + r = device_set_ifindex(dest, val); if (r < 0) return r; } - if (sd_device_get_property_value(old_device, "MAJOR", &val) >= 0) { + if (sd_device_get_property_value(device, "MAJOR", &val) >= 0) { const char *minor = NULL; - (void) sd_device_get_property_value(old_device, "MINOR", &minor); - r = device_set_devnum(ret, val, minor); + (void) sd_device_get_property_value(device, "MINOR", &minor); + r = device_set_devnum(dest, val, minor); if (r < 0) return r; } - /* And then read uevent file, but ignore errors, as some devices seem to return a spurious - * error on read, e.g. -ENODEV, and even if ifindex or devnum is set in the above, - * sd_device_get_ifindex() or sd_device_get_devnum() fails. See. #19788. */ - (void) device_read_uevent_file(ret); + r = device_read_uevent_file(dest); + if (r < 0) + return r; - *new_device = TAKE_PTR(ret); + *ret = TAKE_PTR(dest); return 0; } -int device_clone_with_db(sd_device *old_device, sd_device **new_device) { - _cleanup_(sd_device_unrefp) sd_device *ret = NULL; +int device_clone_with_db(sd_device *device, sd_device **ret) { + _cleanup_(sd_device_unrefp) sd_device *dest = NULL; int r; - assert(old_device); - assert(new_device); + assert(device); + assert(ret); - r = device_shallow_clone(old_device, &ret); + r = device_shallow_clone(device, &dest); if (r < 0) return r; - r = device_read_db(ret); + r = device_read_db(dest); if (r < 0) return r; - ret->sealed = true; - - *new_device = TAKE_PTR(ret); + dest->sealed = true; + *ret = TAKE_PTR(dest); return 0; } @@ -923,15 +931,11 @@ static int device_tag(sd_device *device, const char *tag, bool add) { path = strjoina("/run/udev/tags/", tag, "/", id); - if (add) { - r = touch_file(path, true, USEC_INFINITY, UID_INVALID, GID_INVALID, 0444); - if (r < 0) - return r; - } else { - r = unlink(path); - if (r < 0 && errno != ENOENT) - return -errno; - } + if (add) + return touch_file(path, true, USEC_INFINITY, UID_INVALID, GID_INVALID, 0444); + + if (unlink(path) < 0 && errno != ENOENT) + return -errno; return 0; } @@ -940,16 +944,14 @@ int device_tag_index(sd_device *device, sd_device *device_old, bool add) { const char *tag; int r = 0, k; - if (add && device_old) { + if (add && device_old) /* delete possible left-over tags */ - FOREACH_DEVICE_TAG(device_old, tag) { + FOREACH_DEVICE_TAG(device_old, tag) if (!sd_device_has_tag(device, tag)) { k = device_tag(device_old, tag, false); if (r >= 0 && k < 0) r = k; } - } - } FOREACH_DEVICE_TAG(device, tag) { k = device_tag(device, tag, add); @@ -1007,8 +1009,7 @@ int device_update_db(sd_device *device) { /* do not store anything for otherwise empty devices */ if (!has_info && major(device->devnum) == 0 && device->ifindex == 0) { - r = unlink(path); - if (r < 0 && errno != ENOENT) + if (unlink(path) < 0 && errno != ENOENT) return -errno; return 0; @@ -1096,8 +1097,7 @@ int device_delete_db(sd_device *device) { path = strjoina("/run/udev/data/", id); - r = unlink(path); - if (r < 0 && errno != ENOENT) + if (unlink(path) < 0 && errno != ENOENT) return -errno; return 0; diff --git a/src/libsystemd/sd-device/device-private.h b/src/libsystemd/sd-device/device-private.h index 68caee2cb46..b6b36b67270 100644 --- a/src/libsystemd/sd-device/device-private.h +++ b/src/libsystemd/sd-device/device-private.h @@ -18,11 +18,11 @@ static inline int device_new_from_watch_handle(sd_device **ret, int wd) { } int device_get_device_id(sd_device *device, const char **ret); -int device_get_devlink_priority(sd_device *device, int *priority); +int device_get_devlink_priority(sd_device *device, int *ret); 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); +int device_get_devnode_mode(sd_device *device, mode_t *ret); +int device_get_devnode_uid(sd_device *device, uid_t *ret); +int device_get_devnode_gid(sd_device *device, gid_t *ret); int device_cache_sysattr_value(sd_device *device, const char *key, char *value); int device_get_cached_sysattr_value(sd_device *device, const char *key, const char **ret_value); @@ -51,8 +51,8 @@ int device_get_properties_nulstr(sd_device *device, const uint8_t **nulstr, size int device_get_properties_strv(sd_device *device, char ***strv); int device_rename(sd_device *device, const char *name); -int device_shallow_clone(sd_device *old_device, sd_device **new_device); -int device_clone_with_db(sd_device *old_device, sd_device **new_device); +int device_shallow_clone(sd_device *device, sd_device **ret); +int device_clone_with_db(sd_device *device, sd_device **ret); int device_copy_properties(sd_device *device_dst, sd_device *device_src); int device_tag_index(sd_device *dev, sd_device *dev_old, bool add); diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 6c8e7d646fb..0c5b2a67e33 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -724,11 +724,14 @@ int device_read_uevent_file(sd_device *device) { path = strjoina(syspath, "/uevent"); r = read_full_virtual_file(path, &uevent, &uevent_len); - if (IN_SET(r, -EACCES, -ENOENT)) - /* The uevent files may be write-only, or the device may not have uevent file. */ - return 0; - if (r < 0) + if (r < 0) { + /* The uevent files may be write-only, the device may be already removed, or the device + * may not have the uevent file. */ + if (r == -EACCES || ERRNO_IS_DEVICE_ABSENT(r)) + return 0; + return log_device_debug_errno(device, r, "sd-device: Failed to read uevent file '%s': %m", path); + } for (size_t i = 0; i < uevent_len; i++) switch (state) { @@ -1437,37 +1440,30 @@ int device_get_device_id(sd_device *device, const char **ret) { return r; if (sd_device_get_devnum(device, &devnum) >= 0) { - assert(subsystem); - /* use dev_t — b259:131072, c254:0 */ - r = asprintf(&id, "%c%u:%u", + if (asprintf(&id, "%c%u:%u", streq(subsystem, "block") ? 'b' : 'c', - major(devnum), minor(devnum)); - if (r < 0) + major(devnum), minor(devnum)) < 0) return -ENOMEM; } else if (sd_device_get_ifindex(device, &ifindex) >= 0) { /* use netdev ifindex — n3 */ - r = asprintf(&id, "n%u", (unsigned) ifindex); - if (r < 0) + if (asprintf(&id, "n%u", (unsigned) ifindex) < 0) return -ENOMEM; } else { + _cleanup_free_ char *sysname = NULL; + /* use $subsys:$sysname — pci:0000:00:1f.2 - * sysname() has '!' translated, get it from devpath - */ - const char *sysname; + * sd_device_get_sysname() has '!' translated, get it from devpath */ + r = path_extract_filename(device->devpath, &sysname); + if (r < 0) + return r; - sysname = basename(device->devpath); - if (!sysname) - return -EINVAL; - - if (!subsystem) - return -EINVAL; - - if (streq(subsystem, "drivers")) - /* the 'drivers' pseudo-subsystem is special, and needs the real subsystem - * encoded as well */ + if (streq(subsystem, "drivers")) { + /* the 'drivers' pseudo-subsystem is special, and needs the real + * subsystem encoded as well */ + assert(device->driver_subsystem); id = strjoin("+drivers:", device->driver_subsystem, ":", sysname); - else + } else id = strjoin("+", subsystem, ":", sysname); if (!id) return -ENOMEM; diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index 78e8f3018c5..9b531051999 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -869,15 +869,21 @@ int udev_event_spawn( } static int rename_netif(UdevEvent *event) { - sd_device *dev = event->dev; const char *oldname; + sd_device *dev; unsigned flags; int ifindex, r; + assert(event); + if (!event->name) return 0; /* No new name is requested. */ - r = sd_device_get_sysname(dev, &oldname); + dev = ASSERT_PTR(event->dev); + + /* Read sysname from cloned sd-device object, otherwise use-after-free is triggered, as the + * main object will be renamed and dev->sysname will be freed in device_rename(). */ + r = sd_device_get_sysname(event->dev_db_clone, &oldname); if (r < 0) return log_device_error_errno(dev, r, "Failed to get sysname: %m");