From 7ca31a91deced165e7d1b4e8ce83fd50ac002dd8 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 19 Dec 2023 12:27:53 +0900 Subject: [PATCH 1/4] udev-manager: use ASSERT_PTR() --- src/udev/udev-manager.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/udev/udev-manager.c b/src/udev/udev-manager.c index 8077e51055..859fc02436 100644 --- a/src/udev/udev-manager.c +++ b/src/udev/udev-manager.c @@ -333,11 +333,9 @@ static int on_event_timeout_warning(sd_event_source *s, uint64_t usec, void *use } static void worker_attach_event(Worker *worker, Event *event) { - Manager *manager; - sd_event *e; + Manager *manager = ASSERT_PTR(ASSERT_PTR(worker)->manager); + sd_event *e = ASSERT_PTR(manager->event); - assert(worker); - assert(worker->manager); assert(event); assert(!event->worker); assert(!worker->event); @@ -347,9 +345,6 @@ static void worker_attach_event(Worker *worker, Event *event) { event->state = EVENT_RUNNING; event->worker = worker; - manager = worker->manager; - e = manager->event; - (void) sd_event_add_time_relative(e, &event->timeout_warning_event, CLOCK_MONOTONIC, udev_warn_timeout(manager->timeout_usec), USEC_PER_SEC, on_event_timeout_warning, event); From aff70e13885696822c2caac44fb1612c3141431c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 19 Dec 2023 12:28:53 +0900 Subject: [PATCH 2/4] udev: handle event_timeout=infinity correctly This is a paranoia, as even USEC_INFINITY / 3 is finite, it is still so large in general. --- src/udev/udev-spawn.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/udev/udev-spawn.h b/src/udev/udev-spawn.h index 5efea2e886..ba6f1ae872 100644 --- a/src/udev/udev-spawn.h +++ b/src/udev/udev-spawn.h @@ -24,5 +24,8 @@ int udev_event_spawn( void udev_event_execute_run(UdevEvent *event, usec_t timeout_usec, int timeout_signal); static inline usec_t udev_warn_timeout(usec_t timeout_usec) { + if (timeout_usec == USEC_INFINITY) + return USEC_INFINITY; + return DIV_ROUND_UP(timeout_usec, 3); } From 9cceb0be2115210d2733408523afb90a29f31a88 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 19 Dec 2023 12:15:25 +0900 Subject: [PATCH 3/4] udev: refuse too short timeout value Setting zero or too short timeout for each uevent is meaningless, and causes the system fails to boot. Let's refuse such values. Also, delaying execution of RUN= commands too long also makes many uevents enter the failed state. So, let's refuse such misconfiguration. --- src/udev/udev-manager.c | 22 +++++++++++++++++++++- src/udev/udev-manager.h | 1 + src/udev/udev-worker.h | 3 +++ src/udev/udevd.c | 2 ++ 4 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/udev/udev-manager.c b/src/udev/udev-manager.c index 859fc02436..d56ddfe538 100644 --- a/src/udev/udev-manager.c +++ b/src/udev/udev-manager.c @@ -1189,13 +1189,33 @@ Manager* manager_new(void) { .worker_watch = EBADF_PAIR, .log_level = LOG_INFO, .resolve_name_timing = RESOLVE_NAME_EARLY, - .timeout_usec = 180 * USEC_PER_SEC, + .timeout_usec = DEFAULT_WORKER_TIMEOUT_USEC, .timeout_signal = SIGKILL, }; return manager; } +void manager_adjust_arguments(Manager *manager) { + assert(manager); + + if (manager->timeout_usec < MIN_WORKER_TIMEOUT_USEC) { + log_debug("Timeout (%s) for processing event is too small, using the default: %s", + FORMAT_TIMESPAN(manager->timeout_usec, 1), + FORMAT_TIMESPAN(DEFAULT_WORKER_TIMEOUT_USEC, 1)); + + manager->timeout_usec = DEFAULT_WORKER_TIMEOUT_USEC; + } + + if (manager->exec_delay_usec >= manager->timeout_usec) { + log_debug("Delay (%s) for executing RUN= commands is too large compared with the timeout (%s) for event execution, ignoring the delay.", + FORMAT_TIMESPAN(manager->exec_delay_usec, 1), + FORMAT_TIMESPAN(manager->timeout_usec, 1)); + + manager->exec_delay_usec = 0; + } +} + int manager_init(Manager *manager, int fd_ctrl, int fd_uevent) { _cleanup_free_ char *cgroup = NULL; int r; diff --git a/src/udev/udev-manager.h b/src/udev/udev-manager.h index afbc67f696..864fc0290e 100644 --- a/src/udev/udev-manager.h +++ b/src/udev/udev-manager.h @@ -56,6 +56,7 @@ Manager* manager_new(void); Manager* manager_free(Manager *manager); DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free); +void manager_adjust_arguments(Manager *manager); int manager_init(Manager *manager, int fd_ctrl, int fd_uevent); int manager_main(Manager *manager); diff --git a/src/udev/udev-worker.h b/src/udev/udev-worker.h index 05c319e309..e9aefc5b04 100644 --- a/src/udev/udev-worker.h +++ b/src/udev/udev-worker.h @@ -11,6 +11,9 @@ #include "hashmap.h" #include "time-util.h" +#define DEFAULT_WORKER_TIMEOUT_USEC (3 * USEC_PER_MINUTE) +#define MIN_WORKER_TIMEOUT_USEC (1 * USEC_PER_MSEC) + typedef struct UdevRules UdevRules; typedef struct UdevWorker { diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 2ed4282253..6f8546385e 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -355,6 +355,8 @@ int run_udevd(int argc, char *argv[]) { log_set_max_level(LOG_DEBUG); } + manager_adjust_arguments(manager); + r = must_be_root(); if (r < 0) return r; From 5d64eb559358e5db26e86a102a2362ad896da331 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 19 Dec 2023 13:58:35 +0900 Subject: [PATCH 4/4] udev-spawn: slightly adjust logs about timed out commands - Add full stop to the messages. - Do not kill commands before logging "killing", but do after. --- src/udev/udev-spawn.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/udev/udev-spawn.c b/src/udev/udev-spawn.c index 67a3005325..9d6993a242 100644 --- a/src/udev/udev-spawn.c +++ b/src/udev/udev-spawn.c @@ -105,19 +105,18 @@ static int on_spawn_timeout(sd_event_source *s, uint64_t usec, void *userdata) { DEVICE_TRACE_POINT(spawn_timeout, spawn->device, spawn->cmd); - kill_and_sigcont(spawn->pid, spawn->timeout_signal); - - log_device_error(spawn->device, "Spawned process '%s' ["PID_FMT"] timed out after %s, killing", + log_device_error(spawn->device, "Spawned process '%s' ["PID_FMT"] timed out after %s, killing.", spawn->cmd, spawn->pid, FORMAT_TIMESPAN(spawn->timeout_usec, USEC_PER_SEC)); + kill_and_sigcont(spawn->pid, spawn->timeout_signal); return 1; } static int on_spawn_timeout_warning(sd_event_source *s, uint64_t usec, void *userdata) { Spawn *spawn = ASSERT_PTR(userdata); - log_device_warning(spawn->device, "Spawned process '%s' ["PID_FMT"] is taking longer than %s to complete", + log_device_warning(spawn->device, "Spawned process '%s' ["PID_FMT"] is taking longer than %s to complete.", spawn->cmd, spawn->pid, FORMAT_TIMESPAN(spawn->timeout_warn_usec, USEC_PER_SEC));