From 1f3f6bd0078b9d76d5ed72b74b890ca5e3a1756c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 18 Dec 2018 14:49:17 +0900 Subject: [PATCH 1/7] udevd: use worker_free() on failure in worker_new() Otherwise, worker_monitor may not unrefed correctly. --- src/udev/udevd.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 697506feaf5..33fc0b3c2cd 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -186,6 +186,8 @@ static void worker_free(struct worker *worker) { free(worker); } +DEFINE_TRIVIAL_CLEANUP_FUNC(struct worker *, worker_free); + static void manager_workers_free(Manager *manager) { struct worker *worker; Iterator i; @@ -199,7 +201,7 @@ static void manager_workers_free(Manager *manager) { } static int worker_new(struct worker **ret, Manager *manager, sd_device_monitor *worker_monitor, pid_t pid) { - _cleanup_free_ struct worker *worker = NULL; + _cleanup_(worker_freep) struct worker *worker = NULL; int r; assert(ret); From d40534643b0cc6475813b762ebd573716c4932e3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 18 Dec 2018 14:50:42 +0900 Subject: [PATCH 2/7] udevd: use structured initializer at one more place --- src/udev/udevd.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 33fc0b3c2cd..24f4824018c 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -209,15 +209,18 @@ static int worker_new(struct worker **ret, Manager *manager, sd_device_monitor * assert(worker_monitor); assert(pid > 1); - worker = new0(struct worker, 1); + /* close monitor, but keep address around */ + device_monitor_disconnect(worker_monitor); + + worker = new(struct worker, 1); if (!worker) return -ENOMEM; - worker->manager = manager; - /* close monitor, but keep address around */ - device_monitor_disconnect(worker_monitor); - worker->monitor = sd_device_monitor_ref(worker_monitor); - worker->pid = pid; + *worker = (struct worker) { + .manager = manager, + .monitor = sd_device_monitor_ref(worker_monitor), + .pid = pid, + }; r = hashmap_ensure_allocated(&manager->workers, NULL); if (r < 0) From 956833b4170dcc957befb9009f5ea4a6dbd05e87 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 18 Dec 2018 15:03:50 +0900 Subject: [PATCH 3/7] udevd: provide worker_hash_ops and drop manager_workers_free() --- src/udev/udevd.c | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 24f4824018c..837e13e0dc9 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -187,18 +187,7 @@ static void worker_free(struct worker *worker) { } DEFINE_TRIVIAL_CLEANUP_FUNC(struct worker *, worker_free); - -static void manager_workers_free(Manager *manager) { - struct worker *worker; - Iterator i; - - assert(manager); - - HASHMAP_FOREACH(worker, manager->workers, i) - worker_free(worker); - - manager->workers = hashmap_free(manager->workers); -} +DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(worker_hash_op, void, trivial_hash_func, trivial_compare_func, struct worker, worker_free); static int worker_new(struct worker **ret, Manager *manager, sd_device_monitor *worker_monitor, pid_t pid) { _cleanup_(worker_freep) struct worker *worker = NULL; @@ -222,7 +211,7 @@ static int worker_new(struct worker **ret, Manager *manager, sd_device_monitor * .pid = pid, }; - r = hashmap_ensure_allocated(&manager->workers, NULL); + r = hashmap_ensure_allocated(&manager->workers, &worker_hash_op); if (r < 0) return r; @@ -296,7 +285,7 @@ static void manager_clear_for_worker(Manager *manager) { manager->event = sd_event_unref(manager->event); - manager_workers_free(manager); + manager->workers = hashmap_free(manager->workers); event_queue_cleanup(manager, EVENT_UNDEF); manager->monitor = sd_device_monitor_unref(manager->monitor); From 25d4f5b0716b668a22203ceb6961473e57931db3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 18 Dec 2018 15:11:24 +0900 Subject: [PATCH 4/7] udevd: reject devices which do not have SEQNUM --- src/udev/udevd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 837e13e0dc9..a26691ab9e1 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -399,7 +399,7 @@ static int worker_process_device(Manager *manager, sd_device *dev) { r = sd_device_get_property_value(dev, "SEQNUM", &seqnum); if (r < 0) - log_device_debug_errno(dev, r, "Failed to get SEQNUM: %m"); + return log_device_debug_errno(dev, r, "Failed to get SEQNUM: %m"); log_device_debug(dev, "Processing device (SEQNUM=%s)", seqnum); From c0ff3d6cbc55bc4193cdf185b0a43a28170e1668 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 18 Dec 2018 15:18:26 +0900 Subject: [PATCH 5/7] udevd: make worker also log ACTION property --- src/udev/udevd.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index a26691ab9e1..c43622ce43a 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -391,7 +391,7 @@ static int worker_lock_block_device(sd_device *dev, int *ret_fd) { static int worker_process_device(Manager *manager, sd_device *dev) { _cleanup_(udev_event_freep) UdevEvent *udev_event = NULL; _cleanup_close_ int fd_lock = -1; - const char *seqnum; + const char *seqnum, *action; int r; assert(manager); @@ -401,7 +401,11 @@ static int worker_process_device(Manager *manager, sd_device *dev) { if (r < 0) return log_device_debug_errno(dev, r, "Failed to get SEQNUM: %m"); - log_device_debug(dev, "Processing device (SEQNUM=%s)", seqnum); + r = sd_device_get_property_value(dev, "ACTION", &action); + if (r < 0) + return log_device_debug_errno(dev, r, "Failed to get ACTION: %m"); + + log_device_debug(dev, "Processing device (SEQNUM=%s, ACTION=%s)", seqnum, action); udev_event = udev_event_new(dev, arg_exec_delay_usec, manager->rtnl); if (!udev_event) @@ -427,7 +431,7 @@ static int worker_process_device(Manager *manager, sd_device *dev) { return log_device_debug_errno(dev, r, "Failed to update database under /run/udev/data/: %m"); } - log_device_debug(dev, "Device (SEQNUM=%s) processed", seqnum); + log_device_debug(dev, "Device (SEQNUM=%s, ACTION=%s) processed", seqnum, action); return 0; } From 33ad742a84ebad5eebfe1757512c4140045d37cf Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 18 Dec 2018 15:26:54 +0900 Subject: [PATCH 6/7] udevd: drop unnecessary brackets --- src/udev/udevd.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index c43622ce43a..d7d2ca10f09 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -870,7 +870,7 @@ static void event_queue_start(Manager *manager) { assert_se(sd_event_now(manager->event, CLOCK_MONOTONIC, &usec) >= 0); /* check for changed config, every 3 seconds at most */ if (manager->last_usec == 0 || - (usec - manager->last_usec) > 3 * USEC_PER_SEC) { + usec - manager->last_usec > 3 * USEC_PER_SEC) { if (udev_rules_check_timestamp(manager->rules) || udev_builtin_validate()) manager_reload(manager); @@ -955,12 +955,11 @@ static int on_worker(sd_event_source *s, int fd, uint32_t revents, void *userdat continue; } - CMSG_FOREACH(cmsg, &msghdr) { + CMSG_FOREACH(cmsg, &msghdr) if (cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_CREDENTIALS && cmsg->cmsg_len == CMSG_LEN(sizeof(struct ucred))) ucred = (struct ucred*) CMSG_DATA(cmsg); - } if (!ucred || ucred->pid <= 0) { log_warning("Ignoring worker message without valid PID"); @@ -1333,9 +1332,9 @@ static int on_sigchld(sd_event_source *s, const struct signalfd_siginfo *si, voi log_debug("Worker ["PID_FMT"] exited", pid); else log_warning("Worker ["PID_FMT"] exited with return code %i", pid, WEXITSTATUS(status)); - } else if (WIFSIGNALED(status)) { + } else if (WIFSIGNALED(status)) log_warning("Worker ["PID_FMT"] terminated by signal %i (%s)", pid, WTERMSIG(status), signal_to_string(WTERMSIG(status))); - } else if (WIFSTOPPED(status)) { + else if (WIFSTOPPED(status)) { log_info("Worker ["PID_FMT"] stopped", pid); continue; } else if (WIFCONTINUED(status)) { From e0b7a5d1510e2c2201b99ac6545eb34db1afe04f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 12 Jan 2019 09:31:56 +0900 Subject: [PATCH 7/7] udevd: refuse devices which do not have ACTION property --- src/udev/udevd.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index d7d2ca10f09..51f9bfdd3f6 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -587,8 +587,8 @@ static void event_run(Manager *manager, struct event *event) { static int event_queue_insert(Manager *manager, sd_device *dev) { _cleanup_(sd_device_unrefp) sd_device *clone = NULL; + const char *val, *action; struct event *event; - const char *val; uint64_t seqnum; int r; @@ -613,6 +613,11 @@ static int event_queue_insert(Manager *manager, sd_device *dev) { if (seqnum == 0) return -EINVAL; + /* Refuse devices do not have ACTION property. */ + r = sd_device_get_property_value(dev, "ACTION", &action); + if (r < 0) + return r; + /* Save original device to restore the state on failures. */ r = device_shallow_clone(dev, &clone); if (r < 0) @@ -642,12 +647,7 @@ static int event_queue_insert(Manager *manager, sd_device *dev) { LIST_APPEND(event, manager->events, event); - if (DEBUG_LOGGING) { - if (sd_device_get_property_value(dev, "ACTION", &val) < 0) - val = NULL; - - log_device_debug(dev, "Device (SEQNUM=%"PRIu64", ACTION=%s) is queued", seqnum, strnull(val)); - } + log_device_debug(dev, "Device (SEQNUM=%"PRIu64", ACTION=%s) is queued", seqnum, action); return 0; }