From 665dfe93185a1decdb422f9e5b65a7e39415f129 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 12 Dec 2017 23:21:09 +0100 Subject: [PATCH 01/46] io-util: make flush_fd() return how many bytes where flushed This is useful so that callers know whether anything at all and how much was flushed. This patches through users of this functions to ensure that the return values > 0 which may be returned now are not propagated in public APIs. Also, users that ignore the return value are changed to do so explicitly now. --- src/basic/io-util.c | 9 ++++++--- src/core/manager.c | 2 +- src/libsystemd/sd-login/sd-login.c | 7 ++++++- src/libudev/libudev-queue.c | 10 +++++++++- src/shared/ask-password-api.c | 2 +- src/tty-ask-password-agent/tty-ask-password-agent.c | 2 +- 6 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/basic/io-util.c b/src/basic/io-util.c index 77c9bdc739d..08ad42ed952 100644 --- a/src/basic/io-util.c +++ b/src/basic/io-util.c @@ -33,6 +33,7 @@ int flush_fd(int fd) { .fd = fd, .events = POLLIN, }; + int count = 0; /* Read from the specified file descriptor, until POLLIN is not set anymore, throwing away everything * read. Note that some file descriptors (notable IP sockets) will trigger POLLIN even when no data can be read @@ -52,7 +53,7 @@ int flush_fd(int fd) { return -errno; } else if (r == 0) - return 0; + return count; l = read(fd, buf, sizeof(buf)); if (l < 0) { @@ -61,11 +62,13 @@ int flush_fd(int fd) { continue; if (errno == EAGAIN) - return 0; + return count; return -errno; } else if (l == 0) - return 0; + return count; + + count += (int) l; } } diff --git a/src/core/manager.c b/src/core/manager.c index 15720ada24f..860d58617fa 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -263,7 +263,7 @@ static int manager_dispatch_ask_password_fd(sd_event_source *source, assert(m); - flush_fd(fd); + (void) flush_fd(fd); m->have_ask_password = have_ask_password(); if (m->have_ask_password < 0) diff --git a/src/libsystemd/sd-login/sd-login.c b/src/libsystemd/sd-login/sd-login.c index e8adaa68232..69572d1a515 100644 --- a/src/libsystemd/sd-login/sd-login.c +++ b/src/libsystemd/sd-login/sd-login.c @@ -1061,10 +1061,15 @@ _public_ sd_login_monitor* sd_login_monitor_unref(sd_login_monitor *m) { } _public_ int sd_login_monitor_flush(sd_login_monitor *m) { + int r; assert_return(m, -EINVAL); - return flush_fd(MONITOR_TO_FD(m)); + r = flush_fd(MONITOR_TO_FD(m)); + if (r < 0) + return r; + + return 0; } _public_ int sd_login_monitor_get_fd(sd_login_monitor *m) { diff --git a/src/libudev/libudev-queue.c b/src/libudev/libudev-queue.c index b941afb7739..85ceb263a3a 100644 --- a/src/libudev/libudev-queue.c +++ b/src/libudev/libudev-queue.c @@ -268,8 +268,16 @@ _public_ int udev_queue_get_fd(struct udev_queue *udev_queue) { * Returns: the result of clearing the watch for queue changes. */ _public_ int udev_queue_flush(struct udev_queue *udev_queue) { + int r; + + assert(udev_queue); + if (udev_queue->fd < 0) return -EINVAL; - return flush_fd(udev_queue->fd); + r = flush_fd(udev_queue->fd); + if (r < 0) + return r; + + return 0; } diff --git a/src/shared/ask-password-api.c b/src/shared/ask-password-api.c index 346d8c9d24e..99d6a9b143a 100644 --- a/src/shared/ask-password-api.c +++ b/src/shared/ask-password-api.c @@ -316,7 +316,7 @@ int ask_password_tty( } if (notify >= 0 && pollfd[POLL_INOTIFY].revents != 0) - flush_fd(notify); + (void) flush_fd(notify); if (pollfd[POLL_TTY].revents == 0) continue; diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c index 6e9c10aeb03..d2648ead931 100644 --- a/src/tty-ask-password-agent/tty-ask-password-agent.c +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c @@ -159,7 +159,7 @@ static int ask_password_plymouth( } if (notify >= 0 && pollfd[POLL_INOTIFY].revents != 0) - flush_fd(notify); + (void) flush_fd(notify); if (pollfd[POLL_SOCKET].revents == 0) continue; From e32fd6b47c157643a4f127954a34deb97b26ca37 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 12 Dec 2017 23:23:25 +0100 Subject: [PATCH 02/46] sd-bus: when debug logging about messages, show the same bits of it everywhere Also, include the message signature everywhere. --- src/libsystemd/sd-bus/bus-internal.c | 15 ++++++++++----- src/libsystemd/sd-bus/sd-bus.c | 6 ++++-- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-internal.c b/src/libsystemd/sd-bus/bus-internal.c index 3c381b0ffef..05a022fbf3c 100644 --- a/src/libsystemd/sd-bus/bus-internal.c +++ b/src/libsystemd/sd-bus/bus-internal.c @@ -362,13 +362,18 @@ int bus_maybe_reply_error(sd_bus_message *m, int r, sd_bus_error *error) { } else return r; - log_debug("Failed to process message [type=%s sender=%s path=%s interface=%s member=%s signature=%s]: %s", + log_debug("Failed to process message type=%s sender=%s destination=%s path=%s interface=%s member=%s cookie=%" PRIu64 " reply_cookie=%" PRIu64 " signature=%s error-name=%s error-message=%s: %s", bus_message_type_to_string(m->header->type), - strna(m->sender), - strna(m->path), - strna(m->interface), - strna(m->member), + strna(sd_bus_message_get_sender(m)), + strna(sd_bus_message_get_destination(m)), + strna(sd_bus_message_get_path(m)), + strna(sd_bus_message_get_interface(m)), + strna(sd_bus_message_get_member(m)), + BUS_MESSAGE_COOKIE(m), + m->reply_cookie, strna(m->root_container.signature), + strna(m->error.name), + strna(m->error.message), bus_error_message(error, r)); return 1; diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 3cfc60fe865..5111222dadd 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -57,7 +57,7 @@ #define log_debug_bus_message(m) \ do { \ sd_bus_message *_mm = (m); \ - log_debug("Got message type=%s sender=%s destination=%s object=%s interface=%s member=%s cookie=%" PRIu64 " reply_cookie=%" PRIu64 " error-name=%s error-message=%s", \ + log_debug("Got message type=%s sender=%s destination=%s path=%s interface=%s member=%s cookie=%" PRIu64 " reply_cookie=%" PRIu64 " signature=%s error-name=%s error-message=%s", \ bus_message_type_to_string(_mm->header->type), \ strna(sd_bus_message_get_sender(_mm)), \ strna(sd_bus_message_get_destination(_mm)), \ @@ -66,6 +66,7 @@ strna(sd_bus_message_get_member(_mm)), \ BUS_MESSAGE_COOKIE(_mm), \ _mm->reply_cookie, \ + strna(_mm->root_container.signature), \ strna(_mm->error.name), \ strna(_mm->error.message)); \ } while (false) @@ -1467,7 +1468,7 @@ static int bus_write_message(sd_bus *bus, sd_bus_message *m, bool hint_sync_call return r; if (*idx >= BUS_MESSAGE_SIZE(m)) - log_debug("Sent message type=%s sender=%s destination=%s object=%s interface=%s member=%s cookie=%" PRIu64 " reply_cookie=%" PRIu64 " error-name=%s error-message=%s", + log_debug("Sent message type=%s sender=%s destination=%s path=%s interface=%s member=%s cookie=%" PRIu64 " reply_cookie=%" PRIu64 " signature=%s error-name=%s error-message=%s", bus_message_type_to_string(m->header->type), strna(sd_bus_message_get_sender(m)), strna(sd_bus_message_get_destination(m)), @@ -1476,6 +1477,7 @@ static int bus_write_message(sd_bus *bus, sd_bus_message *m, bool hint_sync_call strna(sd_bus_message_get_member(m)), BUS_MESSAGE_COOKIE(m), m->reply_cookie, + strna(m->root_container.signature), strna(m->error.name), strna(m->error.message)); From 6ae22ffb723f5b15a7cf5fb71895a202b29d4210 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 12 Dec 2017 23:24:44 +0100 Subject: [PATCH 03/46] sd-bus: cast some syscall invocations explicitly to (void) Let's clarify that we knowingly ignore the return values. --- src/libsystemd/sd-bus/bus-socket.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-socket.c b/src/libsystemd/sd-bus/bus-socket.c index 0a1d36a9d3d..5034d51472d 100644 --- a/src/libsystemd/sd-bus/bus-socket.c +++ b/src/libsystemd/sd-bus/bus-socket.c @@ -592,8 +592,8 @@ void bus_socket_setup(sd_bus *b) { assert(b); /* Increase the buffers to 8 MB */ - fd_inc_rcvbuf(b->input_fd, SNDBUF_SIZE); - fd_inc_sndbuf(b->output_fd, SNDBUF_SIZE); + (void) fd_inc_rcvbuf(b->input_fd, SNDBUF_SIZE); + (void) fd_inc_sndbuf(b->output_fd, SNDBUF_SIZE); b->message_version = 1; b->message_endian = 0; @@ -742,10 +742,10 @@ int bus_socket_exec(sd_bus *b) { if (!IN_SET(s[1], STDIN_FILENO, STDOUT_FILENO)) safe_close(s[1]); - fd_cloexec(STDIN_FILENO, false); - fd_cloexec(STDOUT_FILENO, false); - fd_nonblock(STDIN_FILENO, false); - fd_nonblock(STDOUT_FILENO, false); + (void) fd_cloexec(STDIN_FILENO, false); + (void) fd_cloexec(STDOUT_FILENO, false); + (void) fd_nonblock(STDIN_FILENO, false); + (void) fd_nonblock(STDOUT_FILENO, false); if (b->exec_argv) execvp(b->exec_path, b->exec_argv); From b33652fe9117466a2b05bbc698cd1037f439dd9b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 12 Dec 2017 23:26:08 +0100 Subject: [PATCH 04/46] sd-bus: minor coding style fix --- src/libsystemd/sd-bus/sd-bus.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 5111222dadd..9daa758a1c4 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -3253,12 +3253,9 @@ _public_ int sd_bus_default(sd_bus **ret) { /* No type is specified, so we have not other option than to * use the starter address if it is set. */ - e = secure_getenv("DBUS_STARTER_ADDRESS"); - if (e) { - + if (e) return bus_default(sd_bus_open, &default_starter_bus, ret); - } /* Finally, if nothing is set use the cached connection for * the right scope */ From b057498a5275c8734b7e430f7d9b1dd13243c98a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 12 Dec 2017 23:26:36 +0100 Subject: [PATCH 05/46] sd-bus: propagate handling errors for Hello method reply directly Currently, when sd-bus is used to issue a method call, and we get a reply and the specified reply handler fails, we log this locally at debug priority and proceed. The idea is that a bad server-side reply should not be fatal for the program, except when the developer explicitly terminates the event loop. The reply to the initial Hello() method call we issue when joining a bus should not be handled like that however. Instead, propagate the error immediately, as anything that is wrong with the Hello() reply should be considered a fatal connection problem. --- src/libsystemd/sd-bus/sd-bus.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 9daa758a1c4..a99c993dd10 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -2059,6 +2059,7 @@ static int process_timeout(sd_bus *bus) { _cleanup_(sd_bus_message_unrefp) sd_bus_message* m = NULL; struct reply_callback *c; sd_bus_slot *slot; + bool is_hello; usec_t n; int r; @@ -2094,6 +2095,8 @@ static int process_timeout(sd_bus *bus) { bus->iteration_counter++; + is_hello = bus->state == BUS_HELLO && c->callback == hello_callback; + bus->current_message = m; bus->current_slot = sd_bus_slot_ref(slot); bus->current_handler = c->callback; @@ -2111,6 +2114,11 @@ static int process_timeout(sd_bus *bus) { sd_bus_slot_unref(slot); + /* When this is the hello message and it failed, then make sure to propagate the error up, don't just log and + * ignore the callback handler's return value. */ + if (is_hello) + return r; + return bus_maybe_reply_error(m, r, &error_buffer); } @@ -2140,6 +2148,7 @@ static int process_reply(sd_bus *bus, sd_bus_message *m) { _cleanup_(sd_bus_error_free) sd_bus_error error_buffer = SD_BUS_ERROR_NULL; struct reply_callback *c; sd_bus_slot *slot; + bool is_hello; int r; assert(bus); @@ -2193,6 +2202,8 @@ static int process_reply(sd_bus *bus, sd_bus_message *m) { c->timeout = 0; } + is_hello = bus->state == BUS_HELLO && c->callback == hello_callback; + bus->current_slot = sd_bus_slot_ref(slot); bus->current_handler = c->callback; bus->current_userdata = slot->userdata; @@ -2208,6 +2219,11 @@ static int process_reply(sd_bus *bus, sd_bus_message *m) { sd_bus_slot_unref(slot); + /* When this is the hello message and it timed out, then make sure to propagate the error up, don't just log + * and ignore the callback handler's return value. */ + if (is_hello) + return r; + return bus_maybe_reply_error(m, r, &error_buffer); } From 5ae37ad833583e6c1c7765767b7f8360afca3b07 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Dec 2017 22:24:16 +0100 Subject: [PATCH 06/46] sd-bus: when attached to an sd-event loop, disconnect on processing errors If we can't process the bus for some reason we shouldn't just disable the event source, but log something and give up on the connection. Hence do that, and disconnect. --- src/libsystemd/sd-bus/sd-bus.c | 45 +++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index a99c993dd10..0866a09d1f8 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -2967,8 +2967,10 @@ static int io_callback(sd_event_source *s, int fd, uint32_t revents, void *userd assert(bus); r = sd_bus_process(bus, NULL); - if (r < 0) - return r; + if (r < 0) { + log_debug_errno(r, "Processing of bus failed, closing down: %m"); + bus_enter_closing(bus); + } return 1; } @@ -2980,8 +2982,10 @@ static int time_callback(sd_event_source *s, uint64_t usec, void *userdata) { assert(bus); r = sd_bus_process(bus, NULL); - if (r < 0) - return r; + if (r < 0) { + log_debug_errno(r, "Processing of bus failed, closing down: %m"); + bus_enter_closing(bus); + } return 1; } @@ -2995,38 +2999,45 @@ static int prepare_callback(sd_event_source *s, void *userdata) { assert(bus); e = sd_bus_get_events(bus); - if (e < 0) - return e; + if (e < 0) { + r = e; + goto fail; + } if (bus->output_fd != bus->input_fd) { r = sd_event_source_set_io_events(bus->input_io_event_source, e & POLLIN); if (r < 0) - return r; + goto fail; r = sd_event_source_set_io_events(bus->output_io_event_source, e & POLLOUT); - if (r < 0) - return r; - } else { + } else r = sd_event_source_set_io_events(bus->input_io_event_source, e); - if (r < 0) - return r; - } + if (r < 0) + goto fail; r = sd_bus_get_timeout(bus, &until); if (r < 0) - return r; + goto fail; if (r > 0) { int j; j = sd_event_source_set_time(bus->time_event_source, until); - if (j < 0) - return j; + if (j < 0) { + r = j; + goto fail; + } } r = sd_event_source_set_enabled(bus->time_event_source, r > 0); if (r < 0) - return r; + goto fail; + + return 1; + +fail: + log_debug_errno(r, "Preparing of bus events failed, closing down: %m"); + bus_enter_closing(bus); return 1; } From 9e3fa6e82773cb1d8687453ef0ba0ad9e5dda7d2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 27 Dec 2017 16:20:28 +0100 Subject: [PATCH 07/46] fs-util: rework touch_file() so that it can touch socket file nodes Let's rework touch_file() so that it works correctly on sockets, fifos, and device nodes: let's open an O_PATH file descriptor first and operate based on that, if we can. This is usually the better option as it this means we can open AF_UNIX nodes in the file system, and update their timestamps and ownership correctly. It also means we can correctly touch symlinks and block/character devices without triggering their drivers. Moreover, by operating on an O_PATH fd we can make sure that we operate on the same inode the whole time, and it can't be swapped out in the middle. While we are at it, rework the call so that we try to adjust as much as we can before returning on error. This is a good idea as we call the function quite often without checking its result, and hence it's best to leave the files around in the most "correct" fashion possible. --- src/basic/fs-util.c | 55 +++++++++++++++++--------- src/test/test-fs-util.c | 87 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 19 deletions(-) diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index 4ca36faf094..16f97893c95 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -315,43 +315,60 @@ int fd_warn_permissions(const char *path, int fd) { } int touch_file(const char *path, bool parents, usec_t stamp, uid_t uid, gid_t gid, mode_t mode) { - _cleanup_close_ int fd; - int r; + char fdpath[STRLEN("/proc/self/fd/") + DECIMAL_STR_MAX(int)]; + _cleanup_close_ int fd = -1; + int r, ret = 0; assert(path); + /* Note that touch_file() does not follow symlinks: if invoked on an existing symlink, then it is the symlink + * itself which is updated, not its target + * + * Returns the first error we encounter, but tries to apply as much as possible. */ + if (parents) - mkdir_parents(path, 0755); + (void) mkdir_parents(path, 0755); - fd = open(path, O_WRONLY|O_CREAT|O_CLOEXEC|O_NOCTTY, - IN_SET(mode, 0, MODE_INVALID) ? 0644 : mode); - if (fd < 0) - return -errno; + /* Initially, we try to open the node with O_PATH, so that we get a reference to the node. This is useful in + * case the path refers to an existing device or socket node, as we can open it successfully in all cases, and + * won't trigger any driver magic or so. */ + fd = open(path, O_PATH|O_CLOEXEC|O_NOFOLLOW); + if (fd < 0) { + if (errno != ENOENT) + return -errno; - if (mode != MODE_INVALID) { - r = fchmod(fd, mode); - if (r < 0) + /* if the node doesn't exist yet, we create it, but with O_EXCL, so that we only create a regular file + * here, and nothing else */ + fd = open(path, O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, IN_SET(mode, 0, MODE_INVALID) ? 0644 : mode); + if (fd < 0) return -errno; } - if (uid != UID_INVALID || gid != GID_INVALID) { - r = fchown(fd, uid, gid); - if (r < 0) - return -errno; - } + /* Let's make a path from the fd, and operate on that. With this logic, we can adjust the access mode, + * ownership and time of the file node in all cases, even if the fd refers to an O_PATH object — which is + * something fchown(), fchmod(), futimensat() don't allow. */ + xsprintf(fdpath, "/proc/self/fd/%i", fd); + + if (mode != MODE_INVALID) + if (chmod(fdpath, mode) < 0) + ret = -errno; + + if (uid_is_valid(uid) || gid_is_valid(gid)) + if (chown(fdpath, uid, gid) < 0 && ret >= 0) + ret = -errno; if (stamp != USEC_INFINITY) { struct timespec ts[2]; timespec_store(&ts[0], stamp); ts[1] = ts[0]; - r = futimens(fd, ts); + r = utimensat(AT_FDCWD, fdpath, ts, 0); } else - r = futimens(fd, NULL); - if (r < 0) + r = utimensat(AT_FDCWD, fdpath, NULL, 0); + if (r < 0 && ret >= 0) return -errno; - return 0; + return ret; } int touch(const char *path) { diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c index 86d963c4c78..2ad9e718563 100644 --- a/src/test/test-fs-util.c +++ b/src/test/test-fs-util.c @@ -388,6 +388,92 @@ static void test_access_fd(void) { } } +static void test_touch_file(void) { + uid_t test_uid, test_gid; + _cleanup_(rm_rf_physical_and_freep) char *p = NULL; + struct stat st; + const char *a; + usec_t test_mtime; + + test_uid = geteuid() == 0 ? 65534 : getuid(); + test_gid = geteuid() == 0 ? 65534 : getgid(); + + test_mtime = usec_sub_unsigned(now(CLOCK_REALTIME), USEC_PER_WEEK); + + assert_se(mkdtemp_malloc("/dev/shm/touch-file-XXXXXX", &p) >= 0); + + a = strjoina(p, "/regular"); + assert_se(touch_file(a, false, test_mtime, test_uid, test_gid, 0640) >= 0); + assert_se(lstat(a, &st) >= 0); + assert_se(st.st_uid == test_uid); + assert_se(st.st_gid == test_gid); + assert_se(S_ISREG(st.st_mode)); + assert_se((st.st_mode & 0777) == 0640); + assert_se(timespec_load(&st.st_mtim) == test_mtime); + + a = strjoina(p, "/dir"); + assert_se(mkdir(a, 0775) >= 0); + assert_se(touch_file(a, false, test_mtime, test_uid, test_gid, 0640) >= 0); + assert_se(lstat(a, &st) >= 0); + assert_se(st.st_uid == test_uid); + assert_se(st.st_gid == test_gid); + assert_se(S_ISDIR(st.st_mode)); + assert_se((st.st_mode & 0777) == 0640); + assert_se(timespec_load(&st.st_mtim) == test_mtime); + + a = strjoina(p, "/fifo"); + assert_se(mkfifo(a, 0775) >= 0); + assert_se(touch_file(a, false, test_mtime, test_uid, test_gid, 0640) >= 0); + assert_se(lstat(a, &st) >= 0); + assert_se(st.st_uid == test_uid); + assert_se(st.st_gid == test_gid); + assert_se(S_ISFIFO(st.st_mode)); + assert_se((st.st_mode & 0777) == 0640); + assert_se(timespec_load(&st.st_mtim) == test_mtime); + + a = strjoina(p, "/sock"); + assert_se(mknod(a, 0775 | S_IFSOCK, 0) >= 0); + assert_se(touch_file(a, false, test_mtime, test_uid, test_gid, 0640) >= 0); + assert_se(lstat(a, &st) >= 0); + assert_se(st.st_uid == test_uid); + assert_se(st.st_gid == test_gid); + assert_se(S_ISSOCK(st.st_mode)); + assert_se((st.st_mode & 0777) == 0640); + assert_se(timespec_load(&st.st_mtim) == test_mtime); + + if (geteuid() == 0) { + a = strjoina(p, "/cdev"); + assert_se(mknod(a, 0775 | S_IFCHR, makedev(0, 0)) >= 0); + assert_se(touch_file(a, false, test_mtime, test_uid, test_gid, 0640) >= 0); + assert_se(lstat(a, &st) >= 0); + assert_se(st.st_uid == test_uid); + assert_se(st.st_gid == test_gid); + assert_se(S_ISCHR(st.st_mode)); + assert_se((st.st_mode & 0777) == 0640); + assert_se(timespec_load(&st.st_mtim) == test_mtime); + + a = strjoina(p, "/bdev"); + assert_se(mknod(a, 0775 | S_IFBLK, makedev(0, 0)) >= 0); + assert_se(touch_file(a, false, test_mtime, test_uid, test_gid, 0640) >= 0); + assert_se(lstat(a, &st) >= 0); + assert_se(st.st_uid == test_uid); + assert_se(st.st_gid == test_gid); + assert_se(S_ISBLK(st.st_mode)); + assert_se((st.st_mode & 0777) == 0640); + assert_se(timespec_load(&st.st_mtim) == test_mtime); + } + + a = strjoina(p, "/lnk"); + assert_se(symlink("target", a) >= 0); + assert_se(touch_file(a, false, test_mtime, test_uid, test_gid, 0640) >= 0); + assert_se(lstat(a, &st) >= 0); + assert_se(st.st_uid == test_uid); + assert_se(st.st_gid == test_gid); + assert_se(S_ISLNK(st.st_mode)); + assert_se((st.st_mode & 0777) == 0640); + assert_se(timespec_load(&st.st_mtim) == test_mtime); +} + int main(int argc, char *argv[]) { test_unlink_noerrno(); test_get_files_in_directory(); @@ -396,6 +482,7 @@ int main(int argc, char *argv[]) { test_chase_symlinks(); test_dot_or_dot_dot(); test_access_fd(); + test_touch_file(); return 0; } From 294d46f138e67f9e1bfb18f5ec1b009a4a9d5292 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 27 Dec 2017 16:50:24 +0100 Subject: [PATCH 08/46] socket-label: simplify things a bit by using socket_address_get_path() Let's make this more generic and descriptive, and let's reuse our existing utility functions. --- src/basic/socket-label.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/basic/socket-label.c b/src/basic/socket-label.c index 20be406371c..448265b1cd3 100644 --- a/src/basic/socket-label.c +++ b/src/basic/socket-label.c @@ -29,6 +29,7 @@ #include "alloc-util.h" #include "fd-util.h" +#include "fs-util.h" #include "log.h" #include "macro.h" #include "missing.h" @@ -51,6 +52,7 @@ int socket_address_listen( const char *label) { _cleanup_close_ int fd = -1; + const char *p; int r, one; assert(a); @@ -112,16 +114,17 @@ int socket_address_listen( if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one)) < 0) return -errno; - if (socket_address_family(a) == AF_UNIX && a->sockaddr.un.sun_path[0] != 0) { + p = socket_address_get_path(a); + if (p) { /* Create parents */ - (void) mkdir_parents_label(a->sockaddr.un.sun_path, directory_mode); + (void) mkdir_parents_label(p, directory_mode); /* Enforce the right access mode for the socket */ RUN_WITH_UMASK(~socket_mode) { r = mac_selinux_bind(fd, &a->sockaddr.sa, a->size); if (r == -EADDRINUSE) { /* Unlink and try again */ - unlink(a->sockaddr.un.sun_path); + (void) unlink(p); if (bind(fd, &a->sockaddr.sa, a->size) < 0) return -errno; } else if (r < 0) From 706d7c27ad12098a44d83330eab1aef8f8ed12dc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 27 Dec 2017 16:59:44 +0100 Subject: [PATCH 09/46] socket-label: tweak socket_address_listen() a bit This changes two things when binding to AF_UNIX file system sockets: 1. When wethe socket already exists in the fs, and unlink() on it fails, don't bother to bind() a second time: since nothing changed it won't work either. 2. Also use SELinux-aware bind() for the second attempt. --- src/basic/socket-label.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/basic/socket-label.c b/src/basic/socket-label.c index 448265b1cd3..e67a5cfe6a6 100644 --- a/src/basic/socket-label.c +++ b/src/basic/socket-label.c @@ -124,10 +124,13 @@ int socket_address_listen( r = mac_selinux_bind(fd, &a->sockaddr.sa, a->size); if (r == -EADDRINUSE) { /* Unlink and try again */ - (void) unlink(p); - if (bind(fd, &a->sockaddr.sa, a->size) < 0) - return -errno; - } else if (r < 0) + + if (unlink(p) < 0) + return r; /* didn't work, return original error */ + + r = mac_selinux_bind(fd, &a->sockaddr.sa, a->size); + } + if (r < 0) return r; } } else { From 89220e2fb6d49df31bed4205c9c84883749483bc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 27 Dec 2017 17:02:36 +0100 Subject: [PATCH 10/46] socket-util: use parse_ip_port() for parsing IP ports Let's unify some code here, and also use parse_ip_port() for all our IP port parsing needs in socket_address_parse(). --- src/basic/socket-util.c | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/src/basic/socket-util.c b/src/basic/socket-util.c index cb10a1dd07a..d6371f833d4 100644 --- a/src/basic/socket-util.c +++ b/src/basic/socket-util.c @@ -68,7 +68,6 @@ DEFINE_STRING_TABLE_LOOKUP(socket_address_type, int); int socket_address_parse(SocketAddress *a, const char *s) { char *e, *n; - unsigned u; int r; assert(a); @@ -78,6 +77,8 @@ int socket_address_parse(SocketAddress *a, const char *s) { a->type = SOCK_STREAM; if (*s == '[') { + uint16_t port; + /* IPv6 in [x:.....:z]:p notation */ e = strchr(s+1, ']'); @@ -95,15 +96,12 @@ int socket_address_parse(SocketAddress *a, const char *s) { return -EINVAL; e++; - r = safe_atou(e, &u); + r = parse_ip_port(e, &port); if (r < 0) return r; - if (u <= 0 || u > 0xFFFF) - return -EINVAL; - a->sockaddr.in6.sin6_family = AF_INET6; - a->sockaddr.in6.sin6_port = htobe16((uint16_t)u); + a->sockaddr.in6.sin6_port = htobe16(port); a->size = sizeof(struct sockaddr_in6); } else if (*s == '/') { @@ -134,12 +132,13 @@ int socket_address_parse(SocketAddress *a, const char *s) { } else if (startswith(s, "vsock:")) { /* AF_VSOCK socket in vsock:cid:port notation */ const char *cid_start = s + STRLEN("vsock:"); + unsigned port; e = strchr(cid_start, ':'); if (!e) return -EINVAL; - r = safe_atou(e+1, &u); + r = safe_atou(e+1, &port); if (r < 0) return r; @@ -152,19 +151,18 @@ int socket_address_parse(SocketAddress *a, const char *s) { a->sockaddr.vm.svm_cid = VMADDR_CID_ANY; a->sockaddr.vm.svm_family = AF_VSOCK; - a->sockaddr.vm.svm_port = u; + a->sockaddr.vm.svm_port = port; a->size = sizeof(struct sockaddr_vm); } else { + uint16_t port; + e = strchr(s, ':'); if (e) { - r = safe_atou(e+1, &u); + r = parse_ip_port(e + 1, &port); if (r < 0) return r; - if (u <= 0 || u > 0xFFFF) - return -EINVAL; - n = strndupa(s, e-s); /* IPv4 in w.x.y.z:p notation? */ @@ -175,7 +173,7 @@ int socket_address_parse(SocketAddress *a, const char *s) { if (r > 0) { /* Gotcha, it's a traditional IPv4 address */ a->sockaddr.in.sin_family = AF_INET; - a->sockaddr.in.sin_port = htobe16((uint16_t)u); + a->sockaddr.in.sin_port = htobe16(port); a->size = sizeof(struct sockaddr_in); } else { unsigned idx; @@ -189,7 +187,7 @@ int socket_address_parse(SocketAddress *a, const char *s) { return -EINVAL; a->sockaddr.in6.sin6_family = AF_INET6; - a->sockaddr.in6.sin6_port = htobe16((uint16_t)u); + a->sockaddr.in6.sin6_port = htobe16(port); a->sockaddr.in6.sin6_scope_id = idx; a->sockaddr.in6.sin6_addr = in6addr_any; a->size = sizeof(struct sockaddr_in6); @@ -197,21 +195,18 @@ int socket_address_parse(SocketAddress *a, const char *s) { } else { /* Just a port */ - r = safe_atou(s, &u); + r = parse_ip_port(s, &port); if (r < 0) return r; - if (u <= 0 || u > 0xFFFF) - return -EINVAL; - if (socket_ipv6_is_supported()) { a->sockaddr.in6.sin6_family = AF_INET6; - a->sockaddr.in6.sin6_port = htobe16((uint16_t)u); + a->sockaddr.in6.sin6_port = htobe16(port); a->sockaddr.in6.sin6_addr = in6addr_any; a->size = sizeof(struct sockaddr_in6); } else { a->sockaddr.in.sin_family = AF_INET; - a->sockaddr.in.sin_port = htobe16((uint16_t)u); + a->sockaddr.in.sin_port = htobe16(port); a->sockaddr.in.sin_addr.s_addr = INADDR_ANY; a->size = sizeof(struct sockaddr_in); } From 8b7f989a5824869a5ca9983b225758a8c2fd8f4a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 27 Dec 2017 17:18:02 +0100 Subject: [PATCH 11/46] socket-util: explicitly ensure there's one trailing NUL byte on AF_UNIX socket addresses AF_UNIX socket addresses aren't necessarily NUL terminated, however they are usually used as strings which are assumed to be NUL terminated. Let's hence add an extra byte to the end of the sockaddr_un structure, that contains this NUL byte, simply for safety reasons. Note that actually this patch changes exactly nothing IRL, as the other sockaddr structures already are large enough to accomodate for an extra NUL byte. The size of the union hence doesn't change at all by doing this. The entire value of this patch is hence in the philosophical feeling of safety, and by making something explicit that before was implicit. --- src/basic/socket-util.h | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/basic/socket-util.h b/src/basic/socket-util.h index 83af91dbef2..49c937aef59 100644 --- a/src/basic/socket-util.h +++ b/src/basic/socket-util.h @@ -36,16 +36,26 @@ #include "util.h" union sockaddr_union { + /* The minimal, abstract version */ struct sockaddr sa; + + /* The libc provided version that allocates "enough room" for every protocol */ + struct sockaddr_storage storage; + + /* Protoctol-specific implementations */ struct sockaddr_in in; struct sockaddr_in6 in6; struct sockaddr_un un; struct sockaddr_nl nl; - struct sockaddr_storage storage; struct sockaddr_ll ll; struct sockaddr_vm vm; + /* Ensure there is enough space to store Infiniband addresses */ uint8_t ll_buffer[offsetof(struct sockaddr_ll, sll_addr) + CONST_MAX(ETH_ALEN, INFINIBAND_ALEN)]; + + /* Ensure there is enough space after the AF_UNIX sun_path for one more NUL byte, just to be sure that the path + * component is always followed by at least one NUL byte. */ + uint8_t un_buffer[sizeof(struct sockaddr_un) + 1]; }; typedef struct SocketAddress { From 5b5e6deabb8b76bcd691c4e40429bd8478a49a32 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 27 Dec 2017 18:22:31 +0100 Subject: [PATCH 12/46] bus: touch() the AF_UNIX sockets we listen() on after the fact We'd like to use inotify to get notified when AF_UNIX sockets become connectable. That happens at the moment of listen(), but this is doesn't necessarily create in a watchable inotify event. Hence, let's synthesize one whenever we generically create a socket, or when we know we created it for a D-Bus server. Ideally we wouldn't have to do this, and the kernel would generate an event anyway for this. Doing this explicitly isn't too bad however, as the event is still nicely associated with the AF_UNIX socket node, and we generate all D-Bus sockets in our code hence it's safe. --- src/basic/socket-label.c | 5 +++++ src/core/dbus.c | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/src/basic/socket-label.c b/src/basic/socket-label.c index e67a5cfe6a6..97f3ebe2af1 100644 --- a/src/basic/socket-label.c +++ b/src/basic/socket-label.c @@ -142,6 +142,11 @@ int socket_address_listen( if (listen(fd, backlog) < 0) return -errno; + /* Let's trigger an inotify event on the socket node, so that anyone waiting for this socket to be connectable + * gets notified */ + if (p) + (void) touch(p); + r = fd; fd = -1; diff --git a/src/core/dbus.c b/src/core/dbus.c index b7d8af9396a..115d071eb42 100644 --- a/src/core/dbus.c +++ b/src/core/dbus.c @@ -37,6 +37,7 @@ #include "dbus-unit.h" #include "dbus.h" #include "fd-util.h" +#include "fs-util.h" #include "log.h" #include "missing.h" #include "mkdir.h" @@ -1005,6 +1006,9 @@ static int bus_init_private(Manager *m) { if (r < 0) return log_error_errno(errno, "Failed to make private socket listening: %m"); + /* Generate an inotify event in case somebody waits for this socket to appear using inotify() */ + (void) touch(sa.un.sun_path); + r = sd_event_add_io(m->event, &s, fd, EPOLLIN, bus_on_connection, m); if (r < 0) return log_error_errno(r, "Failed to allocate event source: %m"); From 8a5cd31e5fa0b1313a196b902e4b3c4603e7dfdf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Dec 2017 22:24:52 +0100 Subject: [PATCH 13/46] sd-bus: optionally, use inotify to wait for bus sockets to appear This adds a "watch-bind" feature to sd-bus connections. If set and the AF_UNIX socket we are connecting to doesn't exist yet, we'll establish an inotify watch instead, and wait for the socket to appear. In other words, a missing AF_UNIX just makes connecting slower. This is useful for daemons such as networkd or resolved that shall be able to run during early-boot, before dbus-daemon is up, and want to connect to dbus-daemon as soon as it becomes ready. --- src/libsystemd/libsystemd.sym | 6 + src/libsystemd/sd-bus/bus-internal.h | 13 + src/libsystemd/sd-bus/bus-socket.c | 279 +++++++++++++++- src/libsystemd/sd-bus/bus-socket.h | 1 + src/libsystemd/sd-bus/sd-bus.c | 337 +++++++++++++------- src/libsystemd/sd-bus/test-bus-watch-bind.c | 239 ++++++++++++++ src/systemd/sd-bus.h | 4 +- src/test/meson.build | 4 + 8 files changed, 760 insertions(+), 123 deletions(-) create mode 100644 src/libsystemd/sd-bus/test-bus-watch-bind.c diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index 1a29b03e855..4229e0aeb18 100644 --- a/src/libsystemd/libsystemd.sym +++ b/src/libsystemd/libsystemd.sym @@ -530,3 +530,9 @@ global: sd_bus_message_new; sd_bus_message_seal; } LIBSYSTEMD_234; + +LIBSYSTEMD_237 { +global: + sd_bus_set_watch_bind; + sd_bus_get_watch_bind; +} LIBSYSTEMD_236; diff --git a/src/libsystemd/sd-bus/bus-internal.h b/src/libsystemd/sd-bus/bus-internal.h index 4175ca3efe0..629d8b3f370 100644 --- a/src/libsystemd/sd-bus/bus-internal.h +++ b/src/libsystemd/sd-bus/bus-internal.h @@ -157,6 +157,7 @@ struct sd_bus_slot { enum bus_state { BUS_UNSET, + BUS_WATCH_BIND, /* waiting for the socket to appear via inotify */ BUS_OPENING, BUS_AUTHENTICATING, BUS_HELLO, @@ -188,6 +189,7 @@ struct sd_bus { enum bus_state state; int input_fd, output_fd; + int inotify_fd; int message_version; int message_endian; @@ -210,6 +212,7 @@ struct sd_bus { bool exited:1; bool exit_triggered:1; bool is_local:1; + bool watch_bind:1; int use_memfd; @@ -293,6 +296,7 @@ struct sd_bus { sd_event_source *output_io_event_source; sd_event_source *time_event_source; sd_event_source *quit_event_source; + sd_event_source *inotify_event_source; sd_event *event; int event_priority; @@ -312,6 +316,9 @@ struct sd_bus { LIST_HEAD(sd_bus_slot, slots); LIST_HEAD(sd_bus_track, tracks); + + int *inotify_watches; + size_t n_inotify_watches; }; /* For method calls we time-out at 25s, like in the D-Bus reference implementation */ @@ -367,6 +374,12 @@ bool bus_pid_changed(sd_bus *bus); char *bus_address_escape(const char *v); +int bus_attach_io_events(sd_bus *b); +int bus_attach_inotify_event(sd_bus *b); + +void bus_close_inotify_fd(sd_bus *b); +void bus_close_io_fds(sd_bus *b); + #define OBJECT_PATH_FOREACH_PREFIX(prefix, path) \ for (char *_slash = ({ strcpy((prefix), (path)); streq((prefix), "/") ? NULL : strrchr((prefix), '/'); }) ; \ _slash && !(_slash[(_slash) == (prefix)] = 0); \ diff --git a/src/libsystemd/sd-bus/bus-socket.c b/src/libsystemd/sd-bus/bus-socket.c index 5034d51472d..9291fed0e75 100644 --- a/src/libsystemd/sd-bus/bus-socket.c +++ b/src/libsystemd/sd-bus/bus-socket.c @@ -32,9 +32,12 @@ #include "bus-socket.h" #include "fd-util.h" #include "format-util.h" +#include "fs-util.h" #include "hexdecoct.h" +#include "io-util.h" #include "macro.h" #include "missing.h" +#include "path-util.h" #include "selinux-util.h" #include "signal-util.h" #include "stdio-util.h" @@ -688,30 +691,249 @@ int bus_socket_start_auth(sd_bus *b) { return bus_socket_start_auth_client(b); } +static int bus_socket_inotify_setup(sd_bus *b) { + _cleanup_free_ int *new_watches = NULL; + _cleanup_free_ char *absolute = NULL; + size_t n_allocated = 0, n = 0, done = 0, i; + unsigned max_follow = 32; + const char *p; + int wd, r; + + assert(b); + assert(b->watch_bind); + assert(b->sockaddr.sa.sa_family == AF_UNIX); + assert(b->sockaddr.un.sun_path[0] != 0); + + /* Sets up an inotify fd in case watch_bind is enabled: wait until the configured AF_UNIX file system socket + * appears before connecting to it. The implemented is pretty simplistic: we just subscribe to relevant changes + * to all prefix components of the path, and every time we get an event for that we try to reconnect again, + * without actually caring what precisely the event we got told us. If we still can't connect we re-subscribe + * to all relevant changes of anything in the path, so that our watches include any possibly newly created path + * components. */ + + if (b->inotify_fd < 0) { + b->inotify_fd = inotify_init1(IN_NONBLOCK|IN_CLOEXEC); + if (b->inotify_fd < 0) + return -errno; + } + + /* Make sure the path is NUL terminated */ + p = strndupa(b->sockaddr.un.sun_path, sizeof(b->sockaddr.un.sun_path)); + + /* Make sure the path is absolute */ + r = path_make_absolute_cwd(p, &absolute); + if (r < 0) + goto fail; + + /* Watch all parent directories, and don't mind any prefix that doesn't exist yet. For the innermost directory + * that exists we want to know when files are created or moved into it. For all parents of it we just care if + * they are removed or renamed. */ + + if (!GREEDY_REALLOC(new_watches, n_allocated, n + 1)) { + r = -ENOMEM; + goto fail; + } + + /* Start with the top-level directory, which is a bit simpler than the rest, since it can't be a symlink, and + * always exists */ + wd = inotify_add_watch(b->inotify_fd, "/", IN_CREATE|IN_MOVED_TO); + if (wd < 0) { + r = log_debug_errno(errno, "Failed to add inotify watch on /: %m"); + goto fail; + } else + new_watches[n++] = wd; + + for (;;) { + _cleanup_free_ char *component = NULL, *prefix = NULL, *destination = NULL; + size_t n_slashes, n_component; + char *c = NULL; + + n_slashes = strspn(absolute + done, "/"); + n_component = n_slashes + strcspn(absolute + done + n_slashes, "/"); + + if (n_component == 0) /* The end */ + break; + + component = strndup(absolute + done, n_component); + if (!component) { + r = -ENOMEM; + goto fail; + } + + /* A trailing slash? That's a directory, and not a socket then */ + if (path_equal(component, "/")) { + r = -EISDIR; + goto fail; + } + + /* A single dot? Let's eat this up */ + if (path_equal(component, "/.")) { + done += n_component; + continue; + } + + prefix = strndup(absolute, done + n_component); + if (!prefix) { + r = -ENOMEM; + goto fail; + } + + if (!GREEDY_REALLOC(new_watches, n_allocated, n + 1)) { + r = -ENOMEM; + goto fail; + } + + wd = inotify_add_watch(b->inotify_fd, prefix, IN_DELETE_SELF|IN_MOVE_SELF|IN_ATTRIB|IN_CREATE|IN_MOVED_TO|IN_DONT_FOLLOW); + log_debug("Added inotify watch for %s on bus %s: %i", prefix, strna(b->description), wd); + + if (wd < 0) { + if (IN_SET(errno, ENOENT, ELOOP)) + break; /* This component doesn't exist yet, or the path contains a cyclic symlink right now */ + + r = log_debug_errno(errno, "Failed to add inotify watch on %s: %m", isempty(prefix) ? "/" : prefix); + goto fail; + } else + new_watches[n++] = wd; + + /* Check if this is possibly a symlink. If so, let's follow it and watch it too. */ + r = readlink_malloc(prefix, &destination); + if (r == -EINVAL) { /* not a symlink */ + done += n_component; + continue; + } + if (r < 0) + goto fail; + + if (isempty(destination)) { /* Empty symlink target? Yuck! */ + r = -EINVAL; + goto fail; + } + + if (max_follow <= 0) { /* Let's make sure we don't follow symlinks forever */ + r = -ELOOP; + goto fail; + } + + if (path_is_absolute(destination)) { + /* For absolute symlinks we build the new path and start anew */ + c = strjoin(destination, absolute + done + n_component); + done = 0; + } else { + _cleanup_free_ char *t = NULL; + + /* For relative symlinks we replace the last component, and try again */ + t = strndup(absolute, done); + if (!t) + return -ENOMEM; + + c = strjoin(t, "/", destination, absolute + done + n_component); + } + if (!c) { + r = -ENOMEM; + goto fail; + } + + free(absolute); + absolute = c; + + max_follow--; + } + + /* And now, let's remove all watches from the previous iteration we don't need anymore */ + for (i = 0; i < b->n_inotify_watches; i++) { + bool found = false; + size_t j; + + for (j = 0; j < n; j++) + if (new_watches[j] == b->inotify_watches[i]) { + found = true; + break; + } + + if (found) + continue; + + (void) inotify_rm_watch(b->inotify_fd, b->inotify_watches[i]); + } + + free_and_replace(b->inotify_watches, new_watches); + b->n_inotify_watches = n; + + return 0; + +fail: + bus_close_inotify_fd(b); + return r; +} + int bus_socket_connect(sd_bus *b) { + bool inotify_done = false; int r; assert(b); - assert(b->input_fd < 0); - assert(b->output_fd < 0); - assert(b->sockaddr.sa.sa_family != AF_UNSPEC); - b->input_fd = socket(b->sockaddr.sa.sa_family, SOCK_STREAM|SOCK_CLOEXEC|SOCK_NONBLOCK, 0); - if (b->input_fd < 0) - return -errno; + for (;;) { + assert(b->input_fd < 0); + assert(b->output_fd < 0); + assert(b->sockaddr.sa.sa_family != AF_UNSPEC); - b->output_fd = b->input_fd; + b->input_fd = socket(b->sockaddr.sa.sa_family, SOCK_STREAM|SOCK_CLOEXEC|SOCK_NONBLOCK, 0); + if (b->input_fd < 0) + return -errno; - bus_socket_setup(b); + b->output_fd = b->input_fd; + bus_socket_setup(b); - r = connect(b->input_fd, &b->sockaddr.sa, b->sockaddr_size); - if (r < 0) { - if (errno == EINPROGRESS) - return 1; + if (connect(b->input_fd, &b->sockaddr.sa, b->sockaddr_size) < 0) { + if (errno == EINPROGRESS) { - return -errno; + /* If we have any inotify watches open, close them now, we don't need them anymore, as + * we have successfully initiated a connection */ + bus_close_inotify_fd(b); + + /* Note that very likely we are already in BUS_OPENING state here, as we enter it when + * we start parsing the address string. The only reason we set the state explicitly + * here, is to undo BUS_WATCH_BIND, in case we did the inotify magic. */ + b->state = BUS_OPENING; + return 1; + } + + if (IN_SET(errno, ENOENT, ECONNREFUSED) && /* ENOENT → unix socket doesn't exist at all; ECONNREFUSED → unix socket stale */ + b->watch_bind && + b->sockaddr.sa.sa_family == AF_UNIX && + b->sockaddr.un.sun_path[0] != 0) { + + /* This connection attempt failed, let's release the socket for now, and start with a + * fresh one when reconnecting. */ + bus_close_io_fds(b); + + if (inotify_done) { + /* inotify set up already, don't do it again, just return now, and remember + * that we are waiting for inotify events now. */ + b->state = BUS_WATCH_BIND; + return 1; + } + + /* This is a file system socket, and the inotify logic is enabled. Let's create the necessary inotify fd. */ + r = bus_socket_inotify_setup(b); + if (r < 0) + return r; + + /* Let's now try to connect a second time, because in theory there's otherwise a race + * here: the socket might have been created in the time between our first connect() and + * the time we set up the inotify logic. But let's remember that we set up inotify now, + * so that we don't do the connect() more than twice. */ + inotify_done = true; + + } else + return -errno; + } else + break; } + /* Yay, established, we don't need no inotify anymore! */ + bus_close_inotify_fd(b); + return bus_socket_start_auth(b); } @@ -1069,3 +1291,34 @@ int bus_socket_process_authenticating(sd_bus *b) { return bus_socket_read_auth(b); } + +int bus_socket_process_watch_bind(sd_bus *b) { + int r, q; + + assert(b); + assert(b->state == BUS_WATCH_BIND); + assert(b->inotify_fd >= 0); + + r = flush_fd(b->inotify_fd); + if (r <= 0) + return r; + + log_debug("Got inotify event on bus %s.", strna(b->description)); + + /* We flushed events out of the inotify fd. In that case, maybe the socket is valid now? Let's try to connect + * to it again */ + + r = bus_socket_connect(b); + if (r < 0) + return r; + + q = bus_attach_io_events(b); + if (q < 0) + return q; + + q = bus_attach_inotify_event(b); + if (q < 0) + return q; + + return r; +} diff --git a/src/libsystemd/sd-bus/bus-socket.h b/src/libsystemd/sd-bus/bus-socket.h index 915a283f5ad..c180562f981 100644 --- a/src/libsystemd/sd-bus/bus-socket.h +++ b/src/libsystemd/sd-bus/bus-socket.h @@ -34,5 +34,6 @@ int bus_socket_read_message(sd_bus *bus); int bus_socket_process_opening(sd_bus *b); int bus_socket_process_authenticating(sd_bus *b); +int bus_socket_process_watch_bind(sd_bus *b); bool bus_socket_auth_needs_write(sd_bus *b); diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 0866a09d1f8..3b65f51d04c 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -72,23 +72,33 @@ } while (false) static int bus_poll(sd_bus *bus, bool need_more, uint64_t timeout_usec); -static int attach_io_events(sd_bus *b); -static void detach_io_events(sd_bus *b); +static void bus_detach_io_events(sd_bus *b); +static void bus_detach_inotify_event(sd_bus *b); static thread_local sd_bus *default_system_bus = NULL; static thread_local sd_bus *default_user_bus = NULL; static thread_local sd_bus *default_starter_bus = NULL; -static void bus_close_fds(sd_bus *b) { +void bus_close_io_fds(sd_bus *b) { assert(b); - detach_io_events(b); + bus_detach_io_events(b); if (b->input_fd != b->output_fd) safe_close(b->output_fd); b->output_fd = b->input_fd = safe_close(b->input_fd); } +void bus_close_inotify_fd(sd_bus *b) { + assert(b); + + bus_detach_inotify_event(b); + + b->inotify_fd = safe_close(b->inotify_fd); + b->inotify_watches = mfree(b->inotify_watches); + b->n_inotify_watches = 0; +} + static void bus_reset_queues(sd_bus *b) { assert(b); @@ -132,7 +142,8 @@ static void bus_free(sd_bus *b) { if (b->default_bus_ptr) *b->default_bus_ptr = NULL; - bus_close_fds(b); + bus_close_io_fds(b); + bus_close_inotify_fd(b); free(b->label); free(b->groups); @@ -182,6 +193,7 @@ _public_ int sd_bus_new(sd_bus **ret) { r->n_ref = REFCNT_INIT; r->input_fd = r->output_fd = -1; + r->inotify_fd = -1; r->message_version = 1; r->creds_mask |= SD_BUS_CREDS_WELL_KNOWN_NAMES|SD_BUS_CREDS_UNIQUE_NAME; r->hello_flags |= KDBUS_HELLO_ACCEPT_FD; @@ -379,6 +391,22 @@ _public_ int sd_bus_get_allow_interactive_authorization(sd_bus *bus) { return bus->allow_interactive_authorization; } +_public_ int sd_bus_set_watch_bind(sd_bus *bus, int b) { + assert_return(bus, -EINVAL); + assert_return(bus->state == BUS_UNSET, -EPERM); + assert_return(!bus_pid_changed(bus), -ECHILD); + + bus->watch_bind = b; + return 0; +} + +_public_ int sd_bus_get_watch_bind(sd_bus *bus) { + assert_return(bus, -EINVAL); + assert_return(!bus_pid_changed(bus), -ECHILD); + + return bus->watch_bind; +} + static int hello_callback(sd_bus_message *reply, void *userdata, sd_bus_error *error) { const char *s; sd_bus *bus; @@ -901,7 +929,8 @@ static int bus_start_address(sd_bus *b) { assert(b); for (;;) { - bus_close_fds(b); + bus_close_io_fds(b); + bus_close_inotify_fd(b); /* If you provide multiple different bus-addresses, we * try all of them in order and use the first one that @@ -909,20 +938,25 @@ static int bus_start_address(sd_bus *b) { if (b->exec_path) r = bus_socket_exec(b); - else if ((b->nspid > 0 || b->machine) && b->sockaddr.sa.sa_family != AF_UNSPEC) r = bus_container_connect_socket(b); - else if (b->sockaddr.sa.sa_family != AF_UNSPEC) r = bus_socket_connect(b); - else goto next; if (r >= 0) { - r = attach_io_events(b); - if (r >= 0) - return r; + int q; + + q = bus_attach_io_events(b); + if (q < 0) + return q; + + q = bus_attach_inotify_event(b); + if (q < 0) + return q; + + return r; } b->last_connect_error = -r; @@ -1305,7 +1339,8 @@ _public_ void sd_bus_close(sd_bus *bus) { * the bus object and the bus may be freed */ bus_reset_queues(bus); - bus_close_fds(bus); + bus_close_io_fds(bus); + bus_close_inotify_fd(bus); } _public_ sd_bus* sd_bus_flush_close_unref(sd_bus *bus) { @@ -1322,7 +1357,7 @@ _public_ sd_bus* sd_bus_flush_close_unref(sd_bus *bus) { static void bus_enter_closing(sd_bus *bus) { assert(bus); - if (!IN_SET(bus->state, BUS_OPENING, BUS_AUTHENTICATING, BUS_HELLO, BUS_RUNNING)) + if (!IN_SET(bus->state, BUS_WATCH_BIND, BUS_OPENING, BUS_AUTHENTICATING, BUS_HELLO, BUS_RUNNING)) return; bus->state = BUS_CLOSING; @@ -1973,7 +2008,16 @@ _public_ int sd_bus_get_fd(sd_bus *bus) { assert_return(bus->input_fd == bus->output_fd, -EPERM); assert_return(!bus_pid_changed(bus), -ECHILD); - return bus->input_fd; + if (bus->state == BUS_CLOSED) + return -ENOTCONN; + + if (bus->inotify_fd >= 0) + return bus->inotify_fd; + + if (bus->input_fd >= 0) + return bus->input_fd; + + return -ENOTCONN; } _public_ int sd_bus_get_events(sd_bus *bus) { @@ -1982,23 +2026,40 @@ _public_ int sd_bus_get_events(sd_bus *bus) { assert_return(bus, -EINVAL); assert_return(!bus_pid_changed(bus), -ECHILD); - if (!BUS_IS_OPEN(bus->state) && bus->state != BUS_CLOSING) + switch (bus->state) { + + case BUS_UNSET: + case BUS_CLOSED: return -ENOTCONN; - if (bus->state == BUS_OPENING) - flags |= POLLOUT; - else if (bus->state == BUS_AUTHENTICATING) { + case BUS_WATCH_BIND: + flags |= POLLIN; + break; + case BUS_OPENING: + flags |= POLLOUT; + break; + + case BUS_AUTHENTICATING: if (bus_socket_auth_needs_write(bus)) flags |= POLLOUT; flags |= POLLIN; + break; - } else if (IN_SET(bus->state, BUS_RUNNING, BUS_HELLO)) { + case BUS_RUNNING: + case BUS_HELLO: if (bus->rqueue_size <= 0) flags |= POLLIN; if (bus->wqueue_size > 0) flags |= POLLOUT; + break; + + case BUS_CLOSING: + break; + + default: + assert_not_reached("Unknown state"); } return flags; @@ -2019,39 +2080,45 @@ _public_ int sd_bus_get_timeout(sd_bus *bus, uint64_t *timeout_usec) { return 1; } - if (bus->state == BUS_CLOSING) { - *timeout_usec = 0; - return 1; - } + switch (bus->state) { - if (bus->state == BUS_AUTHENTICATING) { + case BUS_AUTHENTICATING: *timeout_usec = bus->auth_timeout; return 1; - } - if (!IN_SET(bus->state, BUS_RUNNING, BUS_HELLO)) { - *timeout_usec = (uint64_t) -1; - return 0; - } + case BUS_RUNNING: + case BUS_HELLO: + if (bus->rqueue_size > 0) { + *timeout_usec = 0; + return 1; + } - if (bus->rqueue_size > 0) { + c = prioq_peek(bus->reply_callbacks_prioq); + if (!c) { + *timeout_usec = (uint64_t) -1; + return 0; + } + + if (c->timeout == 0) { + *timeout_usec = (uint64_t) -1; + return 0; + } + + *timeout_usec = c->timeout; + return 1; + + case BUS_CLOSING: *timeout_usec = 0; return 1; - } - c = prioq_peek(bus->reply_callbacks_prioq); - if (!c) { + case BUS_WATCH_BIND: + case BUS_OPENING: *timeout_usec = (uint64_t) -1; return 0; - } - if (c->timeout == 0) { - *timeout_usec = (uint64_t) -1; - return 0; + default: + assert_not_reached("Unknown or unexpected stat"); } - - *timeout_usec = c->timeout; - return 1; } static int process_timeout(sd_bus *bus) { @@ -2114,8 +2181,8 @@ static int process_timeout(sd_bus *bus) { sd_bus_slot_unref(slot); - /* When this is the hello message and it failed, then make sure to propagate the error up, don't just log and - * ignore the callback handler's return value. */ + /* When this is the hello message and it timed out, then make sure to propagate the error up, don't just log + * and ignore the callback handler's return value. */ if (is_hello) return r; @@ -2219,8 +2286,8 @@ static int process_reply(sd_bus *bus, sd_bus_message *m) { sd_bus_slot_unref(slot); - /* When this is the hello message and it timed out, then make sure to propagate the error up, don't just log - * and ignore the callback handler's return value. */ + /* When this is the hello message and it failed, then make sure to propagate the error up, don't just log and + * ignore the callback handler's return value. */ if (is_hello) return r; @@ -2656,48 +2723,44 @@ static int bus_process_internal(sd_bus *bus, bool hint_priority, int64_t priorit case BUS_CLOSED: return -ECONNRESET; + case BUS_WATCH_BIND: + r = bus_socket_process_watch_bind(bus); + break; + case BUS_OPENING: r = bus_socket_process_opening(bus); - if (IN_SET(r, -ENOTCONN, -ECONNRESET, -EPIPE, -ESHUTDOWN)) { - bus_enter_closing(bus); - r = 1; - } else if (r < 0) - return r; - if (ret) - *ret = NULL; - return r; + break; case BUS_AUTHENTICATING: r = bus_socket_process_authenticating(bus); - if (IN_SET(r, -ENOTCONN, -ECONNRESET, -EPIPE, -ESHUTDOWN)) { - bus_enter_closing(bus); - r = 1; - } else if (r < 0) - return r; - - if (ret) - *ret = NULL; - - return r; + break; case BUS_RUNNING: case BUS_HELLO: r = process_running(bus, hint_priority, priority, ret); - if (IN_SET(r, -ENOTCONN, -ECONNRESET, -EPIPE, -ESHUTDOWN)) { - bus_enter_closing(bus); - r = 1; + if (r >= 0) + return r; - if (ret) - *ret = NULL; - } - - return r; + /* This branch initializes *ret, hence we don't use the generic error checking below */ + break; case BUS_CLOSING: return process_closing(bus, ret); + + default: + assert_not_reached("Unknown state"); } - assert_not_reached("Unknown state"); + if (IN_SET(r, -ENOTCONN, -ECONNRESET, -EPIPE, -ESHUTDOWN)) { + bus_enter_closing(bus); + r = 1; + } else if (r < 0) + return r; + + if (ret) + *ret = NULL; + + return r; } _public_ int sd_bus_process(sd_bus *bus, sd_bus_message **ret) { @@ -2710,7 +2773,7 @@ _public_ int sd_bus_process_priority(sd_bus *bus, int64_t priority, sd_bus_messa static int bus_poll(sd_bus *bus, bool need_more, uint64_t timeout_usec) { struct pollfd p[2] = {}; - int r, e, n; + int r, n; struct timespec ts; usec_t m = USEC_INFINITY; @@ -2722,45 +2785,52 @@ static int bus_poll(sd_bus *bus, bool need_more, uint64_t timeout_usec) { if (!BUS_IS_OPEN(bus->state)) return -ENOTCONN; - e = sd_bus_get_events(bus); - if (e < 0) - return e; + if (bus->state == BUS_WATCH_BIND) { + assert(bus->inotify_fd >= 0); - if (need_more) - /* The caller really needs some more data, he doesn't - * care about what's already read, or any timeouts - * except its own. */ - e |= POLLIN; - else { - usec_t until; - /* The caller wants to process if there's something to - * process, but doesn't care otherwise */ + p[0].events = POLLIN; + p[0].fd = bus->inotify_fd; + n = 1; + } else { + int e; - r = sd_bus_get_timeout(bus, &until); - if (r < 0) - return r; - if (r > 0) { - usec_t nw; - nw = now(CLOCK_MONOTONIC); - m = until > nw ? until - nw : 0; + e = sd_bus_get_events(bus); + if (e < 0) + return e; + + if (need_more) + /* The caller really needs some more data, he doesn't + * care about what's already read, or any timeouts + * except its own. */ + e |= POLLIN; + else { + usec_t until; + /* The caller wants to process if there's something to + * process, but doesn't care otherwise */ + + r = sd_bus_get_timeout(bus, &until); + if (r < 0) + return r; + if (r > 0) + m = usec_sub_unsigned(until, now(CLOCK_MONOTONIC)); + } + + p[0].fd = bus->input_fd; + if (bus->output_fd == bus->input_fd) { + p[0].events = e; + n = 1; + } else { + p[0].events = e & POLLIN; + p[1].fd = bus->output_fd; + p[1].events = e & POLLOUT; + n = 2; } } - if (timeout_usec != (uint64_t) -1 && (m == (uint64_t) -1 || timeout_usec < m)) + if (timeout_usec != (uint64_t) -1 && (m == USEC_INFINITY || timeout_usec < m)) m = timeout_usec; - p[0].fd = bus->input_fd; - if (bus->output_fd == bus->input_fd) { - p[0].events = e; - n = 1; - } else { - p[0].events = e & POLLIN; - p[1].fd = bus->output_fd; - p[1].events = e & POLLOUT; - n = 2; - } - - r = ppoll(p, n, m == (uint64_t) -1 ? NULL : timespec_store(&ts, m), NULL); + r = ppoll(p, n, m == USEC_INFINITY ? NULL : timespec_store(&ts, m), NULL); if (r < 0) return -errno; @@ -2796,6 +2866,10 @@ _public_ int sd_bus_flush(sd_bus *bus) { if (!BUS_IS_OPEN(bus->state)) return -ENOTCONN; + /* We never were connected? Don't hang in inotify for good, as there's no timeout set for it */ + if (bus->state == BUS_WATCH_BIND) + return -EUNATCH; + r = bus_ensure_running(bus); if (r < 0) return r; @@ -2966,6 +3040,8 @@ static int io_callback(sd_event_source *s, int fd, uint32_t revents, void *userd assert(bus); + /* Note that this is called both on input_fd, output_fd as well as inotify_fd events */ + r = sd_bus_process(bus, NULL); if (r < 0) { log_debug_errno(r, "Processing of bus failed, closing down: %m"); @@ -3053,7 +3129,7 @@ static int quit_callback(sd_event_source *event, void *userdata) { return 1; } -static int attach_io_events(sd_bus *bus) { +int bus_attach_io_events(sd_bus *bus) { int r; assert(bus); @@ -3107,7 +3183,7 @@ static int attach_io_events(sd_bus *bus) { return 0; } -static void detach_io_events(sd_bus *bus) { +static void bus_detach_io_events(sd_bus *bus) { assert(bus); if (bus->input_io_event_source) { @@ -3121,6 +3197,44 @@ static void detach_io_events(sd_bus *bus) { } } +int bus_attach_inotify_event(sd_bus *bus) { + int r; + + assert(bus); + + if (bus->inotify_fd < 0) + return 0; + + if (!bus->event) + return 0; + + if (!bus->inotify_event_source) { + r = sd_event_add_io(bus->event, &bus->inotify_event_source, bus->inotify_fd, EPOLLIN, io_callback, bus); + if (r < 0) + return r; + + r = sd_event_source_set_priority(bus->inotify_event_source, bus->event_priority); + if (r < 0) + return r; + + r = sd_event_source_set_description(bus->inotify_event_source, "bus-inotify"); + } else + r = sd_event_source_set_io_fd(bus->inotify_event_source, bus->inotify_fd); + if (r < 0) + return r; + + return 0; +} + +static void bus_detach_inotify_event(sd_bus *bus) { + assert(bus); + + if (bus->inotify_event_source) { + sd_event_source_set_enabled(bus->inotify_event_source, SD_EVENT_OFF); + bus->inotify_event_source = sd_event_source_unref(bus->inotify_event_source); + } +} + _public_ int sd_bus_attach_event(sd_bus *bus, sd_event *event, int priority) { int r; @@ -3161,7 +3275,11 @@ _public_ int sd_bus_attach_event(sd_bus *bus, sd_event *event, int priority) { if (r < 0) goto fail; - r = attach_io_events(bus); + r = bus_attach_io_events(bus); + if (r < 0) + goto fail; + + r = bus_attach_inotify_event(bus); if (r < 0) goto fail; @@ -3178,7 +3296,8 @@ _public_ int sd_bus_detach_event(sd_bus *bus) { if (!bus->event) return 0; - detach_io_events(bus); + bus_detach_io_events(bus); + bus_detach_inotify_event(bus); if (bus->time_event_source) { sd_event_source_set_enabled(bus->time_event_source, SD_EVENT_OFF); diff --git a/src/libsystemd/sd-bus/test-bus-watch-bind.c b/src/libsystemd/sd-bus/test-bus-watch-bind.c new file mode 100644 index 00000000000..aef5ba9486b --- /dev/null +++ b/src/libsystemd/sd-bus/test-bus-watch-bind.c @@ -0,0 +1,239 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ +/*** + This file is part of systemd. + + Copyright 2017 Lennart Poettering + + systemd is free software; you can redistribute it and/or modify it + under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2.1 of the License, or + (at your option) any later version. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with systemd; If not, see . +***/ + +#include + +#include "sd-bus.h" +#include "sd-event.h" +#include "sd-id128.h" + +#include "alloc-util.h" +#include "fd-util.h" +#include "fileio.h" +#include "fs-util.h" +#include "mkdir.h" +#include "path-util.h" +#include "random-util.h" +#include "rm-rf.h" +#include "socket-util.h" +#include "string-util.h" + +static int method_foobar(sd_bus_message *m, void *userdata, sd_bus_error *ret_error) { + log_info("Got Foobar() call."); + + assert_se(sd_event_exit(sd_bus_get_event(sd_bus_message_get_bus(m)), 0) >= 0); + return sd_bus_reply_method_return(m, NULL); +} + +static int method_exit(sd_bus_message *m, void *userdata, sd_bus_error *ret_error) { + log_info("Got Exit() call"); + assert_se(sd_event_exit(sd_bus_get_event(sd_bus_message_get_bus(m)), 1) >= 0); + return sd_bus_reply_method_return(m, NULL); +} + +static const sd_bus_vtable vtable[] = { + SD_BUS_VTABLE_START(0), + SD_BUS_METHOD("Foobar", NULL, NULL, method_foobar, SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD("Exit", NULL, NULL, method_exit, SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_VTABLE_END, +}; + +static void* thread_server(void *p) { + _cleanup_free_ char *suffixed = NULL, *suffixed2 = NULL, *d = NULL; + _cleanup_close_ int fd = -1; + union sockaddr_union u = { + .un.sun_family = AF_UNIX, + }; + const char *path = p; + + log_debug("Initializing server"); + + /* Let's play some games, by slowly creating the socket directory, and renaming it in the middle */ + (void) usleep(100 * USEC_PER_MSEC); + + assert_se(mkdir_parents(path, 0755) >= 0); + (void) usleep(100 * USEC_PER_MSEC); + + d = dirname_malloc(path); + assert_se(d); + assert_se(asprintf(&suffixed, "%s.%" PRIx64, d, random_u64()) >= 0); + assert_se(rename(d, suffixed) >= 0); + (void) usleep(100 * USEC_PER_MSEC); + + assert_se(asprintf(&suffixed2, "%s.%" PRIx64, d, random_u64()) >= 0); + assert_se(symlink(suffixed2, d) >= 0); + (void) usleep(100 * USEC_PER_MSEC); + + assert_se(symlink(basename(suffixed), suffixed2) >= 0); + (void) usleep(100 * USEC_PER_MSEC); + + strncpy(u.un.sun_path, path, sizeof(u.un.sun_path)); + + fd = socket(AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0); + assert_se(fd >= 0); + + assert_se(bind(fd, &u.sa, SOCKADDR_UN_LEN(u.un)) >= 0); + usleep(100 * USEC_PER_MSEC); + + assert_se(listen(fd, SOMAXCONN) >= 0); + usleep(100 * USEC_PER_MSEC); + + assert_se(touch(path) >= 0); + usleep(100 * USEC_PER_MSEC); + + log_debug("Initialized server"); + + for (;;) { + _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; + _cleanup_(sd_event_unrefp) sd_event *event = NULL; + sd_id128_t id; + int bus_fd, code; + + assert_se(sd_id128_randomize(&id) >= 0); + + assert_se(sd_event_new(&event) >= 0); + + bus_fd = accept4(fd, NULL, NULL, SOCK_NONBLOCK|SOCK_CLOEXEC); + assert_se(bus_fd >= 0); + + log_debug("Accepted server connection"); + + assert_se(sd_bus_new(&bus) >= 0); + assert_se(sd_bus_set_description(bus, "server") >= 0); + assert_se(sd_bus_set_fd(bus, bus_fd, bus_fd) >= 0); + assert_se(sd_bus_set_server(bus, true, id) >= 0); + /* assert_se(sd_bus_set_anonymous(bus, true) >= 0); */ + + assert_se(sd_bus_attach_event(bus, event, 0) >= 0); + + assert_se(sd_bus_add_object_vtable(bus, NULL, "/foo", "foo.TestInterface", vtable, NULL) >= 0); + + assert_se(sd_bus_start(bus) >= 0); + + assert_se(sd_event_loop(event) >= 0); + + assert_se(sd_event_get_exit_code(event, &code) >= 0); + + if (code > 0) + break; + } + + log_debug("Server done"); + + return NULL; +} + +static void* thread_client1(void *p) { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; + const char *path = p, *t; + int r; + + log_debug("Initializing client1"); + + assert_se(sd_bus_new(&bus) >= 0); + assert_se(sd_bus_set_description(bus, "client1") >= 0); + + t = strjoina("unix:path=", path); + assert_se(sd_bus_set_address(bus, t) >= 0); + assert_se(sd_bus_set_watch_bind(bus, true) >= 0); + assert_se(sd_bus_start(bus) >= 0); + + r = sd_bus_call_method(bus, "foo.bar", "/foo", "foo.TestInterface", "Foobar", &error, NULL, NULL); + assert_se(r >= 0); + + log_debug("Client1 done"); + + return NULL; +} + +static int client2_callback(sd_bus_message *m, void *userdata, sd_bus_error *ret_error) { + assert_se(sd_bus_message_is_method_error(m, NULL) == 0); + assert_se(sd_event_exit(sd_bus_get_event(sd_bus_message_get_bus(m)), 0) >= 0); + return 0; +} + +static void* thread_client2(void *p) { + _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; + _cleanup_(sd_event_unrefp) sd_event *event = NULL; + const char *path = p, *t; + + log_debug("Initializing client2"); + + assert_se(sd_event_new(&event) >= 0); + assert_se(sd_bus_new(&bus) >= 0); + assert_se(sd_bus_set_description(bus, "client2") >= 0); + + t = strjoina("unix:path=", path); + assert_se(sd_bus_set_address(bus, t) >= 0); + assert_se(sd_bus_set_watch_bind(bus, true) >= 0); + assert_se(sd_bus_attach_event(bus, event, 0) >= 0); + assert_se(sd_bus_start(bus) >= 0); + + assert_se(sd_bus_call_method_async(bus, NULL, "foo.bar", "/foo", "foo.TestInterface", "Foobar", client2_callback, NULL, NULL) >= 0); + + assert_se(sd_event_loop(event) >= 0); + + log_debug("Client2 done"); + + return NULL; +} + +static void request_exit(const char *path) { + _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; + const char *t; + + assert_se(sd_bus_new(&bus) >= 0); + + t = strjoina("unix:path=", path); + assert_se(sd_bus_set_address(bus, t) >= 0); + assert_se(sd_bus_set_watch_bind(bus, true) >= 0); + assert_se(sd_bus_set_description(bus, "request-exit") >= 0); + assert_se(sd_bus_start(bus) >= 0); + + assert_se(sd_bus_call_method(bus, "foo.bar", "/foo", "foo.TestInterface", "Exit", NULL, NULL, NULL) >= 0); +} + +int main(int argc, char *argv[]) { + _cleanup_(rm_rf_physical_and_freep) char *d = NULL; + pthread_t server, client1, client2; + char *path; + + log_set_max_level(LOG_DEBUG); + + /* We use /dev/shm here rather than /tmp, since some weird distros might set up /tmp as some weird fs that + * doesn't support inotify properly. */ + assert_se(mkdtemp_malloc("/dev/shm/systemd-watch-bind-XXXXXX", &d) >= 0); + + path = strjoina(d, "/this/is/a/socket"); + + assert_se(pthread_create(&server, NULL, thread_server, path) == 0); + assert_se(pthread_create(&client1, NULL, thread_client1, path) == 0); + assert_se(pthread_create(&client2, NULL, thread_client2, path) == 0); + + assert_se(pthread_join(client1, NULL) == 0); + assert_se(pthread_join(client2, NULL) == 0); + + request_exit(path); + + assert_se(pthread_join(server, NULL) == 0); + + return 0; +} diff --git a/src/systemd/sd-bus.h b/src/systemd/sd-bus.h index c5c7096d55b..66bc48842bf 100644 --- a/src/systemd/sd-bus.h +++ b/src/systemd/sd-bus.h @@ -150,8 +150,10 @@ int sd_bus_set_allow_interactive_authorization(sd_bus *bus, int b); int sd_bus_get_allow_interactive_authorization(sd_bus *bus); int sd_bus_set_exit_on_disconnect(sd_bus *bus, int b); int sd_bus_get_exit_on_disconnect(sd_bus *bus); +int sd_bus_set_watch_bind(sd_bus *bus, int b); +int sd_bus_get_watch_bind(sd_bus *bus); -int sd_bus_start(sd_bus *ret); +int sd_bus_start(sd_bus *bus); int sd_bus_try_close(sd_bus *bus); void sd_bus_close(sd_bus *bus); diff --git a/src/test/meson.build b/src/test/meson.build index 71b440637d7..18e957ddc82 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -762,6 +762,10 @@ tests += [ [], [threads]], + [['src/libsystemd/sd-bus/test-bus-watch-bind.c'], + [], + [threads], '', 'timeout=120'], + [['src/libsystemd/sd-bus/test-bus-chat.c'], [], [threads]], From ac8029fc251d989cf1d4c25f7125a4295513c765 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Dec 2017 13:53:12 +0100 Subject: [PATCH 14/46] sd-bus: start reply callback timeouts only when the connection is established Currently, reply callback timeouts are started the instant the method calls are enqueued, which can be very early on. For example, the Hello() method call is enqueued right when sd_bus_start() is called, i.e. before the socket connection and everything is established. With this change we instead start the method timeout the moment we actually leave the authentication phase of the connection. This way, the timeout the kernel applies on socket connecting, and we apply on the authentication phase no longer runs in parallel to the Hello() method call, but all three run serially one after the other, which is definitely a cleaner approach. Moreover, this makes the "watch bind" feature a lot more useful, as it allows enqueuing method calls while we are still waiting for inotify events, without them timeouting until the connection is actually established, i.e. when the method call actually has a chance of being actually run. This is a change of behaviour of course, but I think the new behaviour is much better than the old one, since we don't race timeouts against each other anymore... --- src/libsystemd/sd-bus/bus-internal.h | 10 ++--- src/libsystemd/sd-bus/bus-slot.c | 2 +- src/libsystemd/sd-bus/sd-bus.c | 63 ++++++++++++++++++++-------- 3 files changed, 51 insertions(+), 24 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-internal.h b/src/libsystemd/sd-bus/bus-internal.h index 629d8b3f370..72e2ad94fde 100644 --- a/src/libsystemd/sd-bus/bus-internal.h +++ b/src/libsystemd/sd-bus/bus-internal.h @@ -38,7 +38,7 @@ struct reply_callback { sd_bus_message_handler_t callback; - usec_t timeout; + usec_t timeout_usec; /* this is a relative timeout until we reach the BUS_HELLO state, and an absolute one right after */ uint64_t cookie; unsigned prioq_idx; }; @@ -157,10 +157,10 @@ struct sd_bus_slot { enum bus_state { BUS_UNSET, - BUS_WATCH_BIND, /* waiting for the socket to appear via inotify */ - BUS_OPENING, - BUS_AUTHENTICATING, - BUS_HELLO, + BUS_WATCH_BIND, /* waiting for the socket to appear via inotify */ + BUS_OPENING, /* the kernel's connect() is still not ready */ + BUS_AUTHENTICATING, /* we are currently in the "SASL" authorization phase of dbus */ + BUS_HELLO, /* we are waiting for the Hello() response */ BUS_RUNNING, BUS_CLOSING, BUS_CLOSED diff --git a/src/libsystemd/sd-bus/bus-slot.c b/src/libsystemd/sd-bus/bus-slot.c index 756761c3ed2..f8de699b3d2 100644 --- a/src/libsystemd/sd-bus/bus-slot.c +++ b/src/libsystemd/sd-bus/bus-slot.c @@ -81,7 +81,7 @@ void bus_slot_disconnect(sd_bus_slot *slot) { if (slot->reply_callback.cookie != 0) ordered_hashmap_remove(slot->bus->reply_callbacks, &slot->reply_callback.cookie); - if (slot->reply_callback.timeout != 0) + if (slot->reply_callback.timeout_usec != 0) prioq_remove(slot->bus->reply_callbacks_prioq, &slot->reply_callback, &slot->reply_callback.prioq_idx); break; diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 3b65f51d04c..93b1f48c13a 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -461,7 +461,24 @@ static int bus_send_hello(sd_bus *bus) { } int bus_start_running(sd_bus *bus) { + struct reply_callback *c; + Iterator i; + usec_t n; + assert(bus); + assert(bus->state < BUS_HELLO); + + /* We start all method call timeouts when we enter BUS_HELLO or BUS_RUNNING mode. At this point let's convert + * all relative to absolute timestamps. Note that we do not reshuffle the reply callback priority queue since + * adding a fixed value to all entries should not alter the internal order. */ + + n = now(CLOCK_MONOTONIC); + ORDERED_HASHMAP_FOREACH(c, bus->reply_callbacks, i) { + if (c->timeout_usec == 0) + continue; + + c->timeout_usec = usec_add(n, c->timeout_usec); + } if (bus->bus_client) { bus->state = BUS_HELLO; @@ -1721,26 +1738,35 @@ _public_ int sd_bus_send_to(sd_bus *bus, sd_bus_message *m, const char *destinat return sd_bus_send(bus, m, cookie); } -static usec_t calc_elapse(uint64_t usec) { +static usec_t calc_elapse(sd_bus *bus, uint64_t usec) { + assert(bus); + if (usec == (uint64_t) -1) return 0; - return now(CLOCK_MONOTONIC) + usec; + /* We start all timeouts the instant we enter BUS_HELLO/BUS_RUNNING state, so that the don't run in parallel + * with any connection setup states. Hence, if a method callback is started earlier than that we just store the + * relative timestamp, and afterwards the absolute one. */ + + if (IN_SET(bus->state, BUS_WATCH_BIND, BUS_OPENING, BUS_AUTHENTICATING)) + return usec; + else + return now(CLOCK_MONOTONIC) + usec; } static int timeout_compare(const void *a, const void *b) { const struct reply_callback *x = a, *y = b; - if (x->timeout != 0 && y->timeout == 0) + if (x->timeout_usec != 0 && y->timeout_usec == 0) return -1; - if (x->timeout == 0 && y->timeout != 0) + if (x->timeout_usec == 0 && y->timeout_usec != 0) return 1; - if (x->timeout < y->timeout) + if (x->timeout_usec < y->timeout_usec) return -1; - if (x->timeout > y->timeout) + if (x->timeout_usec > y->timeout_usec) return 1; return 0; @@ -1800,11 +1826,11 @@ _public_ int sd_bus_call_async( return r; } - s->reply_callback.timeout = calc_elapse(m->timeout); - if (s->reply_callback.timeout != 0) { + s->reply_callback.timeout_usec = calc_elapse(bus, m->timeout); + if (s->reply_callback.timeout_usec != 0) { r = prioq_put(bus->reply_callbacks_prioq, &s->reply_callback, &s->reply_callback.prioq_idx); if (r < 0) { - s->reply_callback.timeout = 0; + s->reply_callback.timeout_usec = 0; return r; } } @@ -1891,7 +1917,7 @@ _public_ int sd_bus_call( if (r < 0) goto fail; - timeout = calc_elapse(m->timeout); + timeout = calc_elapse(bus, m->timeout); for (;;) { usec_t left; @@ -2099,12 +2125,12 @@ _public_ int sd_bus_get_timeout(sd_bus *bus, uint64_t *timeout_usec) { return 0; } - if (c->timeout == 0) { + if (c->timeout_usec == 0) { *timeout_usec = (uint64_t) -1; return 0; } - *timeout_usec = c->timeout; + *timeout_usec = c->timeout_usec; return 1; case BUS_CLOSING: @@ -2131,13 +2157,14 @@ static int process_timeout(sd_bus *bus) { int r; assert(bus); + assert(IN_SET(bus->state, BUS_RUNNING, BUS_HELLO)); c = prioq_peek(bus->reply_callbacks_prioq); if (!c) return 0; n = now(CLOCK_MONOTONIC); - if (c->timeout > n) + if (c->timeout_usec > n) return 0; r = bus_message_new_synthetic_error( @@ -2153,7 +2180,7 @@ static int process_timeout(sd_bus *bus) { return r; assert_se(prioq_pop(bus->reply_callbacks_prioq) == c); - c->timeout = 0; + c->timeout_usec = 0; ordered_hashmap_remove(bus->reply_callbacks, &c->cookie); c->cookie = 0; @@ -2264,9 +2291,9 @@ static int process_reply(sd_bus *bus, sd_bus_message *m) { return r; } - if (c->timeout != 0) { + if (c->timeout_usec != 0) { prioq_remove(bus->reply_callbacks_prioq, c, &c->prioq_idx); - c->timeout = 0; + c->timeout_usec = 0; } is_hello = bus->state == BUS_HELLO && c->callback == hello_callback; @@ -2602,9 +2629,9 @@ static int process_closing_reply_callback(sd_bus *bus, struct reply_callback *c) if (r < 0) return r; - if (c->timeout != 0) { + if (c->timeout_usec != 0) { prioq_remove(bus->reply_callbacks_prioq, c, &c->prioq_idx); - c->timeout = 0; + c->timeout_usec = 0; } ordered_hashmap_remove(bus->reply_callbacks, &c->cookie); From 56d820b6a43463e44f6304039e510bafab847ec1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Dec 2017 22:19:34 +0100 Subject: [PATCH 15/46] busctl: add a new --watch-bind switch This is useful for testing, and early-boot scripting. --- man/busctl.xml | 10 ++++++++++ src/busctl/busctl.c | 23 ++++++++++++++++++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/man/busctl.xml b/man/busctl.xml index 0c0d28b5d32..2320cb8ed30 100644 --- a/man/busctl.xml +++ b/man/busctl.xml @@ -241,6 +241,16 @@ + + BOOL + + + Controls whether to wait for the specified AF_UNIX bus socket to appear in the + file system before connecting to it. Defaults to off. When enabled, the tool will watch the file system until + the socket is created and then connect to it. + + + diff --git a/src/busctl/busctl.c b/src/busctl/busctl.c index 7a8d6ba5ac7..5623bb2ffab 100644 --- a/src/busctl/busctl.c +++ b/src/busctl/busctl.c @@ -62,6 +62,7 @@ static bool arg_expect_reply = true; static bool arg_auto_start = true; static bool arg_allow_interactive_authorization = true; static bool arg_augment_creds = true; +static bool arg_watch_bind = false; static usec_t arg_timeout = 0; #define NAME_IS_ACQUIRED INT_TO_PTR(1) @@ -1735,7 +1736,9 @@ static int help(void) { " --allow-interactive-authorization=BOOL\n" " Allow interactive authorization for operation\n" " --timeout=SECS Maximum time to wait for method call completion\n" - " --augment-creds=BOOL Extend credential data with data read from /proc/$PID\n\n" + " --augment-creds=BOOL Extend credential data with data read from /proc/$PID\n" + " --watch-bind=BOOL Wait for bus AF_UNIX socket to be bound in the file\n" + " system\n\n" "Commands:\n" " list List bus names\n" " status [SERVICE] Show bus service, process or bus owner credentials\n" @@ -1777,6 +1780,7 @@ static int parse_argv(int argc, char *argv[]) { ARG_ALLOW_INTERACTIVE_AUTHORIZATION, ARG_TIMEOUT, ARG_AUGMENT_CREDS, + ARG_WATCH_BIND, }; static const struct option options[] = { @@ -1803,6 +1807,7 @@ static int parse_argv(int argc, char *argv[]) { { "allow-interactive-authorization", required_argument, NULL, ARG_ALLOW_INTERACTIVE_AUTHORIZATION }, { "timeout", required_argument, NULL, ARG_TIMEOUT }, { "augment-creds",required_argument, NULL, ARG_AUGMENT_CREDS}, + { "watch-bind", required_argument, NULL, ARG_WATCH_BIND }, {}, }; @@ -1953,6 +1958,16 @@ static int parse_argv(int argc, char *argv[]) { arg_augment_creds = !!r; break; + case ARG_WATCH_BIND: + r = parse_boolean(optarg); + if (r < 0) { + log_error("Failed to parse --watch-bind= parameter."); + return r; + } + + arg_watch_bind = !!r; + break; + case '?': return -EINVAL; @@ -2051,6 +2066,12 @@ int main(int argc, char *argv[]) { goto finish; } + r = sd_bus_set_watch_bind(bus, arg_watch_bind); + if (r < 0) { + log_error_errno(r, "Failed to set watch-bind setting to '%s': %m", yes_no(arg_watch_bind)); + goto finish; + } + if (arg_address) r = sd_bus_set_address(bus, arg_address); else { From c7db1984d0ec106436fda4e51194e259b90516e7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Dec 2017 14:21:34 +0100 Subject: [PATCH 16/46] sd-bus: get rid of kdbus flags cruft We only need three bits from the old kdbus flags cruft, hence let's make them proper booleans. --- src/libsystemd/sd-bus/bus-control.c | 2 +- src/libsystemd/sd-bus/bus-internal.h | 68 ++-------------------------- src/libsystemd/sd-bus/bus-kernel.c | 46 ------------------- src/libsystemd/sd-bus/bus-kernel.h | 2 - src/libsystemd/sd-bus/bus-objects.c | 2 +- src/libsystemd/sd-bus/bus-socket.c | 10 ++-- src/libsystemd/sd-bus/sd-bus.c | 47 ++++++------------- 7 files changed, 25 insertions(+), 152 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-control.c b/src/libsystemd/sd-bus/bus-control.c index 0b39115d16a..8352bb0cb05 100644 --- a/src/libsystemd/sd-bus/bus-control.c +++ b/src/libsystemd/sd-bus/bus-control.c @@ -631,7 +631,7 @@ _public_ int sd_bus_get_owner_creds(sd_bus *bus, uint64_t mask, sd_bus_creds **r } #define internal_match(bus, m) \ - ((bus)->hello_flags & KDBUS_HELLO_MONITOR \ + ((bus)->is_monitor \ ? (isempty(m) ? "eavesdrop='true'" : strjoina((m), ",eavesdrop='true'")) \ : (m)) diff --git a/src/libsystemd/sd-bus/bus-internal.h b/src/libsystemd/sd-bus/bus-internal.h index 72e2ad94fde..79012b0ad0e 100644 --- a/src/libsystemd/sd-bus/bus-internal.h +++ b/src/libsystemd/sd-bus/bus-internal.h @@ -213,6 +213,9 @@ struct sd_bus { bool exit_triggered:1; bool is_local:1; bool watch_bind:1; + bool is_monitor:1; + bool accept_fd:1; + bool attach_timestamp:1; int use_memfd; @@ -289,9 +292,6 @@ struct sd_bus { pid_t original_pid; - uint64_t hello_flags; - uint64_t attach_flags; - sd_event_source *input_io_event_source; sd_event_source *output_io_event_source; sd_event_source *time_event_source; @@ -407,65 +407,3 @@ int bus_maybe_reply_error(sd_bus_message *m, int r, sd_bus_error *error); if (!assert_log(expr, #expr)) \ return sd_bus_error_set_errno(error, r); \ } while (false) - -/** - * enum kdbus_attach_flags - flags for metadata attachments - * @KDBUS_ATTACH_TIMESTAMP: Timestamp - * @KDBUS_ATTACH_CREDS: Credentials - * @KDBUS_ATTACH_PIDS: PIDs - * @KDBUS_ATTACH_AUXGROUPS: Auxiliary groups - * @KDBUS_ATTACH_NAMES: Well-known names - * @KDBUS_ATTACH_TID_COMM: The "comm" process identifier of the TID - * @KDBUS_ATTACH_PID_COMM: The "comm" process identifier of the PID - * @KDBUS_ATTACH_EXE: The path of the executable - * @KDBUS_ATTACH_CMDLINE: The process command line - * @KDBUS_ATTACH_CGROUP: The croup membership - * @KDBUS_ATTACH_CAPS: The process capabilities - * @KDBUS_ATTACH_SECLABEL: The security label - * @KDBUS_ATTACH_AUDIT: The audit IDs - * @KDBUS_ATTACH_CONN_DESCRIPTION: The human-readable connection name - * @_KDBUS_ATTACH_ALL: All of the above - * @_KDBUS_ATTACH_ANY: Wildcard match to enable any kind of - * metatdata. - */ -enum kdbus_attach_flags { - KDBUS_ATTACH_TIMESTAMP = 1ULL << 0, - KDBUS_ATTACH_CREDS = 1ULL << 1, - KDBUS_ATTACH_PIDS = 1ULL << 2, - KDBUS_ATTACH_AUXGROUPS = 1ULL << 3, - KDBUS_ATTACH_NAMES = 1ULL << 4, - KDBUS_ATTACH_TID_COMM = 1ULL << 5, - KDBUS_ATTACH_PID_COMM = 1ULL << 6, - KDBUS_ATTACH_EXE = 1ULL << 7, - KDBUS_ATTACH_CMDLINE = 1ULL << 8, - KDBUS_ATTACH_CGROUP = 1ULL << 9, - KDBUS_ATTACH_CAPS = 1ULL << 10, - KDBUS_ATTACH_SECLABEL = 1ULL << 11, - KDBUS_ATTACH_AUDIT = 1ULL << 12, - KDBUS_ATTACH_CONN_DESCRIPTION = 1ULL << 13, - _KDBUS_ATTACH_ALL = (1ULL << 14) - 1, - _KDBUS_ATTACH_ANY = ~0ULL -}; - -/** - * enum kdbus_hello_flags - flags for struct kdbus_cmd_hello - * @KDBUS_HELLO_ACCEPT_FD: The connection allows the reception of - * any passed file descriptors - * @KDBUS_HELLO_ACTIVATOR: Special-purpose connection which registers - * a well-know name for a process to be started - * when traffic arrives - * @KDBUS_HELLO_POLICY_HOLDER: Special-purpose connection which registers - * policy entries for a name. The provided name - * is not activated and not registered with the - * name database, it only allows unprivileged - * connections to acquire a name, talk or discover - * a service - * @KDBUS_HELLO_MONITOR: Special-purpose connection to monitor - * bus traffic - */ -enum kdbus_hello_flags { - KDBUS_HELLO_ACCEPT_FD = 1ULL << 0, - KDBUS_HELLO_ACTIVATOR = 1ULL << 1, - KDBUS_HELLO_POLICY_HOLDER = 1ULL << 2, - KDBUS_HELLO_MONITOR = 1ULL << 3, -}; diff --git a/src/libsystemd/sd-bus/bus-kernel.c b/src/libsystemd/sd-bus/bus-kernel.c index c6179b4d954..b27b9d7d86d 100644 --- a/src/libsystemd/sd-bus/bus-kernel.c +++ b/src/libsystemd/sd-bus/bus-kernel.c @@ -66,49 +66,3 @@ void bus_flush_memfd(sd_bus *b) { for (i = 0; i < b->n_memfd_cache; i++) close_and_munmap(b->memfd_cache[i].fd, b->memfd_cache[i].address, b->memfd_cache[i].mapped); } - -uint64_t attach_flags_to_kdbus(uint64_t mask) { - uint64_t m = 0; - - if (mask & (SD_BUS_CREDS_UID|SD_BUS_CREDS_EUID|SD_BUS_CREDS_SUID|SD_BUS_CREDS_FSUID| - SD_BUS_CREDS_GID|SD_BUS_CREDS_EGID|SD_BUS_CREDS_SGID|SD_BUS_CREDS_FSGID)) - m |= KDBUS_ATTACH_CREDS; - - if (mask & (SD_BUS_CREDS_PID|SD_BUS_CREDS_TID|SD_BUS_CREDS_PPID)) - m |= KDBUS_ATTACH_PIDS; - - if (mask & SD_BUS_CREDS_COMM) - m |= KDBUS_ATTACH_PID_COMM; - - if (mask & SD_BUS_CREDS_TID_COMM) - m |= KDBUS_ATTACH_TID_COMM; - - if (mask & SD_BUS_CREDS_EXE) - m |= KDBUS_ATTACH_EXE; - - if (mask & SD_BUS_CREDS_CMDLINE) - m |= KDBUS_ATTACH_CMDLINE; - - if (mask & (SD_BUS_CREDS_CGROUP|SD_BUS_CREDS_UNIT|SD_BUS_CREDS_USER_UNIT|SD_BUS_CREDS_SLICE|SD_BUS_CREDS_SESSION|SD_BUS_CREDS_OWNER_UID)) - m |= KDBUS_ATTACH_CGROUP; - - if (mask & (SD_BUS_CREDS_EFFECTIVE_CAPS|SD_BUS_CREDS_PERMITTED_CAPS|SD_BUS_CREDS_INHERITABLE_CAPS|SD_BUS_CREDS_BOUNDING_CAPS)) - m |= KDBUS_ATTACH_CAPS; - - if (mask & SD_BUS_CREDS_SELINUX_CONTEXT) - m |= KDBUS_ATTACH_SECLABEL; - - if (mask & (SD_BUS_CREDS_AUDIT_SESSION_ID|SD_BUS_CREDS_AUDIT_LOGIN_UID)) - m |= KDBUS_ATTACH_AUDIT; - - if (mask & SD_BUS_CREDS_WELL_KNOWN_NAMES) - m |= KDBUS_ATTACH_NAMES; - - if (mask & SD_BUS_CREDS_DESCRIPTION) - m |= KDBUS_ATTACH_CONN_DESCRIPTION; - - if (mask & SD_BUS_CREDS_SUPPLEMENTARY_GIDS) - m |= KDBUS_ATTACH_AUXGROUPS; - - return m; -} diff --git a/src/libsystemd/sd-bus/bus-kernel.h b/src/libsystemd/sd-bus/bus-kernel.h index d9f80935fe4..fa78e5c80d1 100644 --- a/src/libsystemd/sd-bus/bus-kernel.h +++ b/src/libsystemd/sd-bus/bus-kernel.h @@ -41,5 +41,3 @@ struct memfd_cache { void close_and_munmap(int fd, void *address, size_t size); void bus_flush_memfd(sd_bus *bus); - -uint64_t attach_flags_to_kdbus(uint64_t sd_bus_flags); diff --git a/src/libsystemd/sd-bus/bus-objects.c b/src/libsystemd/sd-bus/bus-objects.c index 121197bbcb9..1237819b498 100644 --- a/src/libsystemd/sd-bus/bus-objects.c +++ b/src/libsystemd/sd-bus/bus-objects.c @@ -1369,7 +1369,7 @@ int bus_process_object(sd_bus *bus, sd_bus_message *m) { assert(bus); assert(m); - if (bus->hello_flags & KDBUS_HELLO_MONITOR) + if (bus->is_monitor) return 0; if (m->header->type != SD_BUS_MESSAGE_METHOD_CALL) diff --git a/src/libsystemd/sd-bus/bus-socket.c b/src/libsystemd/sd-bus/bus-socket.c index 9291fed0e75..e2991bc8b29 100644 --- a/src/libsystemd/sd-bus/bus-socket.c +++ b/src/libsystemd/sd-bus/bus-socket.c @@ -191,7 +191,7 @@ static int bus_socket_auth_verify_client(sd_bus *b) { if (!e) return 0; - if (b->hello_flags & KDBUS_HELLO_ACCEPT_FD) { + if (b->accept_fd) { f = memmem(e + 2, b->rbuffer_size - (e - (char*) b->rbuffer) - 2, "\r\n", 2); if (!f) return 0; @@ -478,7 +478,7 @@ static int bus_socket_auth_verify_server(sd_bus *b) { r = bus_socket_auth_write_ok(b); } } else if (line_equals(line, l, "NEGOTIATE_UNIX_FD")) { - if (b->auth == _BUS_AUTH_INVALID || !(b->hello_flags & KDBUS_HELLO_ACCEPT_FD)) + if (b->auth == _BUS_AUTH_INVALID || !b->accept_fd) r = bus_socket_auth_write(b, "ERROR\r\n"); else { b->can_fds = true; @@ -655,7 +655,7 @@ static int bus_socket_start_auth_client(sd_bus *b) { if (!b->auth_buffer) return -ENOMEM; - if (b->hello_flags & KDBUS_HELLO_ACCEPT_FD) + if (b->accept_fd) auth_suffix = "\r\nNEGOTIATE_UNIX_FD\r\nBEGIN\r\n"; else auth_suffix = "\r\nBEGIN\r\n"; @@ -679,11 +679,11 @@ int bus_socket_start_auth(sd_bus *b) { b->auth_timeout = now(CLOCK_MONOTONIC) + BUS_AUTH_TIMEOUT; if (sd_is_socket(b->input_fd, AF_UNIX, 0, 0) <= 0) - b->hello_flags &= ~KDBUS_HELLO_ACCEPT_FD; + b->accept_fd = false; if (b->output_fd != b->input_fd) if (sd_is_socket(b->output_fd, AF_UNIX, 0, 0) <= 0) - b->hello_flags &= ~KDBUS_HELLO_ACCEPT_FD; + b->accept_fd = false; if (b->is_server) return bus_socket_read_auth(b); diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 93b1f48c13a..2c20ad6ebf8 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -196,8 +196,7 @@ _public_ int sd_bus_new(sd_bus **ret) { r->inotify_fd = -1; r->message_version = 1; r->creds_mask |= SD_BUS_CREDS_WELL_KNOWN_NAMES|SD_BUS_CREDS_UNIQUE_NAME; - r->hello_flags |= KDBUS_HELLO_ACCEPT_FD; - r->attach_flags |= KDBUS_ATTACH_NAMES; + r->accept_fd = true; r->original_pid = getpid_cached(); r->n_groups = (size_t) -1; @@ -286,7 +285,7 @@ _public_ int sd_bus_set_monitor(sd_bus *bus, int b) { assert_return(bus->state == BUS_UNSET, -EPERM); assert_return(!bus_pid_changed(bus), -ECHILD); - SET_FLAG(bus->hello_flags, KDBUS_HELLO_MONITOR, b); + bus->is_monitor = b; return 0; } @@ -295,30 +294,23 @@ _public_ int sd_bus_negotiate_fds(sd_bus *bus, int b) { assert_return(bus->state == BUS_UNSET, -EPERM); assert_return(!bus_pid_changed(bus), -ECHILD); - SET_FLAG(bus->hello_flags, KDBUS_HELLO_ACCEPT_FD, b); + bus->accept_fd = b; return 0; } _public_ int sd_bus_negotiate_timestamp(sd_bus *bus, int b) { - uint64_t new_flags; assert_return(bus, -EINVAL); assert_return(!IN_SET(bus->state, BUS_CLOSING, BUS_CLOSED), -EPERM); assert_return(!bus_pid_changed(bus), -ECHILD); - new_flags = bus->attach_flags; - SET_FLAG(new_flags, KDBUS_ATTACH_TIMESTAMP, b); - - if (bus->attach_flags == new_flags) - return 0; - - bus->attach_flags = new_flags; + /* This is not actually supported by any of our transports these days, but we do honour it for synthetic + * replies, and maybe one day classic D-Bus learns this too */ + bus->attach_timestamp = b; return 0; } _public_ int sd_bus_negotiate_creds(sd_bus *bus, int b, uint64_t mask) { - uint64_t new_flags; - assert_return(bus, -EINVAL); assert_return(mask <= _SD_BUS_CREDS_ALL, -EINVAL); assert_return(!IN_SET(bus->state, BUS_CLOSING, BUS_CLOSED), -EPERM); @@ -329,13 +321,6 @@ _public_ int sd_bus_negotiate_creds(sd_bus *bus, int b, uint64_t mask) { /* The well knowns we need unconditionally, so that matches can work */ bus->creds_mask |= SD_BUS_CREDS_WELL_KNOWN_NAMES|SD_BUS_CREDS_UNIQUE_NAME; - /* Make sure we don't lose the timestamp flag */ - new_flags = (bus->attach_flags & KDBUS_ATTACH_TIMESTAMP) | attach_flags_to_kdbus(bus->creds_mask); - if (bus->attach_flags == new_flags) - return 0; - - bus->attach_flags = new_flags; - return 0; } @@ -1094,7 +1079,6 @@ _public_ int sd_bus_open(sd_bus **ret) { * be safe, and authenticate everything */ b->trusted = false; b->is_local = false; - b->attach_flags |= KDBUS_ATTACH_CAPS | KDBUS_ATTACH_CREDS; b->creds_mask |= SD_BUS_CREDS_UID | SD_BUS_CREDS_EUID | SD_BUS_CREDS_EFFECTIVE_CAPS; r = sd_bus_start(b); @@ -1140,7 +1124,6 @@ _public_ int sd_bus_open_system(sd_bus **ret) { /* Let's do per-method access control on the system bus. We * need the caller's UID and capability set for that. */ b->trusted = false; - b->attach_flags |= KDBUS_ATTACH_CAPS | KDBUS_ATTACH_CREDS; b->creds_mask |= SD_BUS_CREDS_UID | SD_BUS_CREDS_EUID | SD_BUS_CREDS_EFFECTIVE_CAPS; b->is_local = true; @@ -1419,11 +1402,11 @@ _public_ int sd_bus_can_send(sd_bus *bus, char type) { assert_return(bus->state != BUS_UNSET, -ENOTCONN); assert_return(!bus_pid_changed(bus), -ECHILD); - if (bus->hello_flags & KDBUS_HELLO_MONITOR) + if (bus->is_monitor) return 0; if (type == SD_BUS_TYPE_UNIX_FD) { - if (!(bus->hello_flags & KDBUS_HELLO_ACCEPT_FD)) + if (!bus->accept_fd) return 0; r = bus_ensure_running(bus); @@ -1491,7 +1474,7 @@ int bus_seal_synthetic_message(sd_bus *b, sd_bus_message *m) { /* Fake some timestamps, if they were requested, and not * already initialized */ - if (b->attach_flags & KDBUS_ATTACH_TIMESTAMP) { + if (b->attach_timestamp) { if (m->realtime <= 0) m->realtime = now(CLOCK_REALTIME); @@ -1936,7 +1919,7 @@ _public_ int sd_bus_call( if (incoming->header->type == SD_BUS_MESSAGE_METHOD_RETURN) { - if (incoming->n_fds <= 0 || (bus->hello_flags & KDBUS_HELLO_ACCEPT_FD)) { + if (incoming->n_fds <= 0 || bus->accept_fd) { if (reply) *reply = incoming; else @@ -2262,7 +2245,7 @@ static int process_reply(sd_bus *bus, sd_bus_message *m) { slot = container_of(c, sd_bus_slot, reply_callback); - if (m->n_fds > 0 && !(bus->hello_flags & KDBUS_HELLO_ACCEPT_FD)) { + if (m->n_fds > 0 && !bus->accept_fd) { /* If the reply contained a file descriptor which we * didn't want we pass an error instead. */ @@ -2394,7 +2377,7 @@ static int process_builtin(sd_bus *bus, sd_bus_message *m) { assert(bus); assert(m); - if (bus->hello_flags & KDBUS_HELLO_MONITOR) + if (bus->is_monitor) return 0; if (bus->manual_peer_interface) @@ -2452,13 +2435,13 @@ static int process_fd_check(sd_bus *bus, sd_bus_message *m) { * delivered to us later even though we ourselves did not * negotiate it. */ - if (bus->hello_flags & KDBUS_HELLO_MONITOR) + if (bus->is_monitor) return 0; if (m->n_fds <= 0) return 0; - if (bus->hello_flags & KDBUS_HELLO_ACCEPT_FD) + if (bus->accept_fd) return 0; if (m->header->type != SD_BUS_MESSAGE_METHOD_CALL) @@ -3769,7 +3752,7 @@ _public_ int sd_bus_is_monitor(sd_bus *bus) { assert_return(bus, -EINVAL); assert_return(!bus_pid_changed(bus), -ECHILD); - return !!(bus->hello_flags & KDBUS_HELLO_MONITOR); + return bus->is_monitor; } static void flush_close(sd_bus *bus) { From e8bd7b092f3f442ea238008b6508f75357e092f4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Dec 2017 14:48:16 +0100 Subject: [PATCH 17/46] bus-control: remove kdbus indirection cruft When kdbus was still around we always had two implementations of the various control calls: one for dbus1 and one for kdbus. Let'sget rid of this, simplify things, and just merge the wrappers that used to multiplex this with the implementations. No change in behaviour, just some merging of functions --- src/libsystemd/sd-bus/bus-control.c | 204 +++++++++++----------------- 1 file changed, 76 insertions(+), 128 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-control.c b/src/libsystemd/sd-bus/bus-control.c index 8352bb0cb05..e1e75da6797 100644 --- a/src/libsystemd/sd-bus/bus-control.c +++ b/src/libsystemd/sd-bus/bus-control.c @@ -57,13 +57,27 @@ _public_ int sd_bus_get_unique_name(sd_bus *bus, const char **unique) { return 0; } -static int bus_request_name_dbus1(sd_bus *bus, const char *name, uint64_t flags) { +_public_ int sd_bus_request_name(sd_bus *bus, const char *name, uint64_t flags) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; uint32_t ret, param = 0; int r; - assert(bus); - assert(name); + assert_return(bus, -EINVAL); + assert_return(name, -EINVAL); + assert_return(!bus_pid_changed(bus), -ECHILD); + assert_return(!(flags & ~(SD_BUS_NAME_ALLOW_REPLACEMENT|SD_BUS_NAME_REPLACE_EXISTING|SD_BUS_NAME_QUEUE)), -EINVAL); + assert_return(service_name_is_valid(name), -EINVAL); + assert_return(name[0] != ':', -EINVAL); + + if (!bus->bus_client) + return -EINVAL; + + /* Don't allow requesting the special driver and local names */ + if (STR_IN_SET(name, "org.freedesktop.DBus", "org.freedesktop.DBus.Local")) + return -EINVAL; + + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; if (flags & SD_BUS_NAME_ALLOW_REPLACEMENT) param |= BUS_NAME_ALLOW_REPLACEMENT; @@ -102,35 +116,27 @@ static int bus_request_name_dbus1(sd_bus *bus, const char *name, uint64_t flags) return -EIO; } -_public_ int sd_bus_request_name(sd_bus *bus, const char *name, uint64_t flags) { +_public_ int sd_bus_release_name(sd_bus *bus, const char *name) { + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + uint32_t ret; + int r; + assert_return(bus, -EINVAL); assert_return(name, -EINVAL); assert_return(!bus_pid_changed(bus), -ECHILD); - assert_return(!(flags & ~(SD_BUS_NAME_ALLOW_REPLACEMENT|SD_BUS_NAME_REPLACE_EXISTING|SD_BUS_NAME_QUEUE)), -EINVAL); assert_return(service_name_is_valid(name), -EINVAL); assert_return(name[0] != ':', -EINVAL); if (!bus->bus_client) return -EINVAL; - /* Don't allow requesting the special driver and local names */ + /* Don't allow releasing the special driver and local names */ if (STR_IN_SET(name, "org.freedesktop.DBus", "org.freedesktop.DBus.Local")) return -EINVAL; if (!BUS_IS_OPEN(bus->state)) return -ENOTCONN; - return bus_request_name_dbus1(bus, name, flags); -} - -static int bus_release_name_dbus1(sd_bus *bus, const char *name) { - _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - uint32_t ret; - int r; - - assert(bus); - assert(name); - r = sd_bus_call_method( bus, "org.freedesktop.DBus", @@ -157,31 +163,21 @@ static int bus_release_name_dbus1(sd_bus *bus, const char *name) { return -EINVAL; } -_public_ int sd_bus_release_name(sd_bus *bus, const char *name) { +_public_ int sd_bus_list_names(sd_bus *bus, char ***acquired, char ***activatable) { + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + _cleanup_strv_free_ char **x = NULL, **y = NULL; + int r; + assert_return(bus, -EINVAL); - assert_return(name, -EINVAL); + assert_return(acquired || activatable, -EINVAL); assert_return(!bus_pid_changed(bus), -ECHILD); - assert_return(service_name_is_valid(name), -EINVAL); - assert_return(name[0] != ':', -EINVAL); if (!bus->bus_client) return -EINVAL; - /* Don't allow releasing the special driver and local names */ - if (STR_IN_SET(name, "org.freedesktop.DBus", "org.freedesktop.DBus.Local")) - return -EINVAL; - if (!BUS_IS_OPEN(bus->state)) return -ENOTCONN; - return bus_release_name_dbus1(bus, name); -} - -static int bus_list_names_dbus1(sd_bus *bus, char ***acquired, char ***activatable) { - _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - _cleanup_strv_free_ char **x = NULL, **y = NULL; - int r; - if (acquired) { r = sd_bus_call_method( bus, @@ -231,21 +227,7 @@ static int bus_list_names_dbus1(sd_bus *bus, char ***acquired, char ***activatab return 0; } -_public_ int sd_bus_list_names(sd_bus *bus, char ***acquired, char ***activatable) { - assert_return(bus, -EINVAL); - assert_return(acquired || activatable, -EINVAL); - assert_return(!bus_pid_changed(bus), -ECHILD); - - if (!bus->bus_client) - return -EINVAL; - - if (!BUS_IS_OPEN(bus->state)) - return -ENOTCONN; - - return bus_list_names_dbus1(bus, acquired, activatable); -} - -static int bus_get_name_creds_dbus1( +_public_ int sd_bus_get_name_creds( sd_bus *bus, const char *name, uint64_t mask, @@ -257,6 +239,30 @@ static int bus_get_name_creds_dbus1( pid_t pid = 0; int r; + assert_return(bus, -EINVAL); + assert_return(name, -EINVAL); + assert_return((mask & ~SD_BUS_CREDS_AUGMENT) <= _SD_BUS_CREDS_ALL, -EOPNOTSUPP); + assert_return(mask == 0 || creds, -EINVAL); + assert_return(!bus_pid_changed(bus), -ECHILD); + assert_return(service_name_is_valid(name), -EINVAL); + + if (!bus->bus_client) + return -EINVAL; + + /* Turn off augmenting if this isn't a local connection. If the connection is not local, then /proc is not + * going to match. */ + if (!bus->is_local) + mask &= ~SD_BUS_CREDS_AUGMENT; + + if (streq(name, "org.freedesktop.DBus.Local")) + return -EINVAL; + + if (streq(name, "org.freedesktop.DBus")) + return sd_bus_get_owner_creds(bus, mask, creds); + + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; + /* Only query the owner if the caller wants to know it or if * the caller just wants to check whether a name exists */ if ((mask & SD_BUS_CREDS_UNIQUE_NAME) || mask == 0) { @@ -519,46 +525,22 @@ static int bus_get_name_creds_dbus1( return 0; } -_public_ int sd_bus_get_name_creds( - sd_bus *bus, - const char *name, - uint64_t mask, - sd_bus_creds **creds) { +_public_ int sd_bus_get_owner_creds(sd_bus *bus, uint64_t mask, sd_bus_creds **ret) { + _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *c = NULL; + bool do_label, do_groups; + pid_t pid = 0; + int r; assert_return(bus, -EINVAL); - assert_return(name, -EINVAL); assert_return((mask & ~SD_BUS_CREDS_AUGMENT) <= _SD_BUS_CREDS_ALL, -EOPNOTSUPP); - assert_return(mask == 0 || creds, -EINVAL); + assert_return(ret, -EINVAL); assert_return(!bus_pid_changed(bus), -ECHILD); - assert_return(service_name_is_valid(name), -EINVAL); - - if (!bus->bus_client) - return -EINVAL; - - /* Turn off augmenting if this isn't a local connection. If the connection is not local, then /proc is not - * going to match. */ - if (!bus->is_local) - mask &= ~SD_BUS_CREDS_AUGMENT; - - if (streq(name, "org.freedesktop.DBus.Local")) - return -EINVAL; - - if (streq(name, "org.freedesktop.DBus")) - return sd_bus_get_owner_creds(bus, mask, creds); if (!BUS_IS_OPEN(bus->state)) return -ENOTCONN; - return bus_get_name_creds_dbus1(bus, name, mask, creds); -} - -static int bus_get_owner_creds_dbus1(sd_bus *bus, uint64_t mask, sd_bus_creds **ret) { - _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *c = NULL; - pid_t pid = 0; - bool do_label, do_groups; - int r; - - assert(bus); + if (!bus->is_local) + mask &= ~SD_BUS_CREDS_AUGMENT; do_label = bus->label && (mask & SD_BUS_CREDS_SELINUX_CONTEXT); do_groups = bus->n_groups != (size_t) -1 && (mask & SD_BUS_CREDS_SUPPLEMENTARY_GIDS); @@ -615,36 +597,25 @@ static int bus_get_owner_creds_dbus1(sd_bus *bus, uint64_t mask, sd_bus_creds ** return 0; } -_public_ int sd_bus_get_owner_creds(sd_bus *bus, uint64_t mask, sd_bus_creds **ret) { - assert_return(bus, -EINVAL); - assert_return((mask & ~SD_BUS_CREDS_AUGMENT) <= _SD_BUS_CREDS_ALL, -EOPNOTSUPP); - assert_return(ret, -EINVAL); - assert_return(!bus_pid_changed(bus), -ECHILD); - - if (!BUS_IS_OPEN(bus->state)) - return -ENOTCONN; - - if (!bus->is_local) - mask &= ~SD_BUS_CREDS_AUGMENT; - - return bus_get_owner_creds_dbus1(bus, mask, ret); -} - -#define internal_match(bus, m) \ +#define append_eavesdrop(bus, m) \ ((bus)->is_monitor \ ? (isempty(m) ? "eavesdrop='true'" : strjoina((m), ",eavesdrop='true'")) \ : (m)) -static int bus_add_match_internal_dbus1( +int bus_add_match_internal( sd_bus *bus, - const char *match) { + const char *match, + struct bus_match_component *components, + unsigned n_components) { const char *e; assert(bus); - assert(match); - e = internal_match(bus, match); + if (!bus->bus_client) + return -EINVAL; + + e = append_eavesdrop(bus, match); return sd_bus_call_method( bus, @@ -658,21 +629,7 @@ static int bus_add_match_internal_dbus1( e); } -int bus_add_match_internal( - sd_bus *bus, - const char *match, - struct bus_match_component *components, - unsigned n_components) { - - assert(bus); - - if (!bus->bus_client) - return -EINVAL; - - return bus_add_match_internal_dbus1(bus, match); -} - -static int bus_remove_match_internal_dbus1( +int bus_remove_match_internal( sd_bus *bus, const char *match) { @@ -681,7 +638,10 @@ static int bus_remove_match_internal_dbus1( assert(bus); assert(match); - e = internal_match(bus, match); + if (!bus->bus_client) + return -EINVAL; + + e = append_eavesdrop(bus, match); return sd_bus_call_method( bus, @@ -695,18 +655,6 @@ static int bus_remove_match_internal_dbus1( e); } -int bus_remove_match_internal( - sd_bus *bus, - const char *match) { - - assert(bus); - - if (!bus->bus_client) - return -EINVAL; - - return bus_remove_match_internal_dbus1(bus, match); -} - _public_ int sd_bus_get_name_machine_id(sd_bus *bus, const char *name, sd_id128_t *machine) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL, *m = NULL; const char *mid; From 98c5bbc85dc05c9dbccc1d289000dc554892d0d1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Dec 2017 20:10:13 +0100 Subject: [PATCH 18/46] sd-bus: add APIs to request/release names asynchronously They do the same thing as their synchronous counterparts, but only enqueue the operation, thus removing synchronization points during service initialization. If the callback function is passed as NULL we'll fallback to generic implementations of the reply handlers, that terminate the connection if the requested name cannot be acquired, under the assumption that not being able to acquire the name is a technical problem. --- src/libsystemd/libsystemd.sym | 2 + src/libsystemd/sd-bus/bus-control.c | 250 ++++++++++++++++++++++++--- src/libsystemd/sd-bus/bus-internal.h | 2 + src/libsystemd/sd-bus/sd-bus.c | 2 +- src/systemd/sd-bus.h | 2 + 5 files changed, 237 insertions(+), 21 deletions(-) diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index 4229e0aeb18..9acaf078338 100644 --- a/src/libsystemd/libsystemd.sym +++ b/src/libsystemd/libsystemd.sym @@ -535,4 +535,6 @@ LIBSYSTEMD_237 { global: sd_bus_set_watch_bind; sd_bus_get_watch_bind; + sd_bus_request_name_async; + sd_bus_release_name_async; } LIBSYSTEMD_236; diff --git a/src/libsystemd/sd-bus/bus-control.c b/src/libsystemd/sd-bus/bus-control.c index e1e75da6797..40380013184 100644 --- a/src/libsystemd/sd-bus/bus-control.c +++ b/src/libsystemd/sd-bus/bus-control.c @@ -57,14 +57,18 @@ _public_ int sd_bus_get_unique_name(sd_bus *bus, const char **unique) { return 0; } -_public_ int sd_bus_request_name(sd_bus *bus, const char *name, uint64_t flags) { - _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - uint32_t ret, param = 0; - int r; +static int validate_request_name_parameters( + sd_bus *bus, + const char *name, + uint64_t flags, + uint32_t *ret_param) { + + uint32_t param = 0; + + assert(bus); + assert(name); + assert(ret_param); - assert_return(bus, -EINVAL); - assert_return(name, -EINVAL); - assert_return(!bus_pid_changed(bus), -ECHILD); assert_return(!(flags & ~(SD_BUS_NAME_ALLOW_REPLACEMENT|SD_BUS_NAME_REPLACE_EXISTING|SD_BUS_NAME_QUEUE)), -EINVAL); assert_return(service_name_is_valid(name), -EINVAL); assert_return(name[0] != ':', -EINVAL); @@ -86,6 +90,28 @@ _public_ int sd_bus_request_name(sd_bus *bus, const char *name, uint64_t flags) if (!(flags & SD_BUS_NAME_QUEUE)) param |= BUS_NAME_DO_NOT_QUEUE; + *ret_param = param; + + return 0; +} + +_public_ int sd_bus_request_name( + sd_bus *bus, + const char *name, + uint64_t flags) { + + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + uint32_t ret, param = 0; + int r; + + assert_return(bus, -EINVAL); + assert_return(name, -EINVAL); + assert_return(!bus_pid_changed(bus), -ECHILD); + + r = validate_request_name_parameters(bus, name, flags, ¶m); + if (r < 0) + return r; + r = sd_bus_call_method( bus, "org.freedesktop.DBus", @@ -104,26 +130,112 @@ _public_ int sd_bus_request_name(sd_bus *bus, const char *name, uint64_t flags) if (r < 0) return r; - if (ret == BUS_NAME_ALREADY_OWNER) + switch (ret) { + + case BUS_NAME_ALREADY_OWNER: return -EALREADY; - else if (ret == BUS_NAME_EXISTS) + + case BUS_NAME_EXISTS: return -EEXIST; - else if (ret == BUS_NAME_IN_QUEUE) + + case BUS_NAME_IN_QUEUE: return 0; - else if (ret == BUS_NAME_PRIMARY_OWNER) + + case BUS_NAME_PRIMARY_OWNER: return 1; + } return -EIO; } -_public_ int sd_bus_release_name(sd_bus *bus, const char *name) { - _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; +static int default_request_name_handler( + sd_bus_message *m, + void *userdata, + sd_bus_error *ret_error) { + uint32_t ret; int r; + assert(m); + + if (sd_bus_message_is_method_error(m, NULL)) { + log_debug_errno(sd_bus_message_get_errno(m), + "Unable to request name, failing connection: %s", + sd_bus_message_get_error(m)->message); + + bus_enter_closing(sd_bus_message_get_bus(m)); + return 1; + } + + r = sd_bus_message_read(m, "u", &ret); + if (r < 0) + return r; + + switch (ret) { + + case BUS_NAME_ALREADY_OWNER: + log_debug("Already owner of requested service name, ignoring."); + return 1; + + case BUS_NAME_IN_QUEUE: + log_debug("In queue for requested service name."); + return 1; + + case BUS_NAME_PRIMARY_OWNER: + log_debug("Successfully acquired requested service name."); + return 1; + + case BUS_NAME_EXISTS: + log_debug("Requested service name already owned, failing connection."); + bus_enter_closing(sd_bus_message_get_bus(m)); + return 1; + } + + log_debug("Unexpected response from RequestName(), failing connection."); + bus_enter_closing(sd_bus_message_get_bus(m)); + return 1; +} + +_public_ int sd_bus_request_name_async( + sd_bus *bus, + sd_bus_slot **ret_slot, + const char *name, + uint64_t flags, + sd_bus_message_handler_t callback, + void *userdata) { + + uint32_t param = 0; + int r; + assert_return(bus, -EINVAL); assert_return(name, -EINVAL); assert_return(!bus_pid_changed(bus), -ECHILD); + + r = validate_request_name_parameters(bus, name, flags, ¶m); + if (r < 0) + return r; + + return sd_bus_call_method_async( + bus, + ret_slot, + "org.freedesktop.DBus", + "/org/freedesktop/DBus", + "org.freedesktop.DBus", + "RequestName", + callback ?: default_request_name_handler, + userdata, + "su", + name, + param); +} + +static int validate_release_name_parameters( + sd_bus *bus, + const char *name) { + + assert(bus); + assert(name); + assert_return(service_name_is_valid(name), -EINVAL); assert_return(name[0] != ':', -EINVAL); @@ -137,6 +249,25 @@ _public_ int sd_bus_release_name(sd_bus *bus, const char *name) { if (!BUS_IS_OPEN(bus->state)) return -ENOTCONN; + return 0; +} + +_public_ int sd_bus_release_name( + sd_bus *bus, + const char *name) { + + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + uint32_t ret; + int r; + + assert_return(bus, -EINVAL); + assert_return(name, -EINVAL); + assert_return(!bus_pid_changed(bus), -ECHILD); + + r = validate_release_name_parameters(bus, name); + if (r < 0) + return r; + r = sd_bus_call_method( bus, "org.freedesktop.DBus", @@ -153,14 +284,93 @@ _public_ int sd_bus_release_name(sd_bus *bus, const char *name) { r = sd_bus_message_read(reply, "u", &ret); if (r < 0) return r; - if (ret == BUS_NAME_NON_EXISTENT) - return -ESRCH; - if (ret == BUS_NAME_NOT_OWNER) - return -EADDRINUSE; - if (ret == BUS_NAME_RELEASED) - return 0; - return -EINVAL; + switch (ret) { + + case BUS_NAME_NON_EXISTENT: + return -ESRCH; + + case BUS_NAME_NOT_OWNER: + return -EADDRINUSE; + + case BUS_NAME_RELEASED: + return 0; + } + + return -EIO; +} + +static int default_release_name_handler( + sd_bus_message *m, + void *userdata, + sd_bus_error *ret_error) { + + uint32_t ret; + int r; + + assert(m); + + if (sd_bus_message_is_method_error(m, NULL)) { + log_debug_errno(sd_bus_message_get_errno(m), + "Unable to release name, failing connection: %s", + sd_bus_message_get_error(m)->message); + + bus_enter_closing(sd_bus_message_get_bus(m)); + return 1; + } + + r = sd_bus_message_read(m, "u", &ret); + if (r < 0) + return r; + + switch (ret) { + + case BUS_NAME_NON_EXISTENT: + log_debug("Name asked to release is not taken currently, ignoring."); + return 1; + + case BUS_NAME_NOT_OWNER: + log_debug("Name asked to release is owned by somebody else, ignoring."); + return 1; + + case BUS_NAME_RELEASED: + log_debug("Name successfully released."); + return 1; + } + + log_debug("Unexpected response from ReleaseName(), failing connection."); + bus_enter_closing(sd_bus_message_get_bus(m)); + return 1; +} + +_public_ int sd_bus_release_name_async( + sd_bus *bus, + sd_bus_slot **ret_slot, + const char *name, + sd_bus_message_handler_t callback, + void *userdata) { + + int r; + + assert_return(bus, -EINVAL); + assert_return(name, -EINVAL); + assert_return(!bus_pid_changed(bus), -ECHILD); + + r = validate_release_name_parameters(bus, name); + if (r < 0) + return r; + + return sd_bus_call_method_async( + bus, + ret_slot, + "org.freedesktop.DBus", + "/org/freedesktop/DBus", + "org.freedesktop.DBus", + "ReleaseName", + callback ?: default_release_name_handler, + userdata, + "s", + name); } _public_ int sd_bus_list_names(sd_bus *bus, char ***acquired, char ***activatable) { diff --git a/src/libsystemd/sd-bus/bus-internal.h b/src/libsystemd/sd-bus/bus-internal.h index 79012b0ad0e..bf395b1f220 100644 --- a/src/libsystemd/sd-bus/bus-internal.h +++ b/src/libsystemd/sd-bus/bus-internal.h @@ -407,3 +407,5 @@ int bus_maybe_reply_error(sd_bus_message *m, int r, sd_bus_error *error); if (!assert_log(expr, #expr)) \ return sd_bus_error_set_errno(error, r); \ } while (false) + +void bus_enter_closing(sd_bus *bus); diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 2c20ad6ebf8..d551035cd66 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -1354,7 +1354,7 @@ _public_ sd_bus* sd_bus_flush_close_unref(sd_bus *bus) { return sd_bus_unref(bus); } -static void bus_enter_closing(sd_bus *bus) { +void bus_enter_closing(sd_bus *bus) { assert(bus); if (!IN_SET(bus->state, BUS_WATCH_BIND, BUS_OPENING, BUS_AUTHENTICATING, BUS_HELLO, BUS_RUNNING)) diff --git a/src/systemd/sd-bus.h b/src/systemd/sd-bus.h index 66bc48842bf..9cebafd304b 100644 --- a/src/systemd/sd-bus.h +++ b/src/systemd/sd-bus.h @@ -302,7 +302,9 @@ int sd_bus_message_rewind(sd_bus_message *m, int complete); int sd_bus_get_unique_name(sd_bus *bus, const char **unique); int sd_bus_request_name(sd_bus *bus, const char *name, uint64_t flags); +int sd_bus_request_name_async(sd_bus *bus, sd_bus_slot **ret_slot, const char *name, uint64_t flags, sd_bus_message_handler_t callback, void *userdata); int sd_bus_release_name(sd_bus *bus, const char *name); +int sd_bus_release_name_async(sd_bus *bus, sd_bus_slot **ret_slot, const char *name, sd_bus_message_handler_t callback, void *userdata); int sd_bus_list_names(sd_bus *bus, char ***acquired, char ***activatable); /* free the results */ int sd_bus_get_name_creds(sd_bus *bus, const char *name, uint64_t mask, sd_bus_creds **creds); /* unref the result! */ int sd_bus_get_name_machine_id(sd_bus *bus, const char *name, sd_id128_t *machine); From 0259b87f5e5d830c05e97cc285c49a33f01cb9f2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Dec 2017 20:34:21 +0100 Subject: [PATCH 19/46] sd-bus: drop unused parameters from bus_add_match_internal() We don't need the match components anymore, since kdbus is gone, hence drop it. --- src/libsystemd/sd-bus/bus-control.c | 4 +--- src/libsystemd/sd-bus/bus-control.h | 4 +--- src/libsystemd/sd-bus/sd-bus.c | 2 +- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-control.c b/src/libsystemd/sd-bus/bus-control.c index 40380013184..aa247db07f1 100644 --- a/src/libsystemd/sd-bus/bus-control.c +++ b/src/libsystemd/sd-bus/bus-control.c @@ -814,9 +814,7 @@ _public_ int sd_bus_get_owner_creds(sd_bus *bus, uint64_t mask, sd_bus_creds **r int bus_add_match_internal( sd_bus *bus, - const char *match, - struct bus_match_component *components, - unsigned n_components) { + const char *match) { const char *e; diff --git a/src/libsystemd/sd-bus/bus-control.h b/src/libsystemd/sd-bus/bus-control.h index c9d434c6074..c4596fa2943 100644 --- a/src/libsystemd/sd-bus/bus-control.h +++ b/src/libsystemd/sd-bus/bus-control.h @@ -22,7 +22,5 @@ #include "sd-bus.h" -#include "bus-match.h" - -int bus_add_match_internal(sd_bus *bus, const char *match, struct bus_match_component *components, unsigned n_components); +int bus_add_match_internal(sd_bus *bus, const char *match); int bus_remove_match_internal(sd_bus *bus, const char *match); diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index d551035cd66..6ef352305a2 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -2980,7 +2980,7 @@ _public_ int sd_bus_add_match( goto finish; } - r = bus_add_match_internal(bus, s->match_callback.match_string, components, n_components); + r = bus_add_match_internal(bus, s->match_callback.match_string); if (r < 0) goto finish; From aec7b7bae7ccbf0b25fdffb4d9249c420cef308f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Dec 2017 20:45:35 +0100 Subject: [PATCH 20/46] sd-bus: remove bus_remove_match_by_string() helper which is unused --- src/libsystemd/sd-bus/bus-internal.h | 2 -- src/libsystemd/sd-bus/sd-bus.c | 31 ---------------------------- 2 files changed, 33 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-internal.h b/src/libsystemd/sd-bus/bus-internal.h index bf395b1f220..39fc35d342f 100644 --- a/src/libsystemd/sd-bus/bus-internal.h +++ b/src/libsystemd/sd-bus/bus-internal.h @@ -396,8 +396,6 @@ int bus_set_address_user(sd_bus *bus); int bus_set_address_system_remote(sd_bus *b, const char *host); int bus_set_address_system_machine(sd_bus *b, const char *machine); -int bus_remove_match_by_string(sd_bus *bus, const char *match, sd_bus_message_handler_t callback, void *userdata); - int bus_get_root_path(sd_bus *bus); int bus_maybe_reply_error(sd_bus_message *m, int r, sd_bus_error *error); diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 6ef352305a2..a1f8e5bf2e2 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -3004,37 +3004,6 @@ finish: return r; } -int bus_remove_match_by_string( - sd_bus *bus, - const char *match, - sd_bus_message_handler_t callback, - void *userdata) { - - struct bus_match_component *components = NULL; - unsigned n_components = 0; - struct match_callback *c; - int r = 0; - - assert_return(bus, -EINVAL); - assert_return(match, -EINVAL); - assert_return(!bus_pid_changed(bus), -ECHILD); - - r = bus_match_parse(match, &components, &n_components); - if (r < 0) - goto finish; - - r = bus_match_find(&bus->match_callbacks, components, n_components, NULL, NULL, &c); - if (r <= 0) - goto finish; - - sd_bus_slot_unref(container_of(c, sd_bus_slot, match_callback)); - -finish: - bus_match_parse_free(components, n_components); - - return r; -} - bool bus_pid_changed(sd_bus *bus) { assert(bus); From acd340158aacce126cb0736681b1fb7cacb07e81 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Dec 2017 20:50:26 +0100 Subject: [PATCH 21/46] sd-bus: when removing a server-side match, do so in "fire and forget" fashion We currently wait for the RemoveMatch() reply, but then ignore what it actually says. Let's optimize this a bit, and not even ask for an answer back: just enqueue the RemoveMatch() operation, and do not request not wait for any answer. --- src/libsystemd/sd-bus/bus-control.c | 5 ++++- src/libsystemd/sd-bus/bus-slot.c | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-control.c b/src/libsystemd/sd-bus/bus-control.c index aa247db07f1..7182616d2ac 100644 --- a/src/libsystemd/sd-bus/bus-control.c +++ b/src/libsystemd/sd-bus/bus-control.c @@ -851,8 +851,11 @@ int bus_remove_match_internal( e = append_eavesdrop(bus, match); - return sd_bus_call_method( + /* Fire and forget */ + + return sd_bus_call_method_async( bus, + NULL, "org.freedesktop.DBus", "/org/freedesktop/DBus", "org.freedesktop.DBus", diff --git a/src/libsystemd/sd-bus/bus-slot.c b/src/libsystemd/sd-bus/bus-slot.c index f8de699b3d2..0b5e2b7db1f 100644 --- a/src/libsystemd/sd-bus/bus-slot.c +++ b/src/libsystemd/sd-bus/bus-slot.c @@ -94,7 +94,7 @@ void bus_slot_disconnect(sd_bus_slot *slot) { case BUS_MATCH_CALLBACK: if (slot->match_added) - bus_remove_match_internal(slot->bus, slot->match_callback.match_string); + (void) bus_remove_match_internal(slot->bus, slot->match_callback.match_string); slot->bus->match_callbacks_modified = true; bus_match_remove(&slot->bus->match_callbacks, &slot->match_callback); From 7593c7a4953550d61ec53289734abf4ad3065f27 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Dec 2017 21:37:03 +0100 Subject: [PATCH 22/46] sd-bus: add asynchronous version of sd_bus_match() We usually enqueue a number of these calls on each service initialization. Let's do this asynchronously, and thus remove synchronization points. This improves both performance behaviour and reduces the chances to deadlock. --- src/libsystemd/libsystemd.sym | 1 + src/libsystemd/sd-bus/bus-control.c | 28 +++++++ src/libsystemd/sd-bus/bus-control.h | 2 + src/libsystemd/sd-bus/bus-internal.h | 3 + src/libsystemd/sd-bus/bus-slot.c | 5 ++ src/libsystemd/sd-bus/sd-bus.c | 106 +++++++++++++++++++++++++-- src/systemd/sd-bus.h | 1 + 7 files changed, 140 insertions(+), 6 deletions(-) diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index 9acaf078338..820f3e13e1b 100644 --- a/src/libsystemd/libsystemd.sym +++ b/src/libsystemd/libsystemd.sym @@ -537,4 +537,5 @@ global: sd_bus_get_watch_bind; sd_bus_request_name_async; sd_bus_release_name_async; + sd_bus_add_match_async; } LIBSYSTEMD_236; diff --git a/src/libsystemd/sd-bus/bus-control.c b/src/libsystemd/sd-bus/bus-control.c index 7182616d2ac..4adc996a86b 100644 --- a/src/libsystemd/sd-bus/bus-control.c +++ b/src/libsystemd/sd-bus/bus-control.c @@ -836,6 +836,34 @@ int bus_add_match_internal( "s", e); } +int bus_add_match_internal_async( + sd_bus *bus, + sd_bus_slot **ret_slot, + const char *match, + sd_bus_message_handler_t callback, + void *userdata) { + + const char *e; + + assert(bus); + + if (!bus->bus_client) + return -EINVAL; + + e = append_eavesdrop(bus, match); + + return sd_bus_call_method_async( + bus, + ret_slot, + "org.freedesktop.DBus", + "/org/freedesktop/DBus", + "org.freedesktop.DBus", + "AddMatch", + callback, + userdata, + "s", + e); +} int bus_remove_match_internal( sd_bus *bus, diff --git a/src/libsystemd/sd-bus/bus-control.h b/src/libsystemd/sd-bus/bus-control.h index c4596fa2943..3d9acebaf67 100644 --- a/src/libsystemd/sd-bus/bus-control.h +++ b/src/libsystemd/sd-bus/bus-control.h @@ -23,4 +23,6 @@ #include "sd-bus.h" int bus_add_match_internal(sd_bus *bus, const char *match); +int bus_add_match_internal_async(sd_bus *bus, sd_bus_slot **ret, const char *match, sd_bus_message_handler_t callback, void *userdata); + int bus_remove_match_internal(sd_bus *bus, const char *match); diff --git a/src/libsystemd/sd-bus/bus-internal.h b/src/libsystemd/sd-bus/bus-internal.h index 39fc35d342f..29366c4c568 100644 --- a/src/libsystemd/sd-bus/bus-internal.h +++ b/src/libsystemd/sd-bus/bus-internal.h @@ -53,6 +53,9 @@ struct filter_callback { struct match_callback { sd_bus_message_handler_t callback; + sd_bus_message_handler_t install_callback; + + sd_bus_slot *install_slot; /* The AddMatch() call */ unsigned last_iteration; diff --git a/src/libsystemd/sd-bus/bus-slot.c b/src/libsystemd/sd-bus/bus-slot.c index 0b5e2b7db1f..f7c9bfdf65d 100644 --- a/src/libsystemd/sd-bus/bus-slot.c +++ b/src/libsystemd/sd-bus/bus-slot.c @@ -96,6 +96,11 @@ void bus_slot_disconnect(sd_bus_slot *slot) { if (slot->match_added) (void) bus_remove_match_internal(slot->bus, slot->match_callback.match_string); + if (slot->match_callback.install_slot) { + bus_slot_disconnect(slot->match_callback.install_slot); + slot->match_callback.install_slot = sd_bus_slot_unref(slot->match_callback.install_slot); + } + slot->bus->match_callbacks_modified = true; bus_match_remove(&slot->bus->match_callbacks, &slot->match_callback); diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index a1f8e5bf2e2..2f570f1aab7 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -2934,11 +2934,78 @@ _public_ int sd_bus_add_filter( return 0; } -_public_ int sd_bus_add_match( +static int add_match_callback( + sd_bus_message *m, + void *userdata, + sd_bus_error *ret_error) { + + sd_bus_slot *match_slot = userdata; + bool failed = false; + int r; + + assert(m); + assert(match_slot); + + sd_bus_slot_ref(match_slot); + + if (sd_bus_message_is_method_error(m, NULL)) { + log_debug_errno(sd_bus_message_get_errno(m), + "Unable to add match %s, failing connection: %s", + match_slot->match_callback.match_string, + sd_bus_message_get_error(m)->message); + + failed = true; + } else + log_debug("Match %s successfully installed.", match_slot->match_callback.match_string); + + if (match_slot->match_callback.install_callback) { + sd_bus *bus; + + bus = sd_bus_message_get_bus(m); + + /* This function has been called as slot handler, and we want to call another slot handler. Let's + * update the slot callback metadata temporarily with our own data, and then revert back to the old + * values. */ + + assert(bus->current_slot == match_slot->match_callback.install_slot); + assert(bus->current_handler == add_match_callback); + assert(bus->current_userdata == userdata); + + bus->current_slot = match_slot; + bus->current_handler = match_slot->match_callback.install_callback; + bus->current_userdata = match_slot->userdata; + + r = match_slot->match_callback.install_callback(m, match_slot->userdata, ret_error); + + bus->current_slot = match_slot->match_callback.install_slot; + bus->current_handler = add_match_callback; + bus->current_userdata = userdata; + + match_slot->match_callback.install_slot = sd_bus_slot_unref(match_slot->match_callback.install_slot); + } else { + if (failed) /* Generic failure handling: destroy the connection */ + bus_enter_closing(sd_bus_message_get_bus(m)); + + r = 1; + } + + if (failed && match_slot->floating) { + bus_slot_disconnect(match_slot); + sd_bus_slot_unref(match_slot); + } + + sd_bus_slot_unref(match_slot); + + return r; +} + +static int bus_add_match_full( sd_bus *bus, sd_bus_slot **slot, + bool asynchronous, const char *match, sd_bus_message_handler_t callback, + sd_bus_message_handler_t install_callback, void *userdata) { struct bus_match_component *components = NULL; @@ -2961,18 +3028,17 @@ _public_ int sd_bus_add_match( } s->match_callback.callback = callback; + s->match_callback.install_callback = install_callback; if (bus->bus_client) { enum bus_match_scope scope; scope = bus_match_get_scope(components, n_components); - /* Do not install server-side matches for matches - * against the local service, interface or bus path. */ + /* Do not install server-side matches for matches against the local service, interface or bus path. */ if (scope != BUS_MATCH_LOCAL) { - /* We store the original match string, so that - * we can use it to remove the match again. */ + /* We store the original match string, so that we can use it to remove the match again. */ s->match_callback.match_string = strdup(match); if (!s->match_callback.match_string) { @@ -2980,7 +3046,14 @@ _public_ int sd_bus_add_match( goto finish; } - r = bus_add_match_internal(bus, s->match_callback.match_string); + if (asynchronous) + r = bus_add_match_internal_async(bus, + &s->match_callback.install_slot, + s->match_callback.match_string, + add_match_callback, + s); + else + r = bus_add_match_internal(bus, s->match_callback.match_string); if (r < 0) goto finish; @@ -3004,6 +3077,27 @@ finish: return r; } +_public_ int sd_bus_add_match( + sd_bus *bus, + sd_bus_slot **slot, + const char *match, + sd_bus_message_handler_t callback, + void *userdata) { + + return bus_add_match_full(bus, slot, false, match, callback, NULL, userdata); +} + +_public_ int sd_bus_add_match_async( + sd_bus *bus, + sd_bus_slot **slot, + const char *match, + sd_bus_message_handler_t callback, + sd_bus_message_handler_t install_callback, + void *userdata) { + + return bus_add_match_full(bus, slot, true, match, callback, install_callback, userdata); +} + bool bus_pid_changed(sd_bus *bus) { assert(bus); diff --git a/src/systemd/sd-bus.h b/src/systemd/sd-bus.h index 9cebafd304b..8877778769c 100644 --- a/src/systemd/sd-bus.h +++ b/src/systemd/sd-bus.h @@ -195,6 +195,7 @@ sd_event *sd_bus_get_event(sd_bus *bus); int sd_bus_add_filter(sd_bus *bus, sd_bus_slot **slot, sd_bus_message_handler_t callback, void *userdata); int sd_bus_add_match(sd_bus *bus, sd_bus_slot **slot, const char *match, sd_bus_message_handler_t callback, void *userdata); +int sd_bus_add_match_async(sd_bus *bus, sd_bus_slot **slot, const char *match, sd_bus_message_handler_t callback, sd_bus_message_handler_t install_callback, void *userdata); int sd_bus_add_object(sd_bus *bus, sd_bus_slot **slot, const char *path, sd_bus_message_handler_t callback, void *userdata); int sd_bus_add_fallback(sd_bus *bus, sd_bus_slot **slot, const char *prefix, sd_bus_message_handler_t callback, void *userdata); int sd_bus_add_object_vtable(sd_bus *bus, sd_bus_slot **slot, const char *path, const char *interface, const sd_bus_vtable *vtable, void *userdata); From b423e4fb73866e529869b348efb7169ee91f00c9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Dec 2017 21:52:50 +0100 Subject: [PATCH 23/46] sd-bus: add new API sd_bus_match_signal() + sd_bus_match_signal_asnyc() These are convenience helpers that hide the match string logic (which we probably should never have exposed), and instead just takes regular C arguments. --- src/libsystemd/libsystemd.sym | 2 + src/libsystemd/sd-bus/bus-convenience.c | 66 +++++++++++++++++++++++++ src/systemd/sd-bus.h | 3 ++ 3 files changed, 71 insertions(+) diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index 820f3e13e1b..05c3b99bd6b 100644 --- a/src/libsystemd/libsystemd.sym +++ b/src/libsystemd/libsystemd.sym @@ -538,4 +538,6 @@ global: sd_bus_request_name_async; sd_bus_release_name_async; sd_bus_add_match_async; + sd_bus_match_signal; + sd_bus_match_signal_async; } LIBSYSTEMD_236; diff --git a/src/libsystemd/sd-bus/bus-convenience.c b/src/libsystemd/sd-bus/bus-convenience.c index 9d3b5964291..7d702e63765 100644 --- a/src/libsystemd/sd-bus/bus-convenience.c +++ b/src/libsystemd/sd-bus/bus-convenience.c @@ -615,3 +615,69 @@ _public_ int sd_bus_query_sender_privilege(sd_bus_message *call, int capability) return 0; } + +#define make_expression(sender, path, interface, member) \ + strjoina( \ + "type='signal'", \ + sender ? ",sender='" : "", \ + sender ?: "", \ + sender ? "'" : "", \ + path ? ",path='" : "", \ + path ?: "", \ + path ? "'" : "", \ + interface ? ",interface='" : "", \ + interface ?: "", \ + interface ? "'" : "", \ + member ? ",member='" : "", \ + member ?: "", \ + member ? "'" : "" \ + ) + +_public_ int sd_bus_match_signal( + sd_bus *bus, + sd_bus_slot **ret, + const char *sender, + const char *path, + const char *interface, + const char *member, + sd_bus_message_handler_t callback, + void *userdata) { + + const char *expression; + + assert_return(bus, -EINVAL); + assert_return(!bus_pid_changed(bus), -ECHILD); + assert_return(!sender || service_name_is_valid(sender), -EINVAL); + assert_return(!path || object_path_is_valid(path), -EINVAL); + assert_return(!interface || interface_name_is_valid(interface), -EINVAL); + assert_return(!member || member_name_is_valid(member), -EINVAL); + + expression = make_expression(sender, path, interface, member); + + return sd_bus_add_match(bus, ret, expression, callback, userdata); +} + +_public_ int sd_bus_match_signal_async( + sd_bus *bus, + sd_bus_slot **ret, + const char *sender, + const char *path, + const char *interface, + const char *member, + sd_bus_message_handler_t callback, + sd_bus_message_handler_t install_callback, + void *userdata) { + + const char *expression; + + assert_return(bus, -EINVAL); + assert_return(!bus_pid_changed(bus), -ECHILD); + assert_return(!sender || service_name_is_valid(sender), -EINVAL); + assert_return(!path || object_path_is_valid(path), -EINVAL); + assert_return(!interface || interface_name_is_valid(interface), -EINVAL); + assert_return(!member || member_name_is_valid(member), -EINVAL); + + expression = make_expression(sender, path, interface, member); + + return sd_bus_add_match_async(bus, ret, expression, callback, install_callback, userdata); +} diff --git a/src/systemd/sd-bus.h b/src/systemd/sd-bus.h index 8877778769c..9a6581348c1 100644 --- a/src/systemd/sd-bus.h +++ b/src/systemd/sd-bus.h @@ -341,6 +341,9 @@ int sd_bus_emit_interfaces_removed(sd_bus *bus, const char *path, const char *in int sd_bus_query_sender_creds(sd_bus_message *call, uint64_t mask, sd_bus_creds **creds); int sd_bus_query_sender_privilege(sd_bus_message *call, int capability); +int sd_bus_match_signal(sd_bus *bus, sd_bus_slot **ret, const char *sender, const char *path, const char *interface, const char *member, sd_bus_message_handler_t callback, void *userdata); +int sd_bus_match_signal_async(sd_bus *bus, sd_bus_slot **ret, const char *sender, const char *path, const char *interface, const char *member, sd_bus_message_handler_t match_callback, sd_bus_message_handler_t add_callback, void *userdata); + /* Credential handling */ int sd_bus_creds_new_from_pid(sd_bus_creds **ret, pid_t pid, uint64_t creds_mask); From 45754e01eaf0d473ea5b5413c0473d39dc5552eb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Dec 2017 21:58:14 +0100 Subject: [PATCH 24/46] sd-bus: when disconnecting a slot, also reset its memory Yes, we aren#t accessing this anymore after, but it's still nicer if this is actually guaranteed. --- src/libsystemd/sd-bus/bus-slot.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-slot.c b/src/libsystemd/sd-bus/bus-slot.c index f7c9bfdf65d..9a563717152 100644 --- a/src/libsystemd/sd-bus/bus-slot.c +++ b/src/libsystemd/sd-bus/bus-slot.c @@ -104,7 +104,7 @@ void bus_slot_disconnect(sd_bus_slot *slot) { slot->bus->match_callbacks_modified = true; bus_match_remove(&slot->bus->match_callbacks, &slot->match_callback); - free(slot->match_callback.match_string); + slot->match_callback.match_string = mfree(slot->match_callback.match_string); break; @@ -179,7 +179,7 @@ void bus_slot_disconnect(sd_bus_slot *slot) { } } - free(slot->node_vtable.interface); + slot->node_vtable.interface = mfree(slot->node_vtable.interface); if (slot->node_vtable.node) { LIST_REMOVE(vtables, slot->node_vtable.node->vtables, &slot->node_vtable); From 0c0b9306470baa9498900bc7b46149d4a9d06738 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Dec 2017 22:10:11 +0100 Subject: [PATCH 25/46] tree-wide: make name requesting asynchronous in all our services This optimizes service startup a bit, and makes it less prone to deadlocks. --- src/core/dbus.c | 12 +++++------- src/hostname/hostnamed.c | 4 ++-- src/import/importd.c | 4 ++-- src/locale/localed.c | 4 ++-- src/login/logind.c | 4 ++-- src/machine/machined.c | 4 ++-- src/network/networkd-manager.c | 4 ++-- src/resolve/resolved-bus.c | 4 ++-- src/timedate/timedated.c | 4 ++-- 9 files changed, 21 insertions(+), 23 deletions(-) diff --git a/src/core/dbus.c b/src/core/dbus.c index 115d071eb42..5dc019fae1d 100644 --- a/src/core/dbus.c +++ b/src/core/dbus.c @@ -826,14 +826,12 @@ static int bus_setup_api(Manager *m, sd_bus *bus) { if (r < 0) log_warning_errno(r, "Failed to subscribe to activation signal: %m"); - /* Allow replacing of our name, to ease implementation of - * reexecution, where we keep the old connection open until - * after the new connection is set up and the name installed - * to allow clients to synchronously wait for reexecution to - * finish */ - r = sd_bus_request_name(bus,"org.freedesktop.systemd1", SD_BUS_NAME_REPLACE_EXISTING|SD_BUS_NAME_ALLOW_REPLACEMENT); + /* Allow replacing of our name, to ease implementation of reexecution, where we keep the old connection open + * until after the new connection is set up and the name installed to allow clients to synchronously wait for + * reexecution to finish */ + r = sd_bus_request_name_async(bus, NULL, "org.freedesktop.systemd1", SD_BUS_NAME_REPLACE_EXISTING|SD_BUS_NAME_ALLOW_REPLACEMENT, NULL, NULL); if (r < 0) - return log_error_errno(r, "Failed to register name: %m"); + return log_error_errno(r, "Failed to request name: %m"); r = manager_sync_bus_names(m, bus); if (r < 0) diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c index 5feaa60c99f..1c8c76934ce 100644 --- a/src/hostname/hostnamed.c +++ b/src/hostname/hostnamed.c @@ -680,9 +680,9 @@ static int connect_bus(Context *c, sd_event *event, sd_bus **_bus) { if (r < 0) return log_error_errno(r, "Failed to register object: %m"); - r = sd_bus_request_name(bus, "org.freedesktop.hostname1", 0); + r = sd_bus_request_name_async(bus, NULL, "org.freedesktop.hostname1", 0, NULL, NULL); if (r < 0) - return log_error_errno(r, "Failed to register name: %m"); + return log_error_errno(r, "Failed to request name: %m"); r = sd_bus_attach_event(bus, event, 0); if (r < 0) diff --git a/src/import/importd.c b/src/import/importd.c index 21af09fc45a..98ee1a2fab2 100644 --- a/src/import/importd.c +++ b/src/import/importd.c @@ -1149,9 +1149,9 @@ static int manager_add_bus_objects(Manager *m) { if (r < 0) return log_error_errno(r, "Failed to add transfer enumerator: %m"); - r = sd_bus_request_name(m->bus, "org.freedesktop.import1", 0); + r = sd_bus_request_name_async(m->bus, NULL, "org.freedesktop.import1", 0, NULL, NULL); if (r < 0) - return log_error_errno(r, "Failed to register name: %m"); + return log_error_errno(r, "Failed to request name: %m"); r = sd_bus_attach_event(m->bus, m->event, 0); if (r < 0) diff --git a/src/locale/localed.c b/src/locale/localed.c index 3e3f03e046b..02f5e8c6568 100644 --- a/src/locale/localed.c +++ b/src/locale/localed.c @@ -652,9 +652,9 @@ static int connect_bus(Context *c, sd_event *event, sd_bus **_bus) { if (r < 0) return log_error_errno(r, "Failed to register object: %m"); - r = sd_bus_request_name(bus, "org.freedesktop.locale1", 0); + r = sd_bus_request_name_async(bus, NULL, "org.freedesktop.locale1", 0, NULL, NULL); if (r < 0) - return log_error_errno(r, "Failed to register name: %m"); + return log_error_errno(r, "Failed to request name: %m"); r = sd_bus_attach_event(bus, event, 0); if (r < 0) diff --git a/src/login/logind.c b/src/login/logind.c index 49ca367e188..3e4b2fc3501 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -752,9 +752,9 @@ static int manager_connect_bus(Manager *m) { return r; } - r = sd_bus_request_name(m->bus, "org.freedesktop.login1", 0); + r = sd_bus_request_name_async(m->bus, NULL, "org.freedesktop.login1", 0, NULL, NULL); if (r < 0) - return log_error_errno(r, "Failed to register name: %m"); + return log_error_errno(r, "Failed to request name: %m"); r = sd_bus_attach_event(m->bus, m->event, SD_EVENT_PRIORITY_NORMAL); if (r < 0) diff --git a/src/machine/machined.c b/src/machine/machined.c index b5cb863e5d8..9ef121fe662 100644 --- a/src/machine/machined.c +++ b/src/machine/machined.c @@ -277,9 +277,9 @@ static int manager_connect_bus(Manager *m) { return r; } - r = sd_bus_request_name(m->bus, "org.freedesktop.machine1", 0); + r = sd_bus_request_name_async(m->bus, NULL, "org.freedesktop.machine1", 0, NULL, NULL); if (r < 0) - return log_error_errno(r, "Failed to register name: %m"); + return log_error_errno(r, "Failed to request name: %m"); r = sd_bus_attach_event(m->bus, m->event, 0); if (r < 0) diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index e3c514c8044..836f58c78ca 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -183,9 +183,9 @@ int manager_connect_bus(Manager *m) { if (r < 0) return log_error_errno(r, "Failed to add network enumerator: %m"); - r = sd_bus_request_name(m->bus, "org.freedesktop.network1", 0); + r = sd_bus_request_name_async(m->bus, NULL, "org.freedesktop.network1", 0, NULL, NULL); if (r < 0) - return log_error_errno(r, "Failed to register name: %m"); + return log_error_errno(r, "Failed to request name: %m"); r = sd_bus_attach_event(m->bus, m->event, 0); if (r < 0) diff --git a/src/resolve/resolved-bus.c b/src/resolve/resolved-bus.c index 9157d9ea688..5a93f894672 100644 --- a/src/resolve/resolved-bus.c +++ b/src/resolve/resolved-bus.c @@ -1921,9 +1921,9 @@ int manager_connect_bus(Manager *m) { if (r < 0) return log_error_errno(r, "Failed to register dnssd enumerator: %m"); - r = sd_bus_request_name(m->bus, "org.freedesktop.resolve1", 0); + r = sd_bus_request_name_async(m->bus, NULL, "org.freedesktop.resolve1", 0, NULL, NULL); if (r < 0) - return log_error_errno(r, "Failed to register name: %m"); + return log_error_errno(r, "Failed to request name: %m"); r = sd_bus_attach_event(m->bus, m->event, 0); if (r < 0) diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c index a55a929a495..822835cce99 100644 --- a/src/timedate/timedated.c +++ b/src/timedate/timedated.c @@ -675,9 +675,9 @@ static int connect_bus(Context *c, sd_event *event, sd_bus **_bus) { if (r < 0) return log_error_errno(r, "Failed to register object: %m"); - r = sd_bus_request_name(bus, "org.freedesktop.timedate1", 0); + r = sd_bus_request_name_async(bus, NULL, "org.freedesktop.timedate1", 0, NULL, NULL); if (r < 0) - return log_error_errno(r, "Failed to register name: %m"); + return log_error_errno(r, "Failed to request name: %m"); r = sd_bus_attach_event(bus, event, 0); if (r < 0) From 75152a4d6aedbfd3ee8b2d5782b9edf27407622a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 19 Dec 2017 12:29:04 +0100 Subject: [PATCH 26/46] tree-wide: install matches asynchronously Let's remove a number of synchronization points from our service startups: let's drop synchronous match installation, and let's opt for asynchronous instead. Also, let's use sd_bus_match_signal() instead of sd_bus_add_match() where we can. --- src/core/dbus.c | 44 ++++++++-------- src/core/unit.c | 2 +- src/libsystemd/sd-bus/bus-track.c | 4 +- src/libsystemd/sd-bus/test-bus-chat.c | 4 +- src/login/logind.c | 71 ++++++++++++------------- src/machine/machinectl.c | 37 +++++++------- src/machine/machined.c | 74 +++++++++++++-------------- src/network/networkd-manager.c | 17 +++--- src/nspawn/nspawn.c | 16 +++--- src/resolve/resolved-bus.c | 20 ++++---- src/run/run.c | 19 +++---- src/shared/bus-unit-util.c | 30 +++++------ src/shared/bus-util.c | 26 +++++----- src/systemctl/systemctl.c | 15 +++--- 14 files changed, 183 insertions(+), 196 deletions(-) diff --git a/src/core/dbus.c b/src/core/dbus.c index 5dc019fae1d..0d9f1f03467 100644 --- a/src/core/dbus.c +++ b/src/core/dbus.c @@ -604,18 +604,16 @@ static int bus_setup_disconnected_match(Manager *m, sd_bus *bus) { assert(m); assert(bus); - r = sd_bus_add_match( + r = sd_bus_match_signal_async( bus, NULL, - "sender='org.freedesktop.DBus.Local'," - "type='signal'," - "path='/org/freedesktop/DBus/Local'," - "interface='org.freedesktop.DBus.Local'," - "member='Disconnected'", - signal_disconnected, m); - + "org.freedesktop.DBus.Local", + "/org/freedesktop/DBus/Local", + "org.freedesktop.DBus.Local", + "Disconnected", + signal_disconnected, NULL, m); if (r < 0) - return log_error_errno(r, "Failed to register match for Disconnected message: %m"); + return log_error_errno(r, "Failed to request match for Disconnected message: %m"); return 0; } @@ -814,15 +812,14 @@ static int bus_setup_api(Manager *m, sd_bus *bus) { log_error_errno(r, "Failed to subscribe to NameOwnerChanged signal for '%s': %m", name); } - r = sd_bus_add_match( + r = sd_bus_match_signal_async( bus, NULL, - "type='signal'," - "sender='org.freedesktop.DBus'," - "path='/org/freedesktop/DBus'," - "interface='org.freedesktop.systemd1.Activator'," - "member='ActivationRequest'", - signal_activation_request, m); + "org.freedesktop.DBus", + "/org/freedesktop/DBus", + "org.freedesktop.systemd1.Activator", + "ActivationRequest", + signal_activation_request, NULL, m); if (r < 0) log_warning_errno(r, "Failed to subscribe to activation signal: %m"); @@ -856,7 +853,6 @@ static int bus_init_api(Manager *m) { r = sd_bus_open_system(&bus); else r = sd_bus_open_user(&bus); - if (r < 0) { log_debug("Failed to connect to API bus, retrying later..."); return 0; @@ -893,16 +889,16 @@ static int bus_setup_system(Manager *m, sd_bus *bus) { /* if we are a user instance we get the Released message via the system bus */ if (MANAGER_IS_USER(m)) { - r = sd_bus_add_match( + r = sd_bus_match_signal_async( bus, NULL, - "type='signal'," - "interface='org.freedesktop.systemd1.Agent'," - "member='Released'," - "path='/org/freedesktop/systemd1/agent'", - signal_agent_released, m); + NULL, + "/org/freedesktop/systemd1/agent", + "org.freedesktop.systemd1.Agent", + "Released", + signal_agent_released, NULL, m); if (r < 0) - log_warning_errno(r, "Failed to register Released match on system bus: %m"); + log_warning_errno(r, "Failed to request Released match on system bus: %m"); } log_debug("Successfully connected to system bus."); diff --git a/src/core/unit.c b/src/core/unit.c index 652587e6adc..d736d274de4 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -3042,7 +3042,7 @@ int unit_install_bus_match(Unit *u, sd_bus *bus, const char *name) { "member='NameOwnerChanged'," "arg0='", name, "'"); - return sd_bus_add_match(bus, &u->match_bus_slot, match, signal_name_owner_changed, u); + return sd_bus_add_match_async(bus, &u->match_bus_slot, match, signal_name_owner_changed, NULL, u); } int unit_watch_bus_name(Unit *u, const char *name) { diff --git a/src/libsystemd/sd-bus/bus-track.c b/src/libsystemd/sd-bus/bus-track.c index ab22d6e4de2..919cebda0b3 100644 --- a/src/libsystemd/sd-bus/bus-track.c +++ b/src/libsystemd/sd-bus/bus-track.c @@ -259,9 +259,7 @@ _public_ int sd_bus_track_add_name(sd_bus_track *track, const char *name) { bus_track_remove_from_queue(track); /* don't dispatch this while we work in it */ - track->n_adding++; /* make sure we aren't dispatched while we synchronously add this match */ - r = sd_bus_add_match(track->bus, &n->slot, match, on_name_owner_changed, track); - track->n_adding--; + r = sd_bus_add_match_async(track->bus, &n->slot, match, on_name_owner_changed, NULL, track); if (r < 0) { bus_track_add_to_queue(track); return r; diff --git a/src/libsystemd/sd-bus/test-bus-chat.c b/src/libsystemd/sd-bus/test-bus-chat.c index 1b2efb9bb4e..bd6721946ae 100644 --- a/src/libsystemd/sd-bus/test-bus-chat.c +++ b/src/libsystemd/sd-bus/test-bus-chat.c @@ -102,9 +102,9 @@ static int server_init(sd_bus **_bus) { goto fail; } - r = sd_bus_add_match(bus, NULL, "type='signal',interface='foo.bar',member='Notify'", match_callback, NULL); + r = sd_bus_match_signal(bus, NULL, NULL, NULL, "foo.bar", "Notify", match_callback, NULL); if (r < 0) { - log_error_errno(r, "Failed to add match: %m"); + log_error_errno(r, "Failed to request match: %m"); goto fail; } diff --git a/src/login/logind.c b/src/login/logind.c index 3e4b2fc3501..9500fd98920 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -696,48 +696,49 @@ static int manager_connect_bus(Manager *m) { if (r < 0) return log_error_errno(r, "Failed to add user enumerator: %m"); - r = sd_bus_add_match(m->bus, - NULL, - "type='signal'," - "sender='org.freedesktop.systemd1'," - "interface='org.freedesktop.systemd1.Manager'," - "member='JobRemoved'," - "path='/org/freedesktop/systemd1'", - match_job_removed, m); + r = sd_bus_match_signal_async( + m->bus, + NULL, + "org.freedesktop.systemd1", + "/org/freedesktop/systemd1", + "org.freedesktop.systemd1.Manager", + "JobRemoved", + match_job_removed, NULL, m); if (r < 0) - return log_error_errno(r, "Failed to add match for JobRemoved: %m"); + return log_error_errno(r, "Failed to request match for JobRemoved: %m"); - r = sd_bus_add_match(m->bus, - NULL, - "type='signal'," - "sender='org.freedesktop.systemd1'," - "interface='org.freedesktop.systemd1.Manager'," - "member='UnitRemoved'," - "path='/org/freedesktop/systemd1'", - match_unit_removed, m); + r = sd_bus_match_signal_async( + m->bus, + NULL, + "org.freedesktop.systemd1", + "/org/freedesktop/systemd1", + "org.freedesktop.systemd1.Manager", + "UnitRemoved", + match_unit_removed, NULL, m); if (r < 0) - return log_error_errno(r, "Failed to add match for UnitRemoved: %m"); + return log_error_errno(r, "Failed to request match for UnitRemoved: %m"); - r = sd_bus_add_match(m->bus, - NULL, - "type='signal'," - "sender='org.freedesktop.systemd1'," - "interface='org.freedesktop.DBus.Properties'," - "member='PropertiesChanged'", - match_properties_changed, m); + r = sd_bus_match_signal_async( + m->bus, + NULL, + "org.freedesktop.systemd1", + NULL, + "org.freedesktop.DBus.Properties", + "PropertiesChanged", + match_properties_changed, NULL, m); if (r < 0) - return log_error_errno(r, "Failed to add match for PropertiesChanged: %m"); + return log_error_errno(r, "Failed to request match for PropertiesChanged: %m"); - r = sd_bus_add_match(m->bus, - NULL, - "type='signal'," - "sender='org.freedesktop.systemd1'," - "interface='org.freedesktop.systemd1.Manager'," - "member='Reloading'," - "path='/org/freedesktop/systemd1'", - match_reloading, m); + r = sd_bus_match_signal_async( + m->bus, + NULL, + "org.freedesktop.systemd1", + "/org/freedesktop/systemd1", + "org.freedesktop.systemd1.Manager", + "Reloading", + match_reloading, NULL, m); if (r < 0) - return log_error_errno(r, "Failed to add match for Reloading: %m"); + return log_error_errno(r, "Failed to request match for Reloading: %m"); r = sd_bus_call_method( m->bus, diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c index c435bb9b5a2..615db5afe8e 100644 --- a/src/machine/machinectl.c +++ b/src/machine/machinectl.c @@ -1568,9 +1568,9 @@ static int login_machine(int argc, char *argv[], void *userdata) { "member='MachineRemoved'," "arg0='", machine, "'"); - r = sd_bus_add_match(bus, &slot, match, on_machine_removed, &forward); + r = sd_bus_add_match_async(bus, &slot, match, on_machine_removed, NULL, &forward); if (r < 0) - return log_error_errno(r, "Failed to add machine removal match: %m"); + return log_error_errno(r, "Failed to request machine removal match: %m"); r = sd_bus_call_method( bus, @@ -1643,9 +1643,9 @@ static int shell_machine(int argc, char *argv[], void *userdata) { "member='MachineRemoved'," "arg0='", machine, "'"); - r = sd_bus_add_match(bus, &slot, match, on_machine_removed, &forward); + r = sd_bus_add_match_async(bus, &slot, match, on_machine_removed, NULL, &forward); if (r < 0) - return log_error_errno(r, "Failed to add machine removal match: %m"); + return log_error_errno(r, "Failed to request machine removal match: %m"); r = sd_bus_message_new_method_call( bus, @@ -2087,28 +2087,27 @@ static int transfer_image_common(sd_bus *bus, sd_bus_message *m) { if (r < 0) return log_error_errno(r, "Failed to attach bus to event loop: %m"); - r = sd_bus_add_match( + r = sd_bus_match_signal_async( bus, &slot_job_removed, - "type='signal'," - "sender='org.freedesktop.import1'," - "interface='org.freedesktop.import1.Manager'," - "member='TransferRemoved'," - "path='/org/freedesktop/import1'", - match_transfer_removed, &path); + "org.freedesktop.import1", + "/org/freedesktop/import1", + "org.freedesktop.import1.Manager", + "TransferRemoved", + match_transfer_removed, NULL, &path); if (r < 0) - return log_error_errno(r, "Failed to install match: %m"); + return log_error_errno(r, "Failed to request match: %m"); - r = sd_bus_add_match( + r = sd_bus_match_signal_async( bus, &slot_log_message, - "type='signal'," - "sender='org.freedesktop.import1'," - "interface='org.freedesktop.import1.Transfer'," - "member='LogMessage'", - match_log_message, &path); + "org.freedesktop.import1", + NULL, + "org.freedesktop.import1.Transfer", + "LogMessage", + match_log_message, NULL, &path); if (r < 0) - return log_error_errno(r, "Failed to install match: %m"); + return log_error_errno(r, "Failed to request match: %m"); r = sd_bus_call(bus, m, 0, &error, &reply); if (r < 0) { diff --git a/src/machine/machined.c b/src/machine/machined.c index 9ef121fe662..7cbfc922d74 100644 --- a/src/machine/machined.c +++ b/src/machine/machined.c @@ -216,53 +216,49 @@ static int manager_connect_bus(Manager *m) { if (r < 0) return log_error_errno(r, "Failed to add image enumerator: %m"); - r = sd_bus_add_match(m->bus, - NULL, - "type='signal'," - "sender='org.freedesktop.systemd1'," - "interface='org.freedesktop.systemd1.Manager'," - "member='JobRemoved'," - "path='/org/freedesktop/systemd1'", - match_job_removed, - m); + r = sd_bus_match_signal_async( + m->bus, + NULL, + "org.freedesktop.systemd1", + "/org/freedesktop/systemd1", + "org.freedesktop.systemd1.Manager", + "JobRemoved", + match_job_removed, NULL, m); if (r < 0) return log_error_errno(r, "Failed to add match for JobRemoved: %m"); - r = sd_bus_add_match(m->bus, - NULL, - "type='signal'," - "sender='org.freedesktop.systemd1'," - "interface='org.freedesktop.systemd1.Manager'," - "member='UnitRemoved'," - "path='/org/freedesktop/systemd1'", - match_unit_removed, - m); + r = sd_bus_match_signal_async( + m->bus, + NULL, + "org.freedesktop.systemd1", + "/org/freedesktop/systemd1", + "org.freedesktop.systemd1.Manager", + "UnitRemoved", + match_unit_removed, NULL, m); if (r < 0) - return log_error_errno(r, "Failed to add match for UnitRemoved: %m"); + return log_error_errno(r, "Failed to request match for UnitRemoved: %m"); - r = sd_bus_add_match(m->bus, - NULL, - "type='signal'," - "sender='org.freedesktop.systemd1'," - "interface='org.freedesktop.DBus.Properties'," - "member='PropertiesChanged'," - "arg0='org.freedesktop.systemd1.Unit'", - match_properties_changed, - m); + r = sd_bus_match_signal_async( + m->bus, + NULL, + "org.freedesktop.systemd1", + NULL, + "org.freedesktop.DBus.Properties", + "PropertiesChanged", + match_properties_changed, NULL, m); if (r < 0) - return log_error_errno(r, "Failed to add match for PropertiesChanged: %m"); + return log_error_errno(r, "Failed to request match for PropertiesChanged: %m"); - r = sd_bus_add_match(m->bus, - NULL, - "type='signal'," - "sender='org.freedesktop.systemd1'," - "interface='org.freedesktop.systemd1.Manager'," - "member='Reloading'," - "path='/org/freedesktop/systemd1'", - match_reloading, - m); + r = sd_bus_match_signal_async( + m->bus, + NULL, + "org.freedesktop.systemd1", + "/org/freedesktop/systemd1", + "org.freedesktop.systemd1.Manager", + "Reloading", + match_reloading, NULL, m); if (r < 0) - return log_error_errno(r, "Failed to add match for Reloading: %m"); + return log_error_errno(r, "Failed to request match for Reloading: %m"); r = sd_bus_call_method( m->bus, diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index 836f58c78ca..79d58dd0994 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -152,16 +152,15 @@ int manager_connect_bus(Manager *m) { return 0; } - r = sd_bus_add_match(m->bus, &m->prepare_for_sleep_slot, - "type='signal'," - "sender='org.freedesktop.login1'," - "interface='org.freedesktop.login1.Manager'," - "member='PrepareForSleep'," - "path='/org/freedesktop/login1'", - match_prepare_for_sleep, - m); + r = sd_bus_match_signal_async( + m->bus, &m->prepare_for_sleep_slot, + "org.freedesktop.login1", + "/org/freedesktop/login1", + "org.freedesktop.login1.Manager", + "PrepareForSleep", + match_prepare_for_sleep, NULL, m); if (r < 0) - return log_error_errno(r, "Failed to add match for PrepareForSleep: %m"); + return log_error_errno(r, "Failed to request match for PrepareForSleep: %m"); r = sd_bus_add_object_vtable(m->bus, NULL, "/org/freedesktop/network1", "org.freedesktop.network1.Manager", manager_vtable, m); if (r < 0) diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 383047acc79..f580b46f868 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -3622,14 +3622,16 @@ static int run(int master, * case PID 1 will send us a friendly RequestStop signal, when it is asked to terminate the * scope. Let's hook into that, and cleanly shut down the container, and print a friendly message. */ - r = sd_bus_add_match(bus, NULL, - "type='signal'," - "sender='org.freedesktop.systemd1'," - "interface='org.freedesktop.systemd1.Scope'," - "member='RequestStop'", - on_request_stop, PID_TO_PTR(*pid)); + r = sd_bus_match_signal_async( + bus, + NULL, + "org.freedesktop.systemd1", + NULL, + "org.freedesktop.systemd1.Scope", + "RequestStop", + on_request_stop, NULL, PID_TO_PTR(*pid)); if (r < 0) - return log_error_errno(r, "Failed to install request stop match: %m"); + return log_error_errno(r, "Failed to request RequestStop match: %m"); } if (arg_register) { diff --git a/src/resolve/resolved-bus.c b/src/resolve/resolved-bus.c index 5a93f894672..ff629ea41b9 100644 --- a/src/resolve/resolved-bus.c +++ b/src/resolve/resolved-bus.c @@ -1929,16 +1929,18 @@ int manager_connect_bus(Manager *m) { if (r < 0) return log_error_errno(r, "Failed to attach bus to event loop: %m"); - r = sd_bus_add_match(m->bus, &m->prepare_for_sleep_slot, - "type='signal'," - "sender='org.freedesktop.login1'," - "interface='org.freedesktop.login1.Manager'," - "member='PrepareForSleep'," - "path='/org/freedesktop/login1'", - match_prepare_for_sleep, - m); + r = sd_bus_match_signal_async( + m->bus, + &m->prepare_for_sleep_slot, + "org.freedesktop.login1", + "/org/freedesktop/login1", + "org.freedesktop.login1.Manager", + "PrepareForSleep", + match_prepare_for_sleep, + NULL, + m); if (r < 0) - log_error_errno(r, "Failed to add match for PrepareForSleep: %m"); + log_error_errno(r, "Failed to request match for PrepareForSleep: %m"); return 0; } diff --git a/src/run/run.c b/src/run/run.c index 0ff8db6b3ec..a30501169c5 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -1059,7 +1059,6 @@ static int start_transient_service( .inactive_enter_usec = USEC_INFINITY, }; _cleanup_free_ char *path = NULL; - const char *mt; c.bus = sd_bus_ref(bus); @@ -1089,18 +1088,20 @@ static int start_transient_service( if (!path) return log_oom(); - mt = strjoina("type='signal'," - "sender='org.freedesktop.systemd1'," - "path='", path, "'," - "interface='org.freedesktop.DBus.Properties'," - "member='PropertiesChanged'"); - r = sd_bus_add_match(bus, &c.match, mt, on_properties_changed, &c); + r = sd_bus_match_signal_async( + bus, + &c.match, + "org.freedesktop.systemd1", + path, + "org.freedesktop.DBus.Properties", + "PropertiesChanged", + on_properties_changed, NULL, &c); if (r < 0) - return log_error_errno(r, "Failed to add properties changed signal."); + return log_error_errno(r, "Failed to request properties changed signal match: %m"); r = sd_bus_attach_event(bus, c.event, SD_EVENT_PRIORITY_NORMAL); if (r < 0) - return log_error_errno(r, "Failed to attach bus to event loop."); + return log_error_errno(r, "Failed to attach bus to event loop: %m"); r = run_context_update(&c, path); if (r < 0) diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index a46fa62c2cc..94ee71697e7 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -1730,31 +1730,25 @@ int bus_wait_for_jobs_new(sd_bus *bus, BusWaitForJobs **ret) { /* When we are a bus client we match by sender. Direct * connections OTOH have no initialized sender field, and * hence we ignore the sender then */ - r = sd_bus_add_match( + r = sd_bus_match_signal_async( bus, &d->slot_job_removed, - bus->bus_client ? - "type='signal'," - "sender='org.freedesktop.systemd1'," - "interface='org.freedesktop.systemd1.Manager'," - "member='JobRemoved'," - "path='/org/freedesktop/systemd1'" : - "type='signal'," - "interface='org.freedesktop.systemd1.Manager'," - "member='JobRemoved'," - "path='/org/freedesktop/systemd1'", - match_job_removed, d); + bus->bus_client ? "org.freedesktop.systemd1" : NULL, + "/org/freedesktop/systemd1", + "org.freedesktop.systemd1.Manager", + "JobRemoved", + match_job_removed, NULL, d); if (r < 0) return r; - r = sd_bus_add_match( + r = sd_bus_match_signal_async( bus, &d->slot_disconnected, - "type='signal'," - "sender='org.freedesktop.DBus.Local'," - "interface='org.freedesktop.DBus.Local'," - "member='Disconnected'", - match_disconnected, d); + "org.freedesktop.DBus.Local", + NULL, + "org.freedesktop.DBus.Local", + "Disconnected", + match_disconnected, NULL, d); if (r < 0) return r; diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index 50731fa5ff0..b9e25a0d4e7 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -68,7 +68,7 @@ static int name_owner_change_callback(sd_bus_message *m, void *userdata, sd_bus_ } int bus_async_unregister_and_exit(sd_event *e, sd_bus *bus, const char *name) { - _cleanup_free_ char *match = NULL; + const char *match; const char *unique; int r; @@ -85,23 +85,21 @@ int bus_async_unregister_and_exit(sd_event *e, sd_bus *bus, const char *name) { if (r < 0) return r; - r = asprintf(&match, - "sender='org.freedesktop.DBus'," - "type='signal'," - "interface='org.freedesktop.DBus'," - "member='NameOwnerChanged'," - "path='/org/freedesktop/DBus'," - "arg0='%s'," - "arg1='%s'," - "arg2=''", name, unique); - if (r < 0) - return -ENOMEM; + match = strjoina( + "sender='org.freedesktop.DBus'," + "type='signal'," + "interface='org.freedesktop.DBus'," + "member='NameOwnerChanged'," + "path='/org/freedesktop/DBus'," + "arg0='", name, "',", + "arg1='", unique, "',", + "arg2=''"); - r = sd_bus_add_match(bus, NULL, match, name_owner_change_callback, e); + r = sd_bus_add_match_async(bus, NULL, match, name_owner_change_callback, NULL, e); if (r < 0) return r; - r = sd_bus_release_name(bus, name); + r = sd_bus_release_name_async(bus, NULL, name, NULL, NULL); if (r < 0) return r; diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 1376497f8dc..5f8ff17a9b4 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -2905,7 +2905,6 @@ static int start_unit_one( if (wait_context) { _cleanup_free_ char *unit_path = NULL; - const char* mt; log_debug("Watching for property changes of %s", name); r = sd_bus_call_method( @@ -2928,13 +2927,15 @@ static int start_unit_one( if (r < 0) return log_error_errno(r, "Failed to add unit path %s to set: %m", unit_path); - mt = strjoina("type='signal'," - "interface='org.freedesktop.DBus.Properties'," - "path='", unit_path, "'," - "member='PropertiesChanged'"); - r = sd_bus_add_match(bus, &wait_context->match, mt, on_properties_changed, wait_context); + r = sd_bus_match_signal_async(bus, + &wait_context->match, + NULL, + unit_path, + "org.freedesktop.DBus.Properties", + "PropertiesChanged", + on_properties_changed, NULL, wait_context); if (r < 0) - return log_error_errno(r, "Failed to add match for PropertiesChanged signal: %m"); + return log_error_errno(r, "Failed to request match for PropertiesChanged signal: %m"); } log_debug("%s manager for %s on %s, %s", From 8c095a070b4c44147bef319c55e70a9f38277cfb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 19 Dec 2017 15:39:44 +0100 Subject: [PATCH 27/46] man: let's drop references to /var/run in public man pages /var/run is a legacy compatibility feature, let's avoid mentioning it. --- man/runlevel.xml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/man/runlevel.xml b/man/runlevel.xml index b3d90d8ff4c..596307af9fc 100644 --- a/man/runlevel.xml +++ b/man/runlevel.xml @@ -173,10 +173,9 @@ - /var/run/utmp + /run/utmp - The utmp database runlevel - reads the previous and current runlevel + The utmp database runlevel reads the previous and current runlevel from. From 15ca0a42c7db49146a7169dbcb0683f0836cf8c2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 19 Dec 2017 15:41:58 +0100 Subject: [PATCH 28/46] sd-bus: drop references to legacy /var/run D-Bus socket Let's directly reference /run instead, so that we can work without /var being around, or with /var/run being incorrectly set up. Note that we keep the old socket path in place when referencing the system bus of containers, as they might be foreign operating systems, that still don't have adopted /run, and where it makes sense to use the standardized name instead. On local systems, we insist on /run being set up properly however, hence this limitation does not apply. Also, get rid of the UNIX_SYSTEM_BUS_ADDRESS and UNIX_USER_BUS_ADDRESS_FMT defines. They had a purpose when we still did kdbus, as we then had to support two different backends. But since that's gone, we don't need this indirection anymore, hence settle on a one define only. --- src/basic/def.h | 7 ++++--- src/libsystemd/sd-bus/sd-bus.c | 3 ++- src/login/pam_systemd.c | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/basic/def.h b/src/basic/def.h index 77ab735aedb..43e7e170084 100644 --- a/src/basic/def.h +++ b/src/basic/def.h @@ -53,9 +53,10 @@ "/usr/lib/kbd/keymaps/\0" #endif -#define UNIX_SYSTEM_BUS_ADDRESS "unix:path=/var/run/dbus/system_bus_socket" -#define DEFAULT_SYSTEM_BUS_ADDRESS UNIX_SYSTEM_BUS_ADDRESS -#define UNIX_USER_BUS_ADDRESS_FMT "unix:path=%s/bus" +/* Note that we use the new /run prefix here (instead of /var/run) since we require them to be aliases and that way we + * become independent of /var being mounted */ +#define DEFAULT_SYSTEM_BUS_ADDRESS "unix:path=/run/dbus/system_bus_socket" +#define DEFAULT_USER_BUS_ADDRESS_FMT "unix:path=%s/bus" #define PLYMOUTH_SOCKET { \ .un.sun_family = AF_UNIX, \ diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 2f570f1aab7..0266fbaf553 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -832,6 +832,7 @@ static int parse_container_unix_address(sd_bus *b, const char **p, char **guid) b->nspid = 0; b->sockaddr.un.sun_family = AF_UNIX; + /* Note that we use the old /var/run prefix here, to increase compatibility with really old containers */ strncpy(b->sockaddr.un.sun_path, "/var/run/dbus/system_bus_socket", sizeof(b->sockaddr.un.sun_path)); b->sockaddr_size = SOCKADDR_UN_LEN(b->sockaddr.un); b->is_local = false; @@ -1157,7 +1158,7 @@ int bus_set_address_user(sd_bus *b) { if (!ee) return -ENOMEM; - if (asprintf(&s, UNIX_USER_BUS_ADDRESS_FMT, ee) < 0) + if (asprintf(&s, DEFAULT_USER_BUS_ADDRESS_FMT, ee) < 0) return -ENOMEM; b->address = s; diff --git a/src/login/pam_systemd.c b/src/login/pam_systemd.c index 246bbddeee1..1c3ba33e238 100644 --- a/src/login/pam_systemd.c +++ b/src/login/pam_systemd.c @@ -197,7 +197,7 @@ static int export_legacy_dbus_address( return PAM_SUCCESS; s = mfree(s); - if (asprintf(&s, UNIX_USER_BUS_ADDRESS_FMT, runtime) < 0) + if (asprintf(&s, DEFAULT_USER_BUS_ADDRESS_FMT, runtime) < 0) goto error; r = pam_misc_setenv(handle, "DBUS_SESSION_BUS_ADDRESS", s, 0); From f9d5fcceaf7f0ec56e328f7c8efc96c523fc87c1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 19 Dec 2017 15:45:30 +0100 Subject: [PATCH 29/46] sd-bus: modernize how we generate the match string in sd-bus-track strjoina() FTW! --- src/libsystemd/sd-bus/bus-track.c | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-track.c b/src/libsystemd/sd-bus/bus-track.c index 919cebda0b3..0dd06e9b99a 100644 --- a/src/libsystemd/sd-bus/bus-track.c +++ b/src/libsystemd/sd-bus/bus-track.c @@ -48,25 +48,13 @@ struct sd_bus_track { LIST_FIELDS(sd_bus_track, tracks); }; -#define MATCH_PREFIX \ - "type='signal'," \ - "sender='org.freedesktop.DBus'," \ - "path='/org/freedesktop/DBus'," \ - "interface='org.freedesktop.DBus'," \ - "member='NameOwnerChanged'," \ - "arg0='" - -#define MATCH_SUFFIX \ - "'" - -#define MATCH_FOR_NAME(name) \ - ({ \ - char *_x; \ - size_t _l = strlen(name); \ - _x = alloca(STRLEN(MATCH_PREFIX)+_l+STRLEN(MATCH_SUFFIX)+1); \ - strcpy(stpcpy(stpcpy(_x, MATCH_PREFIX), name), MATCH_SUFFIX); \ - _x; \ - }) +#define MATCH_FOR_NAME(name) \ + strjoina("type='signal'," \ + "sender='org.freedesktop.DBus'," \ + "path='/org/freedesktop/DBus'," \ + "interface='org.freedesktop.DBus'," \ + "member='NameOwnerChanged'," \ + "arg0='", name, "'") static struct track_item* track_item_free(struct track_item *i) { From bdbc866914e54189d6f9924349005fa8fa6bf71b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 19 Dec 2017 15:47:09 +0100 Subject: [PATCH 30/46] sd-bus: add new sd_bus_is_ready() API This new call is much light sd_bus_is_open(), but returns true only if the connection is fully set up, i.e. after we finished with the authentication and Hello() phase. This API is useful for clients in particular when using the "watch_bind" feature, as that way it can be determined in advance whether it makes sense to sync on some operation. --- src/libsystemd/libsystemd.sym | 1 + src/libsystemd/sd-bus/sd-bus.c | 7 +++++++ src/systemd/sd-bus.h | 1 + 3 files changed, 9 insertions(+) diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index 05c3b99bd6b..af4226c73af 100644 --- a/src/libsystemd/libsystemd.sym +++ b/src/libsystemd/libsystemd.sym @@ -540,4 +540,5 @@ global: sd_bus_add_match_async; sd_bus_match_signal; sd_bus_match_signal_async; + sd_bus_is_ready; } LIBSYSTEMD_236; diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 0266fbaf553..7967aac7c67 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -1396,6 +1396,13 @@ _public_ int sd_bus_is_open(sd_bus *bus) { return BUS_IS_OPEN(bus->state); } +_public_ int sd_bus_is_ready(sd_bus *bus) { + assert_return(bus, -EINVAL); + assert_return(!bus_pid_changed(bus), -ECHILD); + + return bus->state == BUS_RUNNING; +} + _public_ int sd_bus_can_send(sd_bus *bus, char type) { int r; diff --git a/src/systemd/sd-bus.h b/src/systemd/sd-bus.h index 9a6581348c1..76bd5eff95e 100644 --- a/src/systemd/sd-bus.h +++ b/src/systemd/sd-bus.h @@ -165,6 +165,7 @@ sd_bus *sd_bus_flush_close_unref(sd_bus *bus); void sd_bus_default_flush_close(void); int sd_bus_is_open(sd_bus *bus); +int sd_bus_is_ready(sd_bus *bus); int sd_bus_get_bus_id(sd_bus *bus, sd_id128_t *id); int sd_bus_get_scope(sd_bus *bus, const char **scope); From b38cc8d5631253738d5474f39b2026eb5524f772 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 19 Dec 2017 15:50:05 +0100 Subject: [PATCH 31/46] sd-bus: add new sd_bus_set_connected_signal() API With this new API sd-bus can synthesize a local "Connected" signal when the connection is fully established. It mirrors the local "Disconnected" signal that is already generated when the connection is terminated. This is useful to be notified when connection setup is done, in order to start method calls then, in particular when using "slow" connection methods (for example slow TCP, or most importantly the "watch_bind" inotify logic). Note that one could also use hook into the initial NameAcquired signal received from the bus broker, but that scheme works only if we actually connect to a bus. The benefit of "Connected" OTOH is that it works with any kind of connection. Ideally, we'd just generate this message unconditionally, but in order not to break clients that do not expect this message it is opt-in. --- src/libsystemd/libsystemd.sym | 2 + src/libsystemd/sd-bus/bus-internal.h | 1 + src/libsystemd/sd-bus/sd-bus.c | 73 +++++++++++++++++++++++++++- 3 files changed, 75 insertions(+), 1 deletion(-) diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index af4226c73af..27f3219ee23 100644 --- a/src/libsystemd/libsystemd.sym +++ b/src/libsystemd/libsystemd.sym @@ -541,4 +541,6 @@ global: sd_bus_match_signal; sd_bus_match_signal_async; sd_bus_is_ready; + sd_bus_set_connected_signal; + sd_bus_get_connected_signal; } LIBSYSTEMD_236; diff --git a/src/libsystemd/sd-bus/bus-internal.h b/src/libsystemd/sd-bus/bus-internal.h index 29366c4c568..abda27fd561 100644 --- a/src/libsystemd/sd-bus/bus-internal.h +++ b/src/libsystemd/sd-bus/bus-internal.h @@ -219,6 +219,7 @@ struct sd_bus { bool is_monitor:1; bool accept_fd:1; bool attach_timestamp:1; + bool connected_signal:1; int use_memfd; diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 7967aac7c67..74d54f785fe 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -392,6 +392,66 @@ _public_ int sd_bus_get_watch_bind(sd_bus *bus) { return bus->watch_bind; } +_public_ int sd_bus_set_connected_signal(sd_bus *bus, int b) { + assert_return(bus, -EINVAL); + assert_return(bus->state == BUS_UNSET, -EPERM); + assert_return(!bus_pid_changed(bus), -ECHILD); + + bus->connected_signal = b; + return 0; +} + +_public_ int sd_bus_get_connected_signal(sd_bus *bus) { + assert_return(bus, -EINVAL); + assert_return(!bus_pid_changed(bus), -ECHILD); + + return bus->connected_signal; +} + +static int synthesize_connected_signal(sd_bus *bus) { + _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL; + int r; + + assert(bus); + + /* If enabled, synthesizes a local "Connected" signal mirroring the local "Disconnected" signal. This is called + * whenever we fully established a connection, i.e. after the authorization phase, and after receiving the + * Hello() reply. Or in other words, whenver we enter BUS_RUNNING state. + * + * This is useful so that clients can start doing stuff whenver the connection is fully established in a way + * that works independently from whether we connected to a full bus or just a direct connection. */ + + if (!bus->connected_signal) + return 0; + + r = sd_bus_message_new_signal( + bus, + &m, + "/org/freedesktop/DBus/Local", + "org.freedesktop.DBus.Local", + "Connected"); + if (r < 0) + return r; + + bus_message_set_sender_local(bus, m); + + r = bus_seal_synthetic_message(bus, m); + if (r < 0) + return r; + + r = bus_rqueue_make_room(bus); + if (r < 0) + return r; + + /* Insert at the very front */ + memmove(bus->rqueue + 1, bus->rqueue, sizeof(sd_bus_message*) * bus->rqueue_size); + bus->rqueue[0] = m; + m = NULL; + bus->rqueue_size++; + + return 0; +} + static int hello_callback(sd_bus_message *reply, void *userdata, sd_bus_error *error) { const char *s; sd_bus *bus; @@ -417,9 +477,14 @@ static int hello_callback(sd_bus_message *reply, void *userdata, sd_bus_error *e if (!bus->unique_name) return -ENOMEM; - if (bus->state == BUS_HELLO) + if (bus->state == BUS_HELLO) { bus->state = BUS_RUNNING; + r = synthesize_connected_signal(bus); + if (r < 0) + return r; + } + return 1; } @@ -449,6 +514,7 @@ int bus_start_running(sd_bus *bus) { struct reply_callback *c; Iterator i; usec_t n; + int r; assert(bus); assert(bus->state < BUS_HELLO); @@ -471,6 +537,11 @@ int bus_start_running(sd_bus *bus) { } bus->state = BUS_RUNNING; + + r = synthesize_connected_signal(bus); + if (r < 0) + return r; + return 1; } From d7afd945b5aad5b262a3de97614f486d63d94612 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 19 Dec 2017 15:54:30 +0100 Subject: [PATCH 32/46] networkd,resolved: make use of watch_bind feature to connect to the bus The changes both networkd and resolved to make use of the watch_bind feature of sd-bus to connect to the system bus. This way, both daemons can be started during early boot, and automatically and instantly connect to the system bus as it becomes available. This replaces prior code that used a time-based retry logic to connect to the bus. --- src/network/networkd-manager.c | 95 ++++++++++++++++------------------ src/network/networkd-manager.h | 1 + src/resolve/resolved-bus.c | 31 ++--------- src/shared/bus-util.c | 51 ++++++++++++++++++ src/shared/bus-util.h | 2 + src/systemd/sd-bus.h | 2 + 6 files changed, 106 insertions(+), 76 deletions(-) diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index 79d58dd0994..4733dd5177b 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -82,19 +82,6 @@ static int setup_default_address_pool(Manager *m) { return 0; } -static int on_bus_retry(sd_event_source *s, usec_t usec, void *userdata) { - Manager *m = userdata; - - assert(s); - assert(m); - - m->bus_retry_event_source = sd_event_source_unref(m->bus_retry_event_source); - - manager_connect_bus(m); - - return 0; -} - static int manager_reset_all(Manager *m) { Link *link; Iterator i; @@ -116,6 +103,7 @@ static int match_prepare_for_sleep(sd_bus_message *message, void *userdata, sd_b int b, r; assert(message); + assert(m); r = sd_bus_message_read(message, "b", &b); if (r < 0) { @@ -133,34 +121,32 @@ static int match_prepare_for_sleep(sd_bus_message *message, void *userdata, sd_b return 0; } +static int on_connected(sd_bus_message *message, void *userdata, sd_bus_error *ret_error) { + Manager *m = userdata; + + assert(message); + assert(m); + + /* Did we get a timezone or transient hostname from DHCP while D-Bus wasn't up yet? */ + if (m->dynamic_hostname) + (void) manager_set_hostname(m, m->dynamic_hostname); + if (m->dynamic_timezone) + (void) manager_set_timezone(m, m->dynamic_timezone); + + return 0; +} + int manager_connect_bus(Manager *m) { int r; assert(m); - r = sd_bus_default_system(&m->bus); - if (r < 0) { - /* We failed to connect? Yuck, we must be in early - * boot. Let's try in 5s again. */ - - log_debug_errno(r, "Failed to connect to bus, trying again in 5s: %m"); - - r = sd_event_add_time(m->event, &m->bus_retry_event_source, CLOCK_MONOTONIC, now(CLOCK_MONOTONIC) + 5*USEC_PER_SEC, 0, on_bus_retry, m); - if (r < 0) - return log_error_errno(r, "Failed to install bus reconnect time event: %m"); - + if (m->bus) return 0; - } - r = sd_bus_match_signal_async( - m->bus, &m->prepare_for_sleep_slot, - "org.freedesktop.login1", - "/org/freedesktop/login1", - "org.freedesktop.login1.Manager", - "PrepareForSleep", - match_prepare_for_sleep, NULL, m); + r = bus_open_system_watch_bind(&m->bus); if (r < 0) - return log_error_errno(r, "Failed to request match for PrepareForSleep: %m"); + return log_error_errno(r, "Failed to connect to bus: %m"); r = sd_bus_add_object_vtable(m->bus, NULL, "/org/freedesktop/network1", "org.freedesktop.network1.Manager", manager_vtable, m); if (r < 0) @@ -190,17 +176,27 @@ int manager_connect_bus(Manager *m) { if (r < 0) return log_error_errno(r, "Failed to attach bus to event loop: %m"); - /* Did we get a timezone or transient hostname from DHCP while D-Bus wasn't up yet? */ - if (m->dynamic_hostname) { - r = manager_set_hostname(m, m->dynamic_hostname); - if (r < 0) - return r; - } - if (m->dynamic_timezone) { - r = manager_set_timezone(m, m->dynamic_timezone); - if (r < 0) - return r; - } + r = sd_bus_match_signal_async( + m->bus, + &m->connected_slot, + "org.freedesktop.DBus.Local", + NULL, + "org.freedesktop.DBus.Local", + "Connected", + on_connected, NULL, m); + if (r < 0) + return log_error_errno(r, "Failed to request match on Connected signal: %m"); + + r = sd_bus_match_signal_async( + m->bus, + &m->prepare_for_sleep_slot, + "org.freedesktop.login1", + "/org/freedesktop/login1", + "org.freedesktop.login1.Manager", + "PrepareForSleep", + match_prepare_for_sleep, NULL, m); + if (r < 0) + log_warning_errno(r, "Failed to request match for PrepareForSleep, ignoring: %m"); return 0; } @@ -1320,6 +1316,7 @@ void manager_free(Manager *m) { sd_bus_unref(m->bus); sd_bus_slot_unref(m->prepare_for_sleep_slot); + sd_bus_slot_unref(m->connected_slot); sd_event_source_unref(m->bus_retry_event_source); free(m->dynamic_timezone); @@ -1594,12 +1591,12 @@ int manager_set_hostname(Manager *m, const char *hostname) { int r; log_debug("Setting transient hostname: '%s'", strna(hostname)); + if (free_and_strdup(&m->dynamic_hostname, hostname) < 0) return log_oom(); - if (!m->bus) { - /* TODO: replace by assert when we can rely on kdbus */ - log_info("Not connected to system bus, ignoring transient hostname."); + if (!m->bus || sd_bus_is_ready(m->bus) <= 0) { + log_info("Not connected to system bus, not setting hostname."); return 0; } @@ -1646,8 +1643,8 @@ int manager_set_timezone(Manager *m, const char *tz) { if (free_and_strdup(&m->dynamic_timezone, tz) < 0) return log_oom(); - if (!m->bus) { - log_info("Not connected to system bus, ignoring timezone."); + if (!m->bus || sd_bus_is_ready(m->bus) <= 0) { + log_info("Not connected to system bus, not setting hostname."); return 0; } diff --git a/src/network/networkd-manager.h b/src/network/networkd-manager.h index 186cb418911..1f7f306af93 100644 --- a/src/network/networkd-manager.h +++ b/src/network/networkd-manager.h @@ -43,6 +43,7 @@ struct Manager { sd_event_source *bus_retry_event_source; sd_bus *bus; sd_bus_slot *prepare_for_sleep_slot; + sd_bus_slot *connected_slot; struct udev *udev; struct udev_monitor *udev_monitor; sd_event_source *udev_event_source; diff --git a/src/resolve/resolved-bus.c b/src/resolve/resolved-bus.c index ff629ea41b9..ffd7c4824e3 100644 --- a/src/resolve/resolved-bus.c +++ b/src/resolve/resolved-bus.c @@ -1844,18 +1844,6 @@ static const sd_bus_vtable resolve_vtable[] = { SD_BUS_VTABLE_END, }; -static int on_bus_retry(sd_event_source *s, usec_t usec, void *userdata) { - Manager *m = userdata; - - assert(s); - assert(m); - - m->bus_retry_event_source = sd_event_source_unref(m->bus_retry_event_source); - - manager_connect_bus(m); - return 0; -} - static int match_prepare_for_sleep(sd_bus_message *message, void *userdata, sd_bus_error *ret_error) { Manager *m = userdata; int b, r; @@ -1886,20 +1874,9 @@ int manager_connect_bus(Manager *m) { if (m->bus) return 0; - r = sd_bus_default_system(&m->bus); - if (r < 0) { - /* We failed to connect? Yuck, we must be in early - * boot. Let's try in 5s again. */ - - log_debug_errno(r, "Failed to connect to bus, trying again in 5s: %m"); - - r = sd_event_add_time(m->event, &m->bus_retry_event_source, CLOCK_MONOTONIC, now(CLOCK_MONOTONIC) + 5*USEC_PER_SEC, 0, on_bus_retry, m); - if (r < 0) - return log_error_errno(r, "Failed to install bus reconnect time event: %m"); - - (void) sd_event_source_set_description(m->bus_retry_event_source, "bus-retry"); - return 0; - } + r = bus_open_system_watch_bind(&m->bus); + if (r < 0) + return log_error_errno(r, "Failed to connect to system bus: %m"); r = sd_bus_add_object_vtable(m->bus, NULL, "/org/freedesktop/resolve1", "org.freedesktop.resolve1.Manager", resolve_vtable, m); if (r < 0) @@ -1940,7 +1917,7 @@ int manager_connect_bus(Manager *m) { NULL, m); if (r < 0) - log_error_errno(r, "Failed to request match for PrepareForSleep: %m"); + log_warning_errno(r, "Failed to request match for PrepareForSleep, ignoring: %m"); return 0; } diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index b9e25a0d4e7..b6677e27f60 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -1598,3 +1598,54 @@ int bus_track_add_name_many(sd_bus_track *t, char **l) { return r; } + +int bus_open_system_watch_bind(sd_bus **ret) { + _cleanup_(sd_bus_unrefp) sd_bus *bus = NULL; + const char *e; + int r; + + assert(ret); + + /* Match like sd_bus_open_system(), but with the "watch_bind" feature and the Connected() signal turned on. */ + + r = sd_bus_new(&bus); + if (r < 0) + return r; + + e = secure_getenv("DBUS_SYSTEM_BUS_ADDRESS"); + if (!e) + e = DEFAULT_SYSTEM_BUS_ADDRESS; + + r = sd_bus_set_address(bus, e); + if (r < 0) + return r; + + r = sd_bus_set_bus_client(bus, true); + if (r < 0) + return r; + + r = sd_bus_set_trusted(bus, true); + if (r < 0) + return r; + + r = sd_bus_negotiate_creds(bus, true, SD_BUS_CREDS_UID|SD_BUS_CREDS_EUID|SD_BUS_CREDS_EFFECTIVE_CAPS); + if (r < 0) + return r; + + r = sd_bus_set_watch_bind(bus, true); + if (r < 0) + return r; + + r = sd_bus_set_connected_signal(bus, true); + if (r < 0) + return r; + + r = sd_bus_start(bus); + if (r < 0) + return r; + + *ret = bus; + bus = NULL; + + return 0; +} diff --git a/src/shared/bus-util.h b/src/shared/bus-util.h index a9f4969d7da..cbd22a6cd6b 100644 --- a/src/shared/bus-util.h +++ b/src/shared/bus-util.h @@ -162,3 +162,5 @@ int bus_path_decode_unique(const char *path, const char *prefix, char **ret_send int bus_property_get_rlimit(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, sd_bus_error *error); int bus_track_add_name_many(sd_bus_track *t, char **l); + +int bus_open_system_watch_bind(sd_bus **ret); diff --git a/src/systemd/sd-bus.h b/src/systemd/sd-bus.h index 76bd5eff95e..df21b3f30e6 100644 --- a/src/systemd/sd-bus.h +++ b/src/systemd/sd-bus.h @@ -152,6 +152,8 @@ int sd_bus_set_exit_on_disconnect(sd_bus *bus, int b); int sd_bus_get_exit_on_disconnect(sd_bus *bus); int sd_bus_set_watch_bind(sd_bus *bus, int b); int sd_bus_get_watch_bind(sd_bus *bus); +int sd_bus_set_connected_signal(sd_bus *bus, int b); +int sd_bus_get_connected_signal(sd_bus *bus); int sd_bus_start(sd_bus *bus); From 3e0e196efdad7cc0b11f859eeb6d8747dbfd0ef6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Dec 2017 12:50:43 +0100 Subject: [PATCH 33/46] sd-bus: log about bus state changes Let's unify all state changes in a new helper function, from which we can then debug log all state changes --- src/libsystemd/sd-bus/bus-internal.h | 5 +++- src/libsystemd/sd-bus/bus-socket.c | 6 ++--- src/libsystemd/sd-bus/sd-bus.c | 35 +++++++++++++++++++++++----- 3 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-internal.h b/src/libsystemd/sd-bus/bus-internal.h index abda27fd561..53f3194beff 100644 --- a/src/libsystemd/sd-bus/bus-internal.h +++ b/src/libsystemd/sd-bus/bus-internal.h @@ -166,7 +166,8 @@ enum bus_state { BUS_HELLO, /* we are waiting for the Hello() response */ BUS_RUNNING, BUS_CLOSING, - BUS_CLOSED + BUS_CLOSED, + _BUS_STATE_MAX, }; static inline bool BUS_IS_OPEN(enum bus_state state) { @@ -411,3 +412,5 @@ int bus_maybe_reply_error(sd_bus_message *m, int r, sd_bus_error *error); } while (false) void bus_enter_closing(sd_bus *bus); + +void bus_set_state(sd_bus *bus, enum bus_state state); diff --git a/src/libsystemd/sd-bus/bus-socket.c b/src/libsystemd/sd-bus/bus-socket.c index e2991bc8b29..61539313bce 100644 --- a/src/libsystemd/sd-bus/bus-socket.c +++ b/src/libsystemd/sd-bus/bus-socket.c @@ -675,7 +675,7 @@ int bus_socket_start_auth(sd_bus *b) { bus_get_peercred(b); - b->state = BUS_AUTHENTICATING; + bus_set_state(b, BUS_AUTHENTICATING); b->auth_timeout = now(CLOCK_MONOTONIC) + BUS_AUTH_TIMEOUT; if (sd_is_socket(b->input_fd, AF_UNIX, 0, 0) <= 0) @@ -894,7 +894,7 @@ int bus_socket_connect(sd_bus *b) { /* Note that very likely we are already in BUS_OPENING state here, as we enter it when * we start parsing the address string. The only reason we set the state explicitly * here, is to undo BUS_WATCH_BIND, in case we did the inotify magic. */ - b->state = BUS_OPENING; + bus_set_state(b, BUS_OPENING); return 1; } @@ -910,7 +910,7 @@ int bus_socket_connect(sd_bus *b) { if (inotify_done) { /* inotify set up already, don't do it again, just return now, and remember * that we are waiting for inotify events now. */ - b->state = BUS_WATCH_BIND; + bus_set_state(b, BUS_WATCH_BIND); return 1; } diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 74d54f785fe..671d2e45fbb 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -452,6 +452,29 @@ static int synthesize_connected_signal(sd_bus *bus) { return 0; } +void bus_set_state(sd_bus *bus, enum bus_state state) { + + static const char * const table[_BUS_STATE_MAX] = { + [BUS_UNSET] = "UNSET", + [BUS_WATCH_BIND] = "WATCH_BIND", + [BUS_OPENING] = "OPENING", + [BUS_AUTHENTICATING] = "AUTHENTICATING", + [BUS_HELLO] = "HELLO", + [BUS_RUNNING] = "RUNNING", + [BUS_CLOSING] = "CLOSING", + [BUS_CLOSED] = "CLOSED", + }; + + assert(bus); + assert(state < _BUS_STATE_MAX); + + if (state == bus->state) + return; + + log_debug("Bus %s: changing state %s → %s", strna(bus->description), table[bus->state], table[state]); + bus->state = state; +} + static int hello_callback(sd_bus_message *reply, void *userdata, sd_bus_error *error) { const char *s; sd_bus *bus; @@ -478,7 +501,7 @@ static int hello_callback(sd_bus_message *reply, void *userdata, sd_bus_error *e return -ENOMEM; if (bus->state == BUS_HELLO) { - bus->state = BUS_RUNNING; + bus_set_state(bus, BUS_RUNNING); r = synthesize_connected_signal(bus); if (r < 0) @@ -532,11 +555,11 @@ int bus_start_running(sd_bus *bus) { } if (bus->bus_client) { - bus->state = BUS_HELLO; + bus_set_state(bus, BUS_HELLO); return 1; } - bus->state = BUS_RUNNING; + bus_set_state(bus, BUS_RUNNING); r = synthesize_connected_signal(bus); if (r < 0) @@ -1090,7 +1113,7 @@ _public_ int sd_bus_start(sd_bus *bus) { assert_return(bus->state == BUS_UNSET, -EPERM); assert_return(!bus_pid_changed(bus), -ECHILD); - bus->state = BUS_OPENING; + bus_set_state(bus, BUS_OPENING); if (bus->is_server && bus->bus_client) return -EINVAL; @@ -1403,7 +1426,7 @@ _public_ void sd_bus_close(sd_bus *bus) { if (bus_pid_changed(bus)) return; - bus->state = BUS_CLOSED; + bus_set_state(bus, BUS_CLOSED); sd_bus_detach_event(bus); @@ -1432,7 +1455,7 @@ void bus_enter_closing(sd_bus *bus) { if (!IN_SET(bus->state, BUS_WATCH_BIND, BUS_OPENING, BUS_AUTHENTICATING, BUS_HELLO, BUS_RUNNING)) return; - bus->state = BUS_CLOSING; + bus_set_state(bus, BUS_CLOSING); } _public_ sd_bus *sd_bus_ref(sd_bus *bus) { From dbc526f0bca54aecc2975b9421727cf957156a9e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Dec 2017 19:45:09 +0100 Subject: [PATCH 34/46] sd-bus: accept NULL callbacks in sd_bus_call_async() This way sd_bus_call_method_async() (which is just a wrapper around sd_bus_call_async()) can be used to put method calls together that expect no reply. --- src/libsystemd/sd-bus/sd-bus.c | 41 +++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 671d2e45fbb..556c2173427 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -1871,8 +1871,7 @@ _public_ int sd_bus_call_async( assert_return(m, -EINVAL); assert_return(m->header->type == SD_BUS_MESSAGE_METHOD_CALL, -EINVAL); - assert_return(!(m->header->flags & BUS_MESSAGE_NO_REPLY_EXPECTED), -EINVAL); - assert_return(callback, -EINVAL); + assert_return(!m->sealed || (!!callback == !(m->header->flags & BUS_MESSAGE_NO_REPLY_EXPECTED)), -EINVAL); if (!bus) bus = m->bus; @@ -1882,6 +1881,10 @@ _public_ int sd_bus_call_async( if (!BUS_IS_OPEN(bus->state)) return -ENOTCONN; + /* If no callback is specified and there's no interest in a slot, then there's no reason to ask for a reply */ + if (!callback && !slot && !m->sealed) + m->header->flags |= BUS_MESSAGE_NO_REPLY_EXPECTED; + r = ordered_hashmap_ensure_allocated(&bus->reply_callbacks, &uint64_hash_ops); if (r < 0) return r; @@ -1898,29 +1901,31 @@ _public_ int sd_bus_call_async( if (r < 0) return r; - s = bus_slot_allocate(bus, !slot, BUS_REPLY_CALLBACK, sizeof(struct reply_callback), userdata); - if (!s) - return -ENOMEM; + if (slot || callback) { + s = bus_slot_allocate(bus, !slot, BUS_REPLY_CALLBACK, sizeof(struct reply_callback), userdata); + if (!s) + return -ENOMEM; - s->reply_callback.callback = callback; + s->reply_callback.callback = callback; - s->reply_callback.cookie = BUS_MESSAGE_COOKIE(m); - r = ordered_hashmap_put(bus->reply_callbacks, &s->reply_callback.cookie, &s->reply_callback); - if (r < 0) { - s->reply_callback.cookie = 0; - return r; - } - - s->reply_callback.timeout_usec = calc_elapse(bus, m->timeout); - if (s->reply_callback.timeout_usec != 0) { - r = prioq_put(bus->reply_callbacks_prioq, &s->reply_callback, &s->reply_callback.prioq_idx); + s->reply_callback.cookie = BUS_MESSAGE_COOKIE(m); + r = ordered_hashmap_put(bus->reply_callbacks, &s->reply_callback.cookie, &s->reply_callback); if (r < 0) { - s->reply_callback.timeout_usec = 0; + s->reply_callback.cookie = 0; return r; } + + s->reply_callback.timeout_usec = calc_elapse(bus, m->timeout); + if (s->reply_callback.timeout_usec != 0) { + r = prioq_put(bus->reply_callbacks_prioq, &s->reply_callback, &s->reply_callback.prioq_idx); + if (r < 0) { + s->reply_callback.timeout_usec = 0; + return r; + } + } } - r = sd_bus_send(bus, m, &s->reply_callback.cookie); + r = sd_bus_send(bus, m, s ? &s->reply_callback.cookie : NULL); if (r < 0) return r; From 31b2cd5d23bae18d97b48838a3a4a1a59ee73e15 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Dec 2017 16:40:47 +0100 Subject: [PATCH 35/46] tree-wide: make the Subscribe() method calls asynchronous too --- src/login/logind.c | 14 ++++++-------- src/machine/machined.c | 14 ++++++-------- src/systemctl/systemctl.c | 11 +++++------ 3 files changed, 17 insertions(+), 22 deletions(-) diff --git a/src/login/logind.c b/src/login/logind.c index 9500fd98920..cbaaa48e5a5 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -658,7 +658,6 @@ static int manager_reserve_vt(Manager *m) { } static int manager_connect_bus(Manager *m) { - _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; int r; assert(m); @@ -740,18 +739,17 @@ static int manager_connect_bus(Manager *m) { if (r < 0) return log_error_errno(r, "Failed to request match for Reloading: %m"); - r = sd_bus_call_method( + r = sd_bus_call_method_async( m->bus, + NULL, "org.freedesktop.systemd1", "/org/freedesktop/systemd1", "org.freedesktop.systemd1.Manager", "Subscribe", - &error, - NULL, NULL); - if (r < 0) { - log_error("Failed to enable subscription: %s", bus_error_message(&error, r)); - return r; - } + NULL, NULL, + NULL); + if (r < 0) + return log_error_errno(r, "Failed to enable subscription: %m"); r = sd_bus_request_name_async(m->bus, NULL, "org.freedesktop.login1", 0, NULL, NULL); if (r < 0) diff --git a/src/machine/machined.c b/src/machine/machined.c index 7cbfc922d74..34b2024043a 100644 --- a/src/machine/machined.c +++ b/src/machine/machined.c @@ -186,7 +186,6 @@ int manager_enumerate_machines(Manager *m) { } static int manager_connect_bus(Manager *m) { - _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; int r; assert(m); @@ -260,18 +259,17 @@ static int manager_connect_bus(Manager *m) { if (r < 0) return log_error_errno(r, "Failed to request match for Reloading: %m"); - r = sd_bus_call_method( + r = sd_bus_call_method_async( m->bus, + NULL, "org.freedesktop.systemd1", "/org/freedesktop/systemd1", "org.freedesktop.systemd1.Manager", "Subscribe", - &error, - NULL, NULL); - if (r < 0) { - log_error("Failed to enable subscription: %s", bus_error_message(&error, r)); - return r; - } + NULL, NULL, + NULL); + if (r < 0) + return log_error_errno(r, "Failed to enable subscription: %m"); r = sd_bus_request_name_async(m->bus, NULL, "org.freedesktop.machine1", 0, NULL, NULL); if (r < 0) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 5f8ff17a9b4..906eb1879ac 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -3151,22 +3151,21 @@ static int start_unit(int argc, char *argv[], void *userdata) { } if (arg_wait) { - _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - wait_context.unit_paths = set_new(&string_hash_ops); if (!wait_context.unit_paths) return log_oom(); - r = sd_bus_call_method( + r = sd_bus_call_method_async( bus, + NULL, "org.freedesktop.systemd1", "/org/freedesktop/systemd1", "org.freedesktop.systemd1.Manager", "Subscribe", - &error, - NULL, NULL); + NULL, NULL, + NULL); if (r < 0) - return log_error_errno(r, "Failed to enable subscription: %s", bus_error_message(&error, r)); + return log_error_errno(r, "Failed to enable subscription: %m"); r = sd_event_default(&wait_context.event); if (r < 0) return log_error_errno(r, "Failed to allocate event loop: %m"); From 0f08647120031d674075eeba391d2463870f3a6b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Dec 2017 18:02:16 +0100 Subject: [PATCH 36/46] sd-bus: drop some unused fields from the sd_bus_message structure --- src/libsystemd/sd-bus/bus-message.c | 1 - src/libsystemd/sd-bus/bus-message.h | 4 ---- 2 files changed, 5 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index 219cff1f6e7..119c43bc9bc 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -127,7 +127,6 @@ static void message_free(sd_bus_message *m) { if (m->iovec != m->iovec_fixed) free(m->iovec); - m->destination_ptr = mfree(m->destination_ptr); message_reset_containers(m); free(m->root_container.signature); free(m->root_container.offsets); diff --git a/src/libsystemd/sd-bus/bus-message.h b/src/libsystemd/sd-bus/bus-message.h index 1e4b20926d5..88998700d6b 100644 --- a/src/libsystemd/sd-bus/bus-message.h +++ b/src/libsystemd/sd-bus/bus-message.h @@ -136,10 +136,6 @@ struct sd_bus_message { usec_t timeout; - char sender_buffer[3 + DECIMAL_STR_MAX(uint64_t) + 1]; - char destination_buffer[3 + DECIMAL_STR_MAX(uint64_t) + 1]; - char *destination_ptr; - size_t header_offsets[_BUS_MESSAGE_HEADER_MAX]; unsigned n_header_offsets; }; From 2767027bb7c6c8c71cf8061fb6beabb4c91cf32f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Dec 2017 18:13:23 +0100 Subject: [PATCH 37/46] test: fix condition test if there are no controllers As an optimization cg_mask_to_string() returns NULL if there are no controllers available. We need to handle that. --- src/test/test-condition.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/test-condition.c b/src/test/test-condition.c index 8323a66ad39..ad64a2bb36e 100644 --- a/src/test/test-condition.c +++ b/src/test/test-condition.c @@ -29,8 +29,8 @@ #include "apparmor-util.h" #include "architecture.h" #include "audit-util.h" -#include "condition.h" #include "cgroup-util.h" +#include "condition.h" #include "hostname-util.h" #include "id128-util.h" #include "ima-util.h" @@ -187,12 +187,12 @@ static int test_condition_test_control_group_controller(void) { /* Multiple valid controllers at the same time */ assert_se(cg_mask_to_string(system_mask, &controller_name) >= 0); - condition = condition_new(CONDITION_CONTROL_GROUP_CONTROLLER, controller_name, false, false); + condition = condition_new(CONDITION_CONTROL_GROUP_CONTROLLER, strempty(controller_name), false, false); assert_se(condition); assert_se(condition_test(condition)); condition_free(condition); - condition = condition_new(CONDITION_CONTROL_GROUP_CONTROLLER, controller_name, false, true); + condition = condition_new(CONDITION_CONTROL_GROUP_CONTROLLER, strempty(controller_name), false, true); assert_se(condition); assert_se(!condition_test(condition)); condition_free(condition); From c944ca8e82f7e60fed852d4854cdfa18ea609141 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Dec 2017 19:38:15 +0100 Subject: [PATCH 38/46] verbs: suppress debug log message if SYSTEMD_OFFLINE is not set If SYSTEMD_OFFLINE is not set getenv_bool() for it will return -ENXIO, which is nothing we should log about, not even at LOG_DEBUG level. --- src/basic/verbs.c | 37 ++++++++++++++----------------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/src/basic/verbs.c b/src/basic/verbs.c index d8ebf89d038..556acdcaa3a 100644 --- a/src/basic/verbs.c +++ b/src/basic/verbs.c @@ -30,45 +30,36 @@ #include "verbs.h" #include "virt.h" -/* Wraps running_in_chroot() which is used in various places, - * but also adds an environment variable check so external processes - * can reliably force this on. +/* Wraps running_in_chroot() which is used in various places, but also adds an environment variable check so external + * processes can reliably force this on. */ bool running_in_chroot_or_offline(void) { int r; - /* Added to support use cases like rpm-ostree, where from %post - * scripts we only want to execute "preset", but not "start"/"restart" - * for example. + /* Added to support use cases like rpm-ostree, where from %post scripts we only want to execute "preset", but + * not "start"/"restart" for example. * * See ENVIRONMENT.md for docs. */ r = getenv_bool("SYSTEMD_OFFLINE"); - if (r < 0) - log_debug_errno(r, "Parsing SYSTEMD_OFFLINE: %m"); - else if (r == 0) - return false; - else - return true; + if (r < 0 && r != -ENXIO) + log_debug_errno(r, "Failed to parse $SYSTEMD_OFFLINE: %m"); + else if (r >= 0) + return r > 0; - /* We've had this condition check for a long time which basically - * checks for legacy chroot case like Fedora's - * "mock", which is used for package builds. We don't want - * to try to start systemd services there, since without --new-chroot - * we don't even have systemd running, and even if we did, adding - * a concept of background daemons to builds would be an enormous change, - * requiring considering things like how the journal output is handled, etc. - * And there's really not a use case today for a build talking to a service. + /* We've had this condition check for a long time which basically checks for legacy chroot case like Fedora's + * "mock", which is used for package builds. We don't want to try to start systemd services there, since + * without --new-chroot we don't even have systemd running, and even if we did, adding a concept of background + * daemons to builds would be an enormous change, requiring considering things like how the journal output is + * handled, etc. And there's really not a use case today for a build talking to a service. * * Note this call itself also looks for a different variable SYSTEMD_IGNORE_CHROOT=1. */ r = running_in_chroot(); if (r < 0) log_debug_errno(r, "running_in_chroot(): %m"); - else if (r > 0) - return true; - return false; + return r > 0; } int dispatch_verb(int argc, char *argv[], const Verb verbs[], void *userdata) { From 48ef41a33592bbcf4dac441f13ce4555257a3e7b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Dec 2017 19:41:06 +0100 Subject: [PATCH 39/46] sd-bus: add API to optionally set a sender field on all outgoing messages This is useful on direct connections to generate messages with valid sender fields. This is particularly useful for services that are accessible both through direct connections and the broker, as it allows clients to install matches on the sender service name, and they work the same in both cases. --- src/libsystemd/libsystemd.sym | 3 +++ src/libsystemd/sd-bus/bus-internal.h | 1 + src/libsystemd/sd-bus/bus-message.c | 9 +++++++++ src/libsystemd/sd-bus/sd-bus.c | 29 ++++++++++++++++++++++++++++ src/systemd/sd-bus.h | 3 +++ 5 files changed, 45 insertions(+) diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index 27f3219ee23..5a073441752 100644 --- a/src/libsystemd/libsystemd.sym +++ b/src/libsystemd/libsystemd.sym @@ -543,4 +543,7 @@ global: sd_bus_is_ready; sd_bus_set_connected_signal; sd_bus_get_connected_signal; + sd_bus_set_sender; + sd_bus_get_sender; + sd_bus_message_set_sender; } LIBSYSTEMD_236; diff --git a/src/libsystemd/sd-bus/bus-internal.h b/src/libsystemd/sd-bus/bus-internal.h index 53f3194beff..a28f5f06f45 100644 --- a/src/libsystemd/sd-bus/bus-internal.h +++ b/src/libsystemd/sd-bus/bus-internal.h @@ -316,6 +316,7 @@ struct sd_bus { char *cgroup_root; char *description; + char *patch_sender; sd_bus_track *track_queue; diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index 119c43bc9bc..3fbb6dc306a 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -5487,6 +5487,15 @@ _public_ int sd_bus_message_set_destination(sd_bus_message *m, const char *desti return message_append_field_string(m, BUS_MESSAGE_HEADER_DESTINATION, SD_BUS_TYPE_STRING, destination, &m->destination); } +_public_ int sd_bus_message_set_sender(sd_bus_message *m, const char *sender) { + assert_return(m, -EINVAL); + assert_return(sender, -EINVAL); + assert_return(!m->sealed, -EPERM); + assert_return(!m->sender, -EEXIST); + + return message_append_field_string(m, BUS_MESSAGE_HEADER_SENDER, SD_BUS_TYPE_STRING, sender, &m->sender); +} + int bus_message_get_blob(sd_bus_message *m, void **buffer, size_t *sz) { size_t total; void *p, *e; diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 556c2173427..4156b40f91a 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -154,6 +154,7 @@ static void bus_free(sd_bus *b) { free(b->machine); free(b->cgroup_root); free(b->description); + free(b->patch_sender); free(b->exec_path); strv_free(b->exec_argv); @@ -274,6 +275,7 @@ _public_ int sd_bus_set_exec(sd_bus *bus, const char *path, char *const argv[]) _public_ int sd_bus_set_bus_client(sd_bus *bus, int b) { assert_return(bus, -EINVAL); assert_return(bus->state == BUS_UNSET, -EPERM); + assert_return(!bus->patch_sender, -EPERM); assert_return(!bus_pid_changed(bus), -ECHILD); bus->bus_client = !!b; @@ -1537,6 +1539,8 @@ _public_ int sd_bus_get_bus_id(sd_bus *bus, sd_id128_t *id) { } static int bus_seal_message(sd_bus *b, sd_bus_message *m, usec_t timeout) { + int r; + assert(b); assert(m); @@ -1551,6 +1555,12 @@ static int bus_seal_message(sd_bus *b, sd_bus_message *m, usec_t timeout) { if (timeout == 0) timeout = BUS_DEFAULT_TIMEOUT; + if (!m->sender && b->patch_sender) { + r = sd_bus_message_set_sender(m, b->patch_sender); + if (r < 0) + return r; + } + return sd_bus_message_seal(m, ++b->cookie, timeout); } @@ -3959,3 +3969,22 @@ _public_ int sd_bus_get_exit_on_disconnect(sd_bus *bus) { return bus->exit_on_disconnect; } + +_public_ int sd_bus_set_sender(sd_bus *bus, const char *sender) { + assert_return(bus, -EINVAL); + assert_return(!bus->bus_client, -EPERM); + assert_return(!sender || service_name_is_valid(sender), -EINVAL); + + return free_and_strdup(&bus->patch_sender, sender); +} + +_public_ int sd_bus_get_sender(sd_bus *bus, const char **ret) { + assert_return(bus, -EINVAL); + assert_return(ret, -EINVAL); + + if (!bus->patch_sender) + return -ENODATA; + + *ret = bus->patch_sender; + return 0; +} diff --git a/src/systemd/sd-bus.h b/src/systemd/sd-bus.h index df21b3f30e6..c30ac9e6ec4 100644 --- a/src/systemd/sd-bus.h +++ b/src/systemd/sd-bus.h @@ -154,6 +154,8 @@ int sd_bus_set_watch_bind(sd_bus *bus, int b); int sd_bus_get_watch_bind(sd_bus *bus); int sd_bus_set_connected_signal(sd_bus *bus, int b); int sd_bus_get_connected_signal(sd_bus *bus); +int sd_bus_set_sender(sd_bus *bus, const char *sender); +int sd_bus_get_sender(sd_bus *bus, const char **ret); int sd_bus_start(sd_bus *bus); @@ -273,6 +275,7 @@ int sd_bus_message_set_auto_start(sd_bus_message *m, int b); int sd_bus_message_set_allow_interactive_authorization(sd_bus_message *m, int b); int sd_bus_message_set_destination(sd_bus_message *m, const char *destination); +int sd_bus_message_set_sender(sd_bus_message *m, const char *sender); int sd_bus_message_set_priority(sd_bus_message *m, int64_t priority); int sd_bus_message_append(sd_bus_message *m, const char *types, ...); From a5ea30b75b827a93afaef97f0eb3d13cac3ba8f9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Dec 2017 19:43:05 +0100 Subject: [PATCH 40/46] pid1: set org.freedesktop.systemd1 as sender service name for direct connections This way, clients can install the very same match on direct and broker connections as in both cases the messages will originate from the o.f.s1 service. --- src/core/dbus.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/core/dbus.c b/src/core/dbus.c index 0d9f1f03467..5e8a2999838 100644 --- a/src/core/dbus.c +++ b/src/core/dbus.c @@ -682,6 +682,12 @@ static int bus_on_connection(sd_event_source *s, int fd, uint32_t revents, void return 0; } + r = sd_bus_set_sender(bus, "org.freedesktop.systemd1"); + if (r < 0) { + log_warning_errno(r, "Failed to set direct connection sender: %m"); + return 0; + } + r = sd_bus_start(bus); if (r < 0) { log_warning_errno(r, "Failed to start new connection bus: %m"); From 66baf8c6444fa036e4da4a15d7099c21480bfefb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Dec 2017 19:48:39 +0100 Subject: [PATCH 41/46] sd-bus: remove 'hint_sync_call' parameter from various function calls This is unused since kdbus is gone, hence remove this too. This permits us to get rid of sd_bus_send_internal() and just implement sd_bus_send() directly. --- src/libsystemd/sd-bus/sd-bus.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 4156b40f91a..8c8853c7eeb 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -1604,7 +1604,7 @@ int bus_seal_synthetic_message(sd_bus *b, sd_bus_message *m) { return sd_bus_message_seal(m, 0xFFFFFFFFULL, 0); } -static int bus_write_message(sd_bus *bus, sd_bus_message *m, bool hint_sync_call, size_t *idx) { +static int bus_write_message(sd_bus *bus, sd_bus_message *m, size_t *idx) { int r; assert(bus); @@ -1639,7 +1639,7 @@ static int dispatch_wqueue(sd_bus *bus) { while (bus->wqueue_size > 0) { - r = bus_write_message(bus, bus->wqueue[0], false, &bus->windex); + r = bus_write_message(bus, bus->wqueue[0], &bus->windex); if (r < 0) return r; else if (r == 0) @@ -1718,7 +1718,7 @@ static int dispatch_rqueue(sd_bus *bus, bool hint_priority, int64_t priority, sd } } -static int bus_send_internal(sd_bus *bus, sd_bus_message *_m, uint64_t *cookie, bool hint_sync_call) { +_public_ int sd_bus_send(sd_bus *bus, sd_bus_message *_m, uint64_t *cookie) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = sd_bus_message_ref(_m); int r; @@ -1763,7 +1763,7 @@ static int bus_send_internal(sd_bus *bus, sd_bus_message *_m, uint64_t *cookie, if (IN_SET(bus->state, BUS_RUNNING, BUS_HELLO) && bus->wqueue_size <= 0) { size_t idx = 0; - r = bus_write_message(bus, m, hint_sync_call, &idx); + r = bus_write_message(bus, m, &idx); if (r < 0) { if (IN_SET(r, -ENOTCONN, -ECONNRESET, -EPIPE, -ESHUTDOWN)) { bus_enter_closing(bus); @@ -1803,10 +1803,6 @@ finish: return 1; } -_public_ int sd_bus_send(sd_bus *bus, sd_bus_message *m, uint64_t *cookie) { - return bus_send_internal(bus, m, cookie, false); -} - _public_ int sd_bus_send_to(sd_bus *bus, sd_bus_message *m, const char *destination, uint64_t *cookie) { int r; @@ -2013,7 +2009,7 @@ _public_ int sd_bus_call( if (r < 0) goto fail; - r = bus_send_internal(bus, m, &cookie, true); + r = sd_bus_send(bus, m, &cookie); if (r < 0) goto fail; From 51b13863a2d64fa7c8da8006c4d345e08f9986c2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Dec 2017 12:51:14 +0100 Subject: [PATCH 42/46] meson: resurrect API documentation target We had this functionality back in Automake times, let's resurrect it. --- meson.build | 8 ++++++++ tools/meson-check-api-docs.sh | 11 +++++++++++ 2 files changed, 19 insertions(+) create mode 100644 tools/meson-check-api-docs.sh diff --git a/meson.build b/meson.build index f95feee99de..924274de630 100644 --- a/meson.build +++ b/meson.build @@ -2556,6 +2556,14 @@ endif ############################################################ +meson_check_api_docs_sh = find_program('tools/meson-check-api-docs.sh') +run_target( + 'check-api-docs', + depends : [man, libsystemd, libudev], + command : [meson_check_api_docs_sh, libsystemd.full_path(), libudev.full_path()]) + +############################################################ + status = [ '@0@ @1@'.format(meson.project_name(), meson.project_version()), diff --git a/tools/meson-check-api-docs.sh b/tools/meson-check-api-docs.sh new file mode 100644 index 00000000000..5bc808c1e49 --- /dev/null +++ b/tools/meson-check-api-docs.sh @@ -0,0 +1,11 @@ +#!/bin/sh + +set -eu + +for symbol in `nm -g --defined-only "$@" | grep " T " | cut -d" " -f3 | sort -u` ; do + if test -f ${MESON_BUILD_ROOT}/man/$symbol.3 ; then + echo "✓ Symbol $symbol() is documented." + else + printf " \x1b[1;31mSymbol $symbol() lacks documentation.\x1b[0m\n" + fi +done From d97eac36d889a43bc71dcc9d43f8b6e536dddaae Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Dec 2017 16:31:58 +0100 Subject: [PATCH 43/46] man: document all the new APIs we added --- man/rules/meson.build | 21 +++- man/sd_bus_add_match.xml | 139 ++++++++++++++++------ man/sd_bus_is_open.xml | 137 ++++++++++++++++++++++ man/sd_bus_message_set_destination.xml | 137 ++++++++++++++++++++++ man/sd_bus_request_name.xml | 138 +++++++++++++--------- man/sd_bus_set_connected_signal.xml | 143 +++++++++++++++++++++++ man/sd_bus_set_sender.xml | 136 ++++++++++++++++++++++ man/sd_bus_set_watch_bind.xml | 152 +++++++++++++++++++++++++ 8 files changed, 908 insertions(+), 95 deletions(-) create mode 100644 man/sd_bus_is_open.xml create mode 100644 man/sd_bus_message_set_destination.xml create mode 100644 man/sd_bus_set_connected_signal.xml create mode 100644 man/sd_bus_set_sender.xml create mode 100644 man/sd_bus_set_watch_bind.xml diff --git a/man/rules/meson.build b/man/rules/meson.build index 54c5a9d9235..bfc267b544d 100644 --- a/man/rules/meson.build +++ b/man/rules/meson.build @@ -105,7 +105,12 @@ manpages = [ ['sd-journal', '3', [], ''], ['sd-login', '3', [], 'HAVE_PAM'], ['sd_booted', '3', [], ''], - ['sd_bus_add_match', '3', [], ''], + ['sd_bus_add_match', + '3', + ['sd_bus_add_match_async', + 'sd_bus_match_signal', + 'sd_bus_match_signal_async'], + ''], ['sd_bus_creds_get_pid', '3', ['sd_bus_creds_get_audit_login_uid', @@ -181,6 +186,7 @@ manpages = [ ['SD_BUS_ERROR_END', 'SD_BUS_ERROR_MAP', 'sd_bus_error_map'], ''], ['sd_bus_get_fd', '3', [], ''], + ['sd_bus_is_open', '3', ['sd_bus_is_ready'], ''], ['sd_bus_message_append', '3', ['sd_bus_message_appendv'], ''], ['sd_bus_message_append_array', '3', @@ -200,6 +206,7 @@ manpages = [ ['sd_bus_message_get_realtime_usec', 'sd_bus_message_get_seqnum'], ''], ['sd_bus_message_read_basic', '3', [], ''], + ['sd_bus_message_set_destination', '3', ['sd_bus_message_set_sender'], ''], ['sd_bus_negotiate_fds', '3', ['sd_bus_negotiate_creds', 'sd_bus_negotiate_timestamp'], @@ -210,7 +217,15 @@ manpages = [ ['sd_bus_path_decode', 'sd_bus_path_decode_many', 'sd_bus_path_encode_many'], ''], ['sd_bus_process', '3', [], ''], - ['sd_bus_request_name', '3', ['sd_bus_release_name'], ''], + ['sd_bus_request_name', + '3', + ['sd_bus_release_name', + 'sd_bus_release_name_async', + 'sd_bus_request_name_async'], + ''], + ['sd_bus_set_connected_signal', '3', ['sd_bus_get_connected_signal'], ''], + ['sd_bus_set_sender', '3', ['sd_bus_get_sender'], ''], + ['sd_bus_set_watch_bind', '3', ['sd_bus_get_watch_bind'], ''], ['sd_bus_track_add_name', '3', ['sd_bus_track_add_sender', @@ -655,6 +670,7 @@ manpages = [ ['systemd', '1', ['init'], ''], ['systemd.automount', '5', [], ''], ['systemd.device', '5', [], ''], + ['systemd.dnssd', '5', [], 'ENABLE_RESOLVE'], ['systemd.environment-generator', '7', [], 'ENABLE_ENVIRONMENT_D'], ['systemd.exec', '5', [], ''], ['systemd.generator', '7', [], ''], @@ -663,7 +679,6 @@ manpages = [ ['systemd.link', '5', [], ''], ['systemd.mount', '5', [], ''], ['systemd.netdev', '5', [], 'ENABLE_NETWORKD'], - ['systemd.dnssd', '5', [], 'ENABLE_RESOLVE'], ['systemd.network', '5', [], 'ENABLE_NETWORKD'], ['systemd.nspawn', '5', [], ''], ['systemd.offline-updates', '7', [], ''], diff --git a/man/sd_bus_add_match.xml b/man/sd_bus_add_match.xml index 4772c738ac0..f11d0782842 100644 --- a/man/sd_bus_add_match.xml +++ b/man/sd_bus_add_match.xml @@ -45,14 +45,24 @@ sd_bus_add_match + sd_bus_add_match_async + sd_bus_match_signal + sd_bus_match_signal_async - Add a match rule for message dispatching + Add a match rule for incoming message dispatching #include <systemd/sd-bus.h> + + typedef int (*sd_bus_message_handler_t) + sd_bus_message *m + void *userdata + sd_bus_error *ret_error + + int sd_bus_add_match sd_bus *bus @@ -63,65 +73,122 @@ - typedef int (*sd_bus_message_handler_t) - sd_bus_message *m + int sd_bus_add_match_async + sd_bus *bus + sd_bus_slot **slot + const char *match + sd_bus_message_handler_t callback + sd_bus_message_handler_t install_callback void *userdata - sd_bus_error *ret_error + + + int sd_bus_match_signal + sd_bus *bus + sd_bus_slot **slot + const char *sender + const char *path + const char *interface + const char *member + sd_bus_message_handler_t callback + void *userdata + + + + int sd_bus_match_signal_async + sd_bus *bus + sd_bus_slot **slot + const char *sender + const char *path + const char *interface + const char *member + sd_bus_message_handler_t callback + sd_bus_message_handler_t install_callback + void *userdata + + Description - - sd_bus_add_match() installs a match rule for incoming messages received on the specified bus - connection object bus. The syntax of the match rule expression passed in - match is described in the D-Bus Specification. The specified handler - function callback is called for eaching incoming message matching the specified - expression, the userdata parameter is passed as-is to the callback function. + sd_bus_add_match() installs a match rule for messages received on the specified bus + connection object bus. The syntax of the match rule expression passed in + match is described in the D-Bus Specification. The specified handler + function callback is called for eaching incoming message matching the specified expression, + the userdata parameter is passed as-is to the callback function. The match is installed + synchronously when connected to a bus broker, i.e. the call sends a control message requested the match to be added + to the broker and waits until the broker confirms the match has been installed successfully. + + sd_bus_add_match_async() operates very similar to + sd_bus_match_signal(), however it installs the match asynchronously, in a non-blocking + fashion: a request is sent to the broker, but the call does not wait for a response. The + install_callback function is called when the response is later received, with the response + message from the broker as parameter. If this function is specified as NULL a default + implementation is used that terminates the bus connection should installing the match fail. + + sd_bus_match_signal() is very similar to sd_bus_add_match(), but + only matches signals, and instead of a match expression accepts four parameters: sender (the + service name of the sender), path (the object path of the emitting object), + interface (the interface the signal belongs to), member (the signal + name), from which the match string is internally generated. Optionally, these parameters may be specified as + NULL in which case the relevant field of incoming signals is not tested. + + sd_bus_match_signal_async() is combines the signal matching logic of + sd_bus_match_signal() with the asynchronous behaviour of + sd_bus_add_match_async(). + + On success, and if non-NULL, the slot return parameter will be + set to a slot object that may be used as a reference to the installed match, and may be utilized to remove it again + at a later time with + sd_bus_slot_unref3. If specified + as NULL the lifetime of the match is bound to the lifetime of the bus object itself, and the + match cannot be removed independently. + + The message m passed to the callback is only borrowed, that is, the callback should + not call sd_bus_message_unref3 + on it. If the callback wants to hold on to the message beyond the lifetime of the callback, it needs to call + sd_bus_message_ref3 to create a + new reference. + + If an error occurs during the callback invocation, the callback should return a negative error number + (optionally, a more precise error may be returned in ret_error, as well). If it wants other + callbacks that match the same rule to be called, it should return 0. Otherwise it should return a positive integer. - - On success, and if non-NULL, the slot return parameter will be set to - a slot object that may be used as a reference to the installed match, and may be utilized to remove it again at a - later time with - sd_bus_slot_unref3. If - specified as NULL the lifetime of the match is bound to the lifetime of the bus object itself, and the match - cannot be removed independently. - - - - The message m passed to the callback is only borrowed, that is, the callback should not - call sd_bus_message_unref3 on - it. If the callback wants to hold on to the message beyond the lifetime of the callback, it needs to call - sd_bus_message_ref3 to create - a new reference. - - - - If an error occurs during the callback invocation, the callback should return a negative error number. If it - wants other callbacks that match the same rule to be called, it should return 0. Otherwise it should return a - positive integer. - + If the bus refers to a direct connection (i.e. not a bus connection, as set with + sd_bus_set_bus_client3) the + match is only installed on the client side, and the synchronous and asynchronous functions operate the same. Return Value - On success, sd_bus_add_match() returns 0 or a positive integer. On failure, it returns a - negative errno-style error code. + On success, sd_bus_add_match() and the other calls return 0 or a positive integer. On + failure, they return a negative errno-style error code. + + Notes + + sd_bus_add_match() and the other functions described here are available as a shared + library, which can be compiled and linked to with the libsystemd pkg-config1 file. + + See Also systemd1, - sd-bus3 + sd-bus3, + sd_bus_slot_unref3, + sd_bus_message_ref3, + sd_bus_set_bus_client3 diff --git a/man/sd_bus_is_open.xml b/man/sd_bus_is_open.xml new file mode 100644 index 00000000000..9bc671fc377 --- /dev/null +++ b/man/sd_bus_is_open.xml @@ -0,0 +1,137 @@ + + + + + + + + + sd_bus_is_open + systemd + + + + Developer + Lennart + Poettering + lennart@poettering.net + + + + + + sd_bus_is_open + 3 + + + + sd_bus_is_open + sd_bus_is_ready + + Check whether the a bus connection is open or ready. + + + + + #include <systemd/sd-bus.h> + + + int sd_bus_is_open + sd_bus *bus + + + + int sd_bus_is_ready + sd_bus *bus + + + + + + + Description + + sd_bus_is_open() checks whether the specified bus connection is open, i.e. in the + process of being established, already established or in the process of being torn down. It returns zero when the + connection has not been started yet + (i.e. sd_bus_start3 or some + equivalent call has not been invoked yet), or is fully terminated again (for example after + sd_bus_close3), it returns + positive otherwise. + + sd_bus_is_ready() checks whether the specified connection is fully established, + i.e. completed the connection and authentication phases of the protocol and received the + Hello() method call response, and is not in the process of being torn down again. It returns + zero outside of this state, and positive otherwise. Effectively, this function returns positive while regular + messages can be sent or received on the connection. + + To be notified when the connection is fully established, use + sd_bus_set_connected_signal3 and + install a match for the Connected() signal on the + org.freedesktop.DBus.Local interface. To be notified when the connection is torn down again, + install a match for the Disconnected() signal on the + org.freedesktop.DBus.Local interface. + + + + Return Value + + On success, these functions return 0 or a positive integer. On failure, they return a negative errno-style + error code. + + + + Errors + + Returned errors may indicate the following problems: + + + + -ECHILD + + The bus connection has been created in a different process. + + + + + + Notes + + sd_bus_is_open() and sd_bus_is_ready() are available as + a shared library, which can be compiled and linked to with the libsystemd pkg-config1 file. + + + + See Also + + + systemd1, + sd-bus3, + sd_bus_start3, + sd_bus_close3, + sd_bus_set_connected_signal3 + + + + diff --git a/man/sd_bus_message_set_destination.xml b/man/sd_bus_message_set_destination.xml new file mode 100644 index 00000000000..ff69a231525 --- /dev/null +++ b/man/sd_bus_message_set_destination.xml @@ -0,0 +1,137 @@ + + + + + + + + sd_bus_message_set_destination + systemd + + + + Developer + Lennart + Poettering + lennart@poettering.net + + + + + + sd_bus_message_set_destination + 3 + + + + sd_bus_message_set_destination + sd_bus_message_set_sender + Set the destination or sender service name of a bus message + + + + + #include <systemd/sd-bus.h> + + + int sd_bus_message_set_destination + sd_bus_message *message + const char *destination + + + + int sd_bus_message_set_sender + sd_bus_message *message + const char *sender + + + + + + Description + + sd_bus_message_set_destination() sets the destination service name for the specified bus + message object. The specified name must be a valid unique or well-known service name. + + sd_bus_message_set_sender() sets the sender service name for the specified bus message + object. The specified name must be a valid unique or well-known service name. This function is useful only for + messages to send on direct connections as for connections to bus brokers the broker will fill in the destination + field anyway, and the sender field set by original sender is ignored. + + + + Return Value + + On success, these calls return 0 or a positive integer. On failure, these calls return a negative errno-style + error code. + + + + Errors + + Returned errors may indicate the following problems: + + + + -EINVAL + + A specified parameter is invalid. + + + + -EPERM + + The message is already sealed. + + + + -EEXIST + + The message already has a destination or sender field set. + + + + + + Notes + + The sd_bus_message_set_destination() and + sd_bus_message_set_sender() interfaces + are available as a shared library, which can be compiled and + linked to with the + libsystemd pkg-config1 + file. + + + + See Also + + + systemd1, + sd-bus3, + sd_bus_new3, + sd_bus_set_sender3 + + + + diff --git a/man/sd_bus_request_name.xml b/man/sd_bus_request_name.xml index f44cfdb1ac1..9e668f0b025 100644 --- a/man/sd_bus_request_name.xml +++ b/man/sd_bus_request_name.xml @@ -46,7 +46,9 @@ sd_bus_request_name + sd_bus_request_name_async sd_bus_release_name + sd_bus_release_name_async Request or release a well-known service name on a bus @@ -61,71 +63,103 @@ uint64_t flags + + int sd_bus_request_name_async + sd_bus *bus + sd_bus_slot **slot + const char *name + uint64_t flags + sd_bus_message_handler_t callback + void *userdata + + int sd_bus_release_name sd_bus *bus const char *name + + + int sd_bus_release_name_async + sd_bus *bus + sd_bus_slot **slot + const char *name + sd_bus_message_handler_t callback + void *userdata + Description - sd_bus_request_name() requests a - well-known service name on a bus. It takes a bus connection, a - valid bus name and a flags parameter. The flags parameter is a - combination of the following flags: + sd_bus_request_name() requests a well-known service name on a bus. It takes a bus + connection, a valid bus name and a flags parameter. The flags parameter is a combination of the following + flags: SD_BUS_NAME_ALLOW_REPLACEMENT - After acquiring the name successfully, permit - other peers to take over the name when they try to acquire it - with the SD_BUS_NAME_REPLACE_EXISTING flag - set. If SD_BUS_NAME_ALLOW_REPLACEMENT is - not set on the original request, such a request by other peers - will be denied. + After acquiring the name successfully, permit other peers to take over the name when they try + to acquire it with the SD_BUS_NAME_REPLACE_EXISTING flag set. If + SD_BUS_NAME_ALLOW_REPLACEMENT is not set on the original request, such a request by other + peers will be denied. SD_BUS_NAME_REPLACE_EXISTING - Take over the name if it is already acquired - by another peer, and that other peer has permitted takeover by - setting SD_BUS_NAME_ALLOW_REPLACEMENT while - acquiring it. + Take over the name if it is already acquired by another peer, and that other peer has permitted + takeover by setting SD_BUS_NAME_ALLOW_REPLACEMENT while acquiring it. SD_BUS_NAME_QUEUE - Queue the acquisition of the name when the - name is already taken. + Queue the acquisition of the name when the name is already taken. - sd_bus_release_name() releases an - acquired well-known name. It takes a bus connection and a valid - bus name as parameters. + sd_bus_request_name() operates in a synchronous fashion: a message requesting the name + is sent to the bus broker, and the call waits until the broker responds. + + sd_bus_request_name_async() is an asynchronous version of of + sd_bus_release_name(). Instead of waiting for the request to complete, the request message is + enqueued. The specified callback will be called when the broker's response is received. If + the parameter is specified as NULL a default implementation is used instead which will + terminate the connection when the name cannot be acquired. The function returns a slot object in its + slot parameter — if it is passed as non-NULL — which may be used as a + reference to the name request operation. Use + sd_bus_slot_unref3 to destroy + this reference. Note that destroying the reference will not unregister the name, but simply ensure the specified + callback is no longer called. + + sd_bus_release_name() releases an acquired well-known name. It takes a bus connection + and a valid bus name as parameters. This function operates synchronously, sending a release request message to the + bus broker and waiting for it to reply. + + sd_bus_release_name_async() is an asynchronous version of + sd_bus_release_name(). The specified callback function is called when + the name has been released successfully. If specified as NULL a generic implementation is used + that ignores the result of the operation. As above, the slot (if + non-NULL) is set to an object that may be used to reference the operation. + + These functions are supported only on bus connections, i.e. connections to a bus broker and not on direct + connections. Return Value - On success, these calls return 0 or a positive integer. On - failure, these calls return a negative errno-style error - code. + On success, these calls return 0 or a positive integer. On failure, these calls return a negative errno-style + error code. - If SD_BUS_NAME_QUEUE is specified, - sd_bus_request_name() will return 0 when the - name is already taken by another peer and the client has been - added to the queue for the name. In that case, the caller can - subscribe to NameOwnerChanged signals to be - notified when the name is successfully acquired. - sd_bus_request_name() returns > 0 when the - name has immediately been acquired successfully. + If SD_BUS_NAME_QUEUE is specified, sd_bus_request_name() will return + 0 when the name is already taken by another peer and the client has been added to the queue for the name. In that + case, the caller can subscribe to NameOwnerChanged signals to be notified when the name is + successfully acquired. sd_bus_request_name() returns > 0 when the name has immediately + been acquired successfully. @@ -137,56 +171,50 @@ -EALREADY - The caller already is the owner of the - specified name. + The caller already is the owner of the specified name. -EEXIST - The name has already been acquired by a - different peer, and SD_BUS_NAME_REPLACE_EXISTING was not - specified or the other peer did not specify - SD_BUS_NAME_ALLOW_REPLACEMENT while acquiring the + The name has already been acquired by a different peer, and SD_BUS_NAME_REPLACE_EXISTING was + not specified or the other peer did not specify SD_BUS_NAME_ALLOW_REPLACEMENT while acquiring the name. -ESRCH - It was attempted to release a name that is - currently not registered on the bus. + It was attempted to release a name that is currently not registered on the + bus. -EADDRINUSE - It was attempted to release a name that is - owned by a different peer on the bus. + It was attempted to release a name that is owned by a different peer on the + bus. -EINVAL - A specified parameter is invalid. This is also - generated when the requested name is a special service name - reserved by the D-Bus specification, or when the operation is - requested on a connection that does not refer to a - bus. + A specified parameter is invalid. This is also generated when the requested name is a special + service name reserved by the D-Bus specification, or when the operation is requested on a connection that does + not refer to a bus. -ENOTCONN - The bus connection has been - disconnected. + The bus connection has been disconnected. -ECHILD - The bus connection has been created in a - different process than the current one. + The bus connection has been created in a different process than the current + one. @@ -194,12 +222,9 @@ Notes - The sd_bus_acquire_name() and - sd_bus_release_name() interfaces are - available as a shared library, which can be compiled and linked to - with the - libsystemd pkg-config1 - file. + The sd_bus_acquire_name() and the other interfaces described here are available as a + shared library, which can be compiled and linked to with the libsystemd pkg-config1 file. @@ -208,7 +233,8 @@ systemd1, sd-bus3, - sd_bus_new3 + sd_bus_new3, + sd_bus_slot_unref3 diff --git a/man/sd_bus_set_connected_signal.xml b/man/sd_bus_set_connected_signal.xml new file mode 100644 index 00000000000..9374dac3a5a --- /dev/null +++ b/man/sd_bus_set_connected_signal.xml @@ -0,0 +1,143 @@ + + + + + + + + + sd_bus_set_connected_signal + systemd + + + + Developer + Lennart + Poettering + lennart@poettering.net + + + + + + sd_bus_set_connected_signal + 3 + + + + sd_bus_set_connected_signal + sd_bus_get_connected_signal + + Control emmission of local connection establishment signal on bus connections + + + + + #include <systemd/sd-bus.h> + + + int sd_bus_set_connected_signal + sd_bus *bus + int b + + + + int sd_bus_get_connected_signal + sd_bus *bus + + + + + + + Description + + sd_bus_set_connected_signal() may be used to control whether a local, synthetic + Connected() signal message shall be generated and enqueued for dispatching when the connection + is fully established. If the b parameter is zero the message is not generated (the default), + otherwise it is generated. + + sd_bus_get_connected_signal() may be used to query whether this feature is enabled. It + returns zero if not, positive otherwise. + + The Connected() signal message is generated from the + org.freedesktop.DBus.Local service and interface, and + /org/freedesktop/DBus/Local object path. Use + sd_bus_match_signal_async3 to + match on this signal. + + This message is particularly useful on slow transports where connections take a long time to be + established. This is especially the case when + sd_bus_set_watch_bind3 is + used. The signal is generated when the + sd_bus_is_ready3 returns + positive for the first time. + + The Connected() signal corresponds with the Disconnected() signal + that is synthesized locally when the connection is terminated. The latter is generated unconditionally however, + unlike the former which needs to be enabled explicitly before it is generated, with + sd_bus_set_connected_signal(). + + + + Return Value + + On success, these functions return 0 or a positive integer. On failure, they return a negative errno-style + error code. + + + + Errors + + Returned errors may indicate the following problems: + + + + -ECHILD + + The bus connection has been created in a different process. + + + + + + Notes + + sd_bus_set_connected_signal() and sd_bus_get_connected_signal() are available as + a shared library, which can be compiled and linked to with the libsystemd pkg-config1 file. + + + + See Also + + + systemd1, + sd-bus3, + sd_bus_match_signal_async3, + sd_bus_set_watch_bind3, + sd_bus_is_ready3 + + + + diff --git a/man/sd_bus_set_sender.xml b/man/sd_bus_set_sender.xml new file mode 100644 index 00000000000..38efda7286c --- /dev/null +++ b/man/sd_bus_set_sender.xml @@ -0,0 +1,136 @@ + + + + + + + + + sd_bus_set_sender + systemd + + + + Developer + Lennart + Poettering + lennart@poettering.net + + + + + + sd_bus_set_sender + 3 + + + + sd_bus_set_sender + sd_bus_get_sender + + Configure default sender for outgoing messages + + + + + #include <systemd/sd-bus.h> + + + int sd_bus_set_sender + sd_bus *bus + const char* name + + + + int sd_bus_get_sender + sd_bus *bus + const char** name + + + + + + + Description + + sd_bus_set_sender() configures the default sender service name to use for outgoing + messages. The service name specified in the name parameter is set on all outgoing messages + that are sent on the connection and have no sender set yet, for example through + sd_bus_message_set_sender3. Note + that this function is only supported on direct connections, i.e. not on connections to a bus broker as the broker + will fill in the sender service name automatically anyway. By default no sender name is configured, and hence + messages are sent without sender field set. If the name parameter is specified as + NULL the default sender service name is cleared, returning to the default state if a default + sender service name was set before. If passed as non-NULL the specified name must be a valid + unique or well-known service name. + + sd_bus_get_sender() may be used to query the current default service name for outgoing + messages. + + + + Return Value + + On success, these functions return 0 or a positive integer. On failure, they return a negative errno-style + error code. + + + + Errors + + Returned errors may indicate the following problems: + + + + -ECHILD + + The bus connection has been created in a different process. + + + + -EPERM + + The specified bus connection object is a not a direct but a brokered connection. + + + + + + Notes + + sd_bus_set_sender() and sd_bus_get_sender() are available as + a shared library, which can be compiled and linked to with the libsystemd pkg-config1 file. + + + + See Also + + + systemd1, + sd-bus3, + sd_bus_message_set_sender3 + + + + diff --git a/man/sd_bus_set_watch_bind.xml b/man/sd_bus_set_watch_bind.xml new file mode 100644 index 00000000000..5e6a6fa5880 --- /dev/null +++ b/man/sd_bus_set_watch_bind.xml @@ -0,0 +1,152 @@ + + + + + + + + + sd_bus_set_watch_bind + systemd + + + + Developer + Lennart + Poettering + lennart@poettering.net + + + + + + sd_bus_set_watch_bind + 3 + + + + sd_bus_set_watch_bind + sd_bus_get_watch_bind + + Control socket binding watching on bus connections + + + + + #include <systemd/sd-bus.h> + + + int sd_bus_set_watch_bind + sd_bus *bus + int b + + + + int sd_bus_get_watch_bind + sd_bus *bus + + + + + + + Description + + sd_bus_set_watch_bind() may be used to enable or disable client-side watching of server + socket binding for a bus connection object. If the b is true, the feature is enabled, + otherwise disabled (which is the default). When enabled, and the selected bus address refers to an + AF_UNIX socket in the file system which does not exist while the connection attempt is made an + inotify7 watch is installed on + it, waiting for the socket to appear. As soon as the socket appears the connection is made. This functionality is + useful in particular in early-boot programs that need to run before the system bus is available, but want to + connect to it the instant it may be connected to. + + sd_bus_get_watch_bind() may be used to query the current setting of this feature. It + returns zero when the feature is disabled, and positive if enabled. + + Note that no timeout is applied while it is waited for the socket to appear. This means that any synchronous + remote operation (such as + sd_bus_call3, + sd_bus_add_match3 or + sd_bus_request_name3), that is + used on a connection with this feature enabled that is not established yet might block unbounded if the socket is + never created. However, asynchronous remote operations (such as + sd_bus_send3, + sd_bus_add_match_async3 or + sd_bus_request_match_async3) do + not block in this case, and safely enqueue the requested operations to be dispatched the instant the connection is + set up. + + Use sd_bus_is_ready3 to + determine whether the connection is fully established, i.e. whether the peer socket has been bound, connected to + and authenticated. Use + sd_bus_set_connected_signal3 to + be notified when the connection is fully established. + + + + + Return Value + + On success, these functions return 0 or a positive integer. On failure, they return a negative errno-style + error code. + + + + Errors + + Returned errors may indicate the following problems: + + + + -ECHILD + + The bus connection has been created in a different process. + + + + + + Notes + + sd_bus_set_watch_bind() and sd_bus_get_watch_bind() are available as + a shared library, which can be compiled and linked to with the libsystemd pkg-config1 file. + + + + See Also + + + systemd1, + sd-bus3, + inotify7, + sd_bus_call3, + sd_bus_add_match3, + sd_bus_request_name3, + sd_bus_is_ready3, + sd_bus_set_connected_signal3 + + + + From c3cd7cc9297f44a2c856b74a510fdac14bd3ec61 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Dec 2017 19:48:23 +0100 Subject: [PATCH 44/46] update TODO --- TODO | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/TODO b/TODO index 251ce4f8ec7..a28c82ed665 100644 --- a/TODO +++ b/TODO @@ -35,6 +35,9 @@ Features: * add bpf-based implementation of devices cgroup controller logic for compat with cgroupsv2 as supported by newest kernel +* sd-bus: add vtable flag, that may be used to request client creds implicitly + and asynchronously before dispatching the operation + * implement transient socket unit. * make systemd-run create transient path and socket unit. @@ -56,8 +59,6 @@ Features: the runtime dir as we maintain for the fdstore: i.e. keep it around as long as the unit is running or has a job queued. -* add async version of sd_bus_add_match and make use of that - * support projid-based quota in machinectl for containers, and then drop implicit btrfs loopback magic in machined @@ -495,14 +496,12 @@ Features: - see if we can introduce a new sd_bus_get_owner_machine_id() call to retrieve the machine ID of the machine of the bus itself - see if we can drop more message validation on the sending side - add API to clone sd_bus_message objects - - make AddMatch calls on dbus1 transports async? - longer term: priority inheritance - dbus spec updates: - NameLost/NameAcquired obsolete - GVariant - path escaping - update systemd.special(7) to mention that dbus.socket is only about the compatibility socket now - - test bloom filter generation indexes * sd-event - allow multiple signal handlers per signal? From 2ac0ab5921a3153e0334b4342554fc0c87ab01c3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 4 Jan 2018 11:36:35 +0100 Subject: [PATCH 45/46] logind: fix user_object_find() The logic was completely borked since e4d2984bf8514ab576a66d5ac1f1cde746bb32a3, correct that. CID #1384234 --- src/login/logind-user-dbus.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/login/logind-user-dbus.c b/src/login/logind-user-dbus.c index 9fca5ce0cda..d5d086cfe04 100644 --- a/src/login/logind-user-dbus.c +++ b/src/login/logind-user-dbus.c @@ -288,13 +288,13 @@ int user_object_find(sd_bus *bus, const char *path, const char *interface, void return 0; r = parse_uid(p, &uid); - } - if (r < 0) - return 0; + if (r < 0) + return 0; - user = hashmap_get(m->users, UID_TO_PTR(uid)); - if (!user) - return 0; + user = hashmap_get(m->users, UID_TO_PTR(uid)); + if (!user) + return 0; + } *found = user; return 1; From f3c33b234d9f0256805722f02c7b4c4b59fd6de6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 4 Jan 2018 11:36:58 +0100 Subject: [PATCH 46/46] networkd: fix memory corruption When loading .netdev files we parse them twice: first we do one parsing iteration to figure out their "kind", and then we do it again to parse out the kind's parameters. The first iteration is run with a "short" NetDev structure, that only covers the generic NetDev properties. Which should be enough, as we don't parse the per-kind properties. However, before this patch we'd still try to destruct the per-kind properties which resulted in memory corruption. With this change we distuingish the two iterations by the state field, so that the destruction only happens when the state signals we are running with a full NetDev structure. Since this is not obvious, let's add a lot of comments. --- src/network/netdev/netdev.c | 18 +++++++++++++----- src/network/netdev/netdev.h | 5 ++++- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/network/netdev/netdev.c b/src/network/netdev/netdev.c index 7cf110672f2..0bfe49006b9 100644 --- a/src/network/netdev/netdev.c +++ b/src/network/netdev/netdev.c @@ -153,7 +153,15 @@ static void netdev_free(NetDev *netdev) { condition_free_list(netdev->match_kernel_version); condition_free_list(netdev->match_arch); - if (NETDEV_VTABLE(netdev) && + /* Invoke the per-kind done() destructor, but only if the state field is initialized. We conditionalize that + * because we parse .netdev files twice: once to determine the kind (with a short, minimal NetDev structure + * allocation, with no room for per-kind fields), and once to read the kind's properties (with a full, + * comprehensive NetDev structure allocation with enough space for whatever the specific kind needs). Now, in + * the first case we shouldn't try to destruct the per-kind NetDev fields on destruction, in the second case we + * should. We use the state field to discern the two cases: it's _NETDEV_STATE_INVALID on the first "raw" + * call. */ + if (netdev->state != _NETDEV_STATE_INVALID && + NETDEV_VTABLE(netdev) && NETDEV_VTABLE(netdev)->done) NETDEV_VTABLE(netdev)->done(netdev); @@ -615,8 +623,8 @@ static int netdev_load_one(Manager *manager, const char *filename) { if (!file) { if (errno == ENOENT) return 0; - else - return -errno; + + return -errno; } if (null_or_empty_fd(fileno(file))) { @@ -630,7 +638,7 @@ static int netdev_load_one(Manager *manager, const char *filename) { netdev_raw->n_ref = 1; netdev_raw->kind = _NETDEV_KIND_INVALID; - netdev_raw->state = _NETDEV_STATE_INVALID; + netdev_raw->state = _NETDEV_STATE_INVALID; /* an invalid state means done() of the implementation won't be called on destruction */ dropin_dirname = strjoina(basename(filename), ".d"); r = config_parse_many(filename, network_dirs, dropin_dirname, @@ -669,7 +677,7 @@ static int netdev_load_one(Manager *manager, const char *filename) { netdev->n_ref = 1; netdev->manager = manager; netdev->kind = netdev_raw->kind; - netdev->state = _NETDEV_STATE_INVALID; + netdev->state = NETDEV_STATE_LOADING; /* we initialize the state here for the first time, so that done() will be called on destruction */ if (NETDEV_VTABLE(netdev)->init) NETDEV_VTABLE(netdev)->init(netdev); diff --git a/src/network/netdev/netdev.h b/src/network/netdev/netdev.h index 507b86a078f..7ae064ba8f9 100644 --- a/src/network/netdev/netdev.h +++ b/src/network/netdev/netdev.h @@ -65,6 +65,7 @@ typedef enum NetDevKind { } NetDevKind; typedef enum NetDevState { + NETDEV_STATE_LOADING, NETDEV_STATE_FAILED, NETDEV_STATE_CREATING, NETDEV_STATE_READY, @@ -149,7 +150,9 @@ extern const NetDevVTable * const netdev_vtable[_NETDEV_KIND_MAX]; /* For casting a netdev into the various netdev kinds */ #define DEFINE_NETDEV_CAST(UPPERCASE, MixedCase) \ static inline MixedCase* UPPERCASE(NetDev *n) { \ - if (_unlikely_(!n || n->kind != NETDEV_KIND_##UPPERCASE)) \ + if (_unlikely_(!n || \ + n->kind != NETDEV_KIND_##UPPERCASE) || \ + n->state == _NETDEV_STATE_INVALID) \ return NULL; \ \ return (MixedCase*) n; \