From 5b2dce150d5eadcd33d620e095c9c1e2de51dd24 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 14 Oct 2024 00:55:43 +0900 Subject: [PATCH 1/3] udev: do not re-create database on remove event Fixes a bug introduced by f6bda694f908cc227b002570b893029aa4c9e173 (v256). With the offending commit, on remove event, database file for a device is once removed in event_execute_rules_on_remove(), but later re-created here. This fixes the issue, and makes the database file not re-created on remove event. --- src/udev/udev-worker.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/udev/udev-worker.c b/src/udev/udev-worker.c index 5577f5fab8d..7f22faccdc1 100644 --- a/src/udev/udev-worker.c +++ b/src/udev/udev-worker.c @@ -200,26 +200,31 @@ static int worker_process_device(UdevWorker *worker, sd_device *dev) { if (r < 0) return r; + /* Process RUN=. */ udev_event_execute_run(udev_event); if (!worker->rtnl) /* in case rtnl was initialized */ worker->rtnl = sd_netlink_ref(udev_event->rtnl); + /* Enable watch if requested. */ if (udev_event->inotify_watch) { r = udev_watch_begin(worker->inotify_fd, dev); if (r < 0 && r != -ENOENT) /* The device may be already removed, ignore -ENOENT. */ log_device_warning_errno(dev, r, "Failed to add inotify watch, ignoring: %m"); } - /* Finalize database. */ - r = device_add_property(dev, "ID_PROCESSING", NULL); - if (r < 0) - return log_device_warning_errno(dev, r, "Failed to remove 'ID_PROCESSING' property: %m"); + /* Finalize database. But do not re-create database on remove, which has been already removed in + * event_execute_rules_on_remove(). */ + if (!device_for_action(dev, SD_DEVICE_REMOVE)) { + r = device_add_property(dev, "ID_PROCESSING", NULL); + if (r < 0) + return log_device_warning_errno(dev, r, "Failed to remove 'ID_PROCESSING' property: %m"); - r = device_update_db(dev); - if (r < 0) - return log_device_warning_errno(dev, r, "Failed to update database under /run/udev/data/: %m"); + r = device_update_db(dev); + if (r < 0) + return log_device_warning_errno(dev, r, "Failed to update database under /run/udev/data/: %m"); + } log_device_uevent(dev, "Device processed"); return 0; From e8df18c9e171c87aebb2df8ac3bdd8f116236892 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 15 Oct 2024 06:22:24 +0900 Subject: [PATCH 2/3] udev: do not try to lock whole block device on remove event As another device may be created with the same device node while udevd is processing the remove event of the previous owner of the device node. This also adds comment why we skip watching device node on remove. --- src/udev/udev-watch.c | 3 +++ src/udev/udev-worker.c | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/src/udev/udev-watch.c b/src/udev/udev-watch.c index c28c43b2af4..1b8e2b8dbd4 100644 --- a/src/udev/udev-watch.c +++ b/src/udev/udev-watch.c @@ -181,6 +181,9 @@ int udev_watch_begin(int inotify_fd, sd_device *dev) { assert(inotify_fd >= 0); assert(dev); + /* Ignore the request of watching the device node on remove event, as the device node specified by + * DEVNAME= has already been removed, and may already be assigned to another device. Consider the + * case e.g. a USB stick memory was unplugged and then another one is plugged. */ if (device_for_action(dev, SD_DEVICE_REMOVE)) return 0; diff --git a/src/udev/udev-worker.c b/src/udev/udev-worker.c index 7f22faccdc1..59f56f653cd 100644 --- a/src/udev/udev-worker.c +++ b/src/udev/udev-worker.c @@ -97,6 +97,12 @@ static int worker_lock_whole_disk(sd_device *dev, int *ret_fd) { * event handling; in the case udev acquired the lock, the external process can block until udev has * finished its event handling. */ + /* Do not try to lock device on remove event, as the device node specified by DEVNAME= has already + * been removed, and may already be assigned to another device. Consider the case e.g. a USB stick + * memory was unplugged and then another one is plugged. */ + if (device_for_action(dev, SD_DEVICE_REMOVE)) + goto nolock; + r = udev_get_whole_disk(dev, &dev_whole_disk, &val); if (r < 0) return r; From 49c46fbaf15b95a8264d01213539914e15fdc6fe Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 14 Oct 2024 01:28:23 +0900 Subject: [PATCH 3/3] TEST-17-UDEV: check if udev database file is removed on remove event --- test/units/TEST-17-UDEV.database.sh | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100755 test/units/TEST-17-UDEV.database.sh diff --git a/test/units/TEST-17-UDEV.database.sh b/test/units/TEST-17-UDEV.database.sh new file mode 100755 index 00000000000..2b66333cadb --- /dev/null +++ b/test/units/TEST-17-UDEV.database.sh @@ -0,0 +1,22 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: LGPL-2.1-or-later +set -ex +set -o pipefail + +udevadm control --log-level=debug + +IFNAME=test-udev-aaa +ip link add "$IFNAME" type dummy +IFINDEX=$(ip -json link show "$IFNAME" | jq '.[].ifindex') +udevadm wait --timeout 10 "/sys/class/net/$IFNAME" +# Check if the database file is created. +[[ -e "/run/udev/data/n$IFINDEX" ]] + +ip link del "$IFNAME" +udevadm wait --timeout 10 --removed --settle "/sys/class/net/$IFNAME" +# CHeck if the database file is removed. +[[ ! -e "/run/udev/data/n$IFINDEX" ]] + +udevadm control --log-level=info + +exit 0