From 90d260d81f56adb831f51acfc086fcd0a7fb8712 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 21 Jan 2021 14:06:43 +0900 Subject: udev: add debugging logs in spawn_wait() --- src/udev/udev-event.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index 9fb5e4ae62..009a48020e 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -661,7 +661,7 @@ static int spawn_wait(Spawn *spawn) { r = sd_event_new(&e); if (r < 0) - return r; + return log_device_debug_errno(spawn->device, r, "Failed to allocate sd-event object: %m"); if (spawn->timeout_usec > 0) { usec_t usec, age_usec; @@ -678,7 +678,7 @@ static int spawn_wait(Spawn *spawn) { usec + spawn->timeout_warn_usec, USEC_PER_SEC, on_spawn_timeout_warning, spawn); if (r < 0) - return r; + return log_device_debug_errno(spawn->device, r, "Failed to create timeout warning event source: %m"); } spawn->timeout_usec -= age_usec; @@ -686,35 +686,35 @@ static int spawn_wait(Spawn *spawn) { r = sd_event_add_time(e, NULL, CLOCK_MONOTONIC, usec + spawn->timeout_usec, USEC_PER_SEC, on_spawn_timeout, spawn); if (r < 0) - return r; + return log_device_debug_errno(spawn->device, r, "Failed to create timeout event source: %m"); } } if (spawn->fd_stdout >= 0) { r = sd_event_add_io(e, &stdout_source, spawn->fd_stdout, EPOLLIN, on_spawn_io, spawn); if (r < 0) - return r; + return log_device_debug_errno(spawn->device, r, "Failed to create stdio event source: %m"); r = sd_event_source_set_enabled(stdout_source, SD_EVENT_ONESHOT); if (r < 0) - return r; + return log_device_debug_errno(spawn->device, r, "Failed to enable stdio event source: %m"); } if (spawn->fd_stderr >= 0) { r = sd_event_add_io(e, &stderr_source, spawn->fd_stderr, EPOLLIN, on_spawn_io, spawn); if (r < 0) - return r; + return log_device_debug_errno(spawn->device, r, "Failed to create stderr event source: %m"); r = sd_event_source_set_enabled(stderr_source, SD_EVENT_ONESHOT); if (r < 0) - return r; + return log_device_debug_errno(spawn->device, r, "Failed to enable stderr event source: %m"); } r = sd_event_add_child(e, &sigchld_source, spawn->pid, WEXITED, on_spawn_sigchld, spawn); if (r < 0) - return r; + return log_device_debug_errno(spawn->device, r, "Failed to create sigchild event source: %m"); /* SIGCHLD should be processed after IO is complete */ r = sd_event_source_set_priority(sigchld_source, SD_EVENT_PRIORITY_NORMAL + 1); if (r < 0) - return r; + return log_device_debug_errno(spawn->device, r, "Failed to set priority to sigchild event source: %m"); return sd_event_loop(e); } -- cgit v1.2.1 From 28a5065149c31a8689738ddcd1100502371a7c34 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 21 Jan 2021 14:14:11 +0900 Subject: udev: use strv_split_newlines() to parse result of spawned command --- src/udev/udev-rules.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index e72013ea56..a87c5d119e 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -1730,7 +1730,8 @@ static int udev_rule_apply_token_to_event( return token->op == OP_MATCH; } case TK_M_IMPORT_PROGRAM: { - char result[UDEV_LINE_SIZE], *line, *pos; + _cleanup_strv_free_ char **lines = NULL; + char result[UDEV_LINE_SIZE], **line; (void) udev_event_apply_format(event, token->value, buf, sizeof(buf), false); log_rule_debug(dev, rules, "Importing properties from results of '%s'", buf); @@ -1744,18 +1745,18 @@ static int udev_rule_apply_token_to_event( return token->op == OP_NOMATCH; } - for (line = result; !isempty(line); line = pos) { - char *key, *value; + lines = strv_split_newlines(result); + if (!lines) + return log_oom(); - pos = strchr(line, '\n'); - if (pos) - *pos++ = '\0'; + STRV_FOREACH(line, lines) { + char *key, *value; - r = get_property_from_string(line, &key, &value); + r = get_property_from_string(*line, &key, &value); if (r < 0) { log_rule_debug_errno(dev, rules, r, "Failed to parse key and value from '%s', ignoring: %m", - line); + *line); continue; } if (r == 0) -- cgit v1.2.1 From 2be0218a70a8d074edb998237ce41294d59040ca Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 2 Feb 2021 00:54:40 +0900 Subject: udev: ignore OOM error in on_spawn_io() --- src/udev/udev-event.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index 009a48020e..f55e65c95b 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -575,19 +575,19 @@ static int on_spawn_io(sd_event_source *s, int fd, uint32_t revents, void *userd v = strv_split_newlines(p); if (!v) - return 0; + log_oom_debug(); STRV_FOREACH(q, v) log_device_debug(spawn->device, "'%s'(%s) '%s'", spawn->cmd, fd == spawn->fd_stdout ? "out" : "err", *q); } - if (l == 0) return 0; - /* Re-enable the event source if we did not encounter EOF */ reenable: + /* Re-enable the event source if we did not encounter EOF */ + r = sd_event_source_set_enabled(s, SD_EVENT_ONESHOT); if (r < 0) log_device_error_errno(spawn->device, r, -- cgit v1.2.1 From ac714078bffc76e8b9b57ecdb595613d77a9b37e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 21 Jan 2021 14:15:37 +0900 Subject: test: add test for udev_event_spawn() --- src/udev/meson.build | 6 +++ src/udev/test-udev-event.c | 108 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+) create mode 100644 src/udev/test-udev-event.c diff --git a/src/udev/meson.build b/src/udev/meson.build index eee785179b..4a95c431a5 100644 --- a/src/udev/meson.build +++ b/src/udev/meson.build @@ -191,6 +191,12 @@ fuzzers += [ ] tests += [ + [['src/udev/test-udev-event.c'], + [libudevd_core, + libshared], + [threads, + libacl]], + [['src/udev/fido_id/test-fido-id-desc.c', 'src/udev/fido_id/fido_id_desc.c']], ] diff --git a/src/udev/test-udev-event.c b/src/udev/test-udev-event.c new file mode 100644 index 0000000000..584e5c27ed --- /dev/null +++ b/src/udev/test-udev-event.c @@ -0,0 +1,108 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "path-util.h" +#include "signal-util.h" +#include "strv.h" +#include "tests.h" +#include "udev-event.h" +#include "util.h" + +#define BUF_SIZE 1024 + +static void test_event_spawn_core(bool with_pidfd, const char *cmd, char result_buf[BUF_SIZE]) { + _cleanup_(sd_device_unrefp) sd_device *dev = NULL; + _cleanup_(udev_event_freep) UdevEvent *event = NULL; + + assert_se(setenv("SYSTEMD_PIDFD", yes_no(with_pidfd), 1) >= 0); + + assert_se(sd_device_new_from_syspath(&dev, "/sys/class/net/lo") >= 0); + assert_se(event = udev_event_new(dev, 0, NULL, LOG_DEBUG)); + assert_se(udev_event_spawn(event, 5 * USEC_PER_SEC, SIGKILL, false, cmd, result_buf, BUF_SIZE) == 0); + + assert_se(unsetenv("SYSTEMD_PIDFD") >= 0); +} + +static void test_event_spawn_cat(bool with_pidfd) { + _cleanup_strv_free_ char **lines = NULL; + _cleanup_free_ char *cmd = NULL; + char result_buf[BUF_SIZE]; + + log_debug("/* %s(%s) */", __func__, yes_no(with_pidfd)); + + assert_se(find_executable("cat", &cmd) >= 0); + assert_se(strextend_with_separator(&cmd, " ", "/sys/class/net/lo/uevent")); + + test_event_spawn_core(with_pidfd, cmd, result_buf); + + assert_se(lines = strv_split_newlines(result_buf)); + strv_print(lines); + + assert_se(strv_contains(lines, "INTERFACE=lo")); + assert_se(strv_contains(lines, "IFINDEX=1")); +} + +static void test_event_spawn_self(const char *self, const char *arg, bool with_pidfd) { + _cleanup_strv_free_ char **lines = NULL; + _cleanup_free_ char *cmd = NULL; + char result_buf[BUF_SIZE]; + + log_debug("/* %s(%s, %s) */", __func__, arg, yes_no(with_pidfd)); + + assert_se(cmd = strjoin(self, " ", arg)); + + test_event_spawn_core(with_pidfd, cmd, result_buf); + + assert_se(lines = strv_split_newlines(result_buf)); + strv_print(lines); + + assert_se(strv_contains(lines, "aaa")); + assert_se(strv_contains(lines, "bbb")); +} + +static void test1(void) { + fprintf(stdout, "aaa\nbbb"); + fprintf(stderr, "ccc\nddd"); +} + +static void test2(void) { + char buf[16384]; + + fprintf(stdout, "aaa\nbbb"); + + memset(buf, 'a', sizeof(buf) - 1); + char_array_0(buf); + fputs(buf, stderr); +} + +int main(int argc, char *argv[]) { + _cleanup_free_ char *self = NULL; + + if (argc > 1) { + if (streq(argv[1], "test1")) + test1(); + else if (streq(argv[1], "test2")) + test2(); + else + assert_not_reached("unknown command."); + + return 0; + } + + test_setup_logging(LOG_DEBUG); + + assert_se(sigprocmask_many(SIG_BLOCK, NULL, SIGCHLD, -1) >= 0); + + test_event_spawn_cat(true); + test_event_spawn_cat(false); + + assert_se(path_make_absolute_cwd(argv[0], &self) >= 0); + path_simplify(self, true); + + test_event_spawn_self(self, "test1", true); + test_event_spawn_self(self, "test1", false); + + test_event_spawn_self(self, "test2", true); + test_event_spawn_self(self, "test2", false); + + return 0; +} -- cgit v1.2.1 From 8b9708d1d39ca1007d0709d7d54588e0afc49994 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 21 Jan 2021 16:08:33 +0900 Subject: sd-event: retrieve more events when epoll_wait() returns number equivalent to the buffer size When epoll_wait() returns number equivalent to the buffer size, there may exist remaining events which may have higher priority. To make priority sorting correctly, let's retrieve all events. --- src/libsystemd/sd-event/sd-event.c | 44 ++++++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c index 05e21968ac..8abbefcf27 100644 --- a/src/libsystemd/sd-event/sd-event.c +++ b/src/libsystemd/sd-event/sd-event.c @@ -3781,8 +3781,8 @@ pending: } _public_ int sd_event_wait(sd_event *e, uint64_t timeout) { - size_t event_queue_max; - int r, m, i; + size_t n_event_queue, m; + int r, msec; assert_return(e, -EINVAL); assert_return(e = event_resolve(e), -ENOPKG); @@ -3795,29 +3795,45 @@ _public_ int sd_event_wait(sd_event *e, uint64_t timeout) { return 1; } - event_queue_max = MAX(e->n_sources, 1u); - if (!GREEDY_REALLOC(e->event_queue, e->event_queue_allocated, event_queue_max)) + n_event_queue = MAX(e->n_sources, 1u); + if (!GREEDY_REALLOC(e->event_queue, e->event_queue_allocated, n_event_queue)) return -ENOMEM; /* If we still have inotify data buffered, then query the other fds, but don't wait on it */ if (e->inotify_data_buffered) - timeout = 0; + msec = 0; + else + msec = timeout == (uint64_t) -1 ? -1 : (int) DIV_ROUND_UP(timeout, USEC_PER_MSEC); - m = epoll_wait(e->epoll_fd, e->event_queue, event_queue_max, - timeout == (uint64_t) -1 ? -1 : (int) DIV_ROUND_UP(timeout, USEC_PER_MSEC)); - if (m < 0) { - if (errno == EINTR) { - e->state = SD_EVENT_PENDING; - return 1; + for (;;) { + r = epoll_wait(e->epoll_fd, e->event_queue, e->event_queue_allocated, msec); + if (r < 0) { + if (errno == EINTR) { + e->state = SD_EVENT_PENDING; + return 1; + } + + r = -errno; + goto finish; } - r = -errno; - goto finish; + m = (size_t) r; + + if (m < e->event_queue_allocated) + break; + + if (e->event_queue_allocated >= n_event_queue * 10) + break; + + if (!GREEDY_REALLOC(e->event_queue, e->event_queue_allocated, e->event_queue_allocated + n_event_queue)) + return -ENOMEM; + + msec = 0; } triple_timestamp_get(&e->timestamp); - for (i = 0; i < m; i++) { + for (size_t i = 0; i < m; i++) { if (e->event_queue[i].data.ptr == INT_TO_PTR(SOURCE_WATCHDOG)) r = flush_timer(e, e->watchdog_fd, e->event_queue[i].events, NULL); -- cgit v1.2.1