From 59f5836e71e8b8c9314d0b705049692fe886ed33 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 14 Feb 2025 14:33:48 +0100 Subject: [PATCH 01/11] sd-varlink: add new sd_varlink_get_n_fds() helper This new call returns how many fds have been associated with the current incoming message. --- src/libsystemd/libsystemd.sym | 1 + src/libsystemd/sd-varlink/sd-varlink.c | 9 +++++++++ src/systemd/sd-varlink.h | 1 + 3 files changed, 11 insertions(+) diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index 726a6bce12a..07553359df9 100644 --- a/src/libsystemd/libsystemd.sym +++ b/src/libsystemd/libsystemd.sym @@ -1069,6 +1069,7 @@ global: sd_varlink_get_current_method; sd_varlink_get_description; sd_varlink_get_input_fd; + sd_varlink_get_n_fds; sd_varlink_get_output_fd; sd_varlink_reset_fds; sd_varlink_server_listen_name; diff --git a/src/libsystemd/sd-varlink/sd-varlink.c b/src/libsystemd/sd-varlink/sd-varlink.c index d2ece7b2e6d..cdfcb9935e8 100644 --- a/src/libsystemd/sd-varlink/sd-varlink.c +++ b/src/libsystemd/sd-varlink/sd-varlink.c @@ -3183,6 +3183,15 @@ _public_ int sd_varlink_take_fd(sd_varlink *v, size_t i) { return TAKE_FD(v->input_fds[i]); } +_public_ int sd_varlink_get_n_fds(sd_varlink *v) { + assert_return(v, -EINVAL); + + if (!v->allow_fd_passing_input) + return -EPERM; + + return (int) v->n_input_fds; +} + static int verify_unix_socket(sd_varlink *v) { assert(v); diff --git a/src/systemd/sd-varlink.h b/src/systemd/sd-varlink.h index 528c20e8295..27e63a7fba1 100644 --- a/src/systemd/sd-varlink.h +++ b/src/systemd/sd-varlink.h @@ -195,6 +195,7 @@ int sd_varlink_reset_fds(sd_varlink *v); int sd_varlink_peek_fd(sd_varlink *v, size_t i); int sd_varlink_peek_dup_fd(sd_varlink *v, size_t i); int sd_varlink_take_fd(sd_varlink *v, size_t i); +int sd_varlink_get_n_fds(sd_varlink *v); int sd_varlink_set_allow_fd_passing_input(sd_varlink *v, int b); int sd_varlink_set_allow_fd_passing_output(sd_varlink *v, int b); From 7f6af95dab037e7d15591a924dbf256460bbf069 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 14 Feb 2025 14:40:31 +0100 Subject: [PATCH 02/11] notify-recv: add generic implementation of sd_notify() server side dgram recv code The code is not trivial, and we implemented this at various places already, introduce a common implementation for this we can reuse. --- src/shared/meson.build | 1 + src/shared/notify-recv.c | 106 +++++++++++++++++++++++++++++++++++++++ src/shared/notify-recv.h | 8 +++ 3 files changed, 115 insertions(+) create mode 100644 src/shared/notify-recv.c create mode 100644 src/shared/notify-recv.h diff --git a/src/shared/meson.build b/src/shared/meson.build index ed7cad7c883..0459975005e 100644 --- a/src/shared/meson.build +++ b/src/shared/meson.build @@ -126,6 +126,7 @@ shared_sources = files( 'netif-naming-scheme.c', 'netif-sriov.c', 'netif-util.c', + 'notify-recv.c', 'nsflags.c', 'nsresource.c', 'numa-util.c', diff --git a/src/shared/notify-recv.c b/src/shared/notify-recv.c new file mode 100644 index 00000000000..c3596d0258a --- /dev/null +++ b/src/shared/notify-recv.c @@ -0,0 +1,106 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "fd-util.h" +#include "notify-recv.h" +#include "socket-util.h" + +int notify_recv(int fd, char **ret_text, struct ucred *ret_ucred, PidRef *ret_pidref) { + CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct ucred)) + + CMSG_SPACE(sizeof(int)) + /* SCM_PIDFD */ + CMSG_SPACE(sizeof(int) * NOTIFY_FD_MAX)) control; + struct iovec iovec; + struct msghdr msghdr = { + .msg_iov = &iovec, + .msg_iovlen = 1, + .msg_control = &control, + .msg_controllen = sizeof(control), + }; + ssize_t n; + + assert(fd >= 0); + + /* Receives a $NOTIFY_SOCKET message (aka sd_notify()). Does various validations. Returns -EAGAIN in + * case an invalid message is received (following the logic that an invalid message shall be ignored, + * and treated like no message at all). */ + + _cleanup_free_ char *buf = new(char, NOTIFY_BUFFER_MAX+1); + if (!buf) + return log_oom_debug(); + + iovec = (struct iovec) { + .iov_base = buf, + .iov_len = NOTIFY_BUFFER_MAX, + }; + + n = recvmsg_safe(fd, &msghdr, MSG_DONTWAIT|MSG_CMSG_CLOEXEC); + if (ERRNO_IS_NEG_TRANSIENT(n)) + return -EAGAIN; + if (n == -ECHRNG) { + log_debug_errno(n, "Got message with truncated control data (unexpected fds sent?), ignoring."); + return -EAGAIN; + } + if (n == -EXFULL) { + log_debug_errno(n, "Got message with truncated payload data, ignoring."); + return -EAGAIN; + } + if (n < 0) + return (int) n; + + const struct ucred *ucred = NULL; + _cleanup_close_ int pidfd = -EBADF; + struct cmsghdr *cmsg; + CMSG_FOREACH(cmsg, &msghdr) { + if (cmsg->cmsg_level != SOL_SOCKET) + continue; + + switch (cmsg->cmsg_type) { + + case SCM_RIGHTS: + /* For now, just close every fd */ + close_many(CMSG_TYPED_DATA(cmsg, int), + (cmsg->cmsg_len - CMSG_LEN(0)) / sizeof(int)); + break; + + case SCM_PIDFD: + assert(cmsg->cmsg_len == CMSG_LEN(sizeof(int))); + pidfd = *CMSG_TYPED_DATA(cmsg, int); + break; + + case SCM_CREDENTIALS: + assert(cmsg->cmsg_len == CMSG_LEN(sizeof(struct ucred))); + ucred = CMSG_TYPED_DATA(cmsg, struct ucred); + break; + } + } + + if ((ret_ucred || ret_pidref) && (!ucred || ucred->pid <= 0)) + return log_debug_errno(SYNTHETIC_ERRNO(EAGAIN), "Got notification datagram lacking valid credential information, ignoring."); + + if (n == 0) + return log_debug_errno(SYNTHETIC_ERRNO(EAGAIN), "Got empty notification message, ignoring."); + if (memchr(buf, 0, n - 1)) + return log_debug_errno(SYNTHETIC_ERRNO(EAGAIN), "Got notification message with embedded NUL, ignoring."); + + if (ret_pidref) { + assert(ucred); + assert(ucred->pid > 0); + + if (pidfd >= 0) + *ret_pidref = (PidRef) { + .pid = ucred->pid, + .fd = TAKE_FD(pidfd), + }; + else + *ret_pidref = PIDREF_MAKE_FROM_PID(ucred->pid); + } + + if (ret_text) { + buf[n] = 0; + *ret_text = TAKE_PTR(buf); + } + + if (ret_ucred) + *ret_ucred = *ucred; + + return 0; +} diff --git a/src/shared/notify-recv.h b/src/shared/notify-recv.h new file mode 100644 index 00000000000..5cd5062b35d --- /dev/null +++ b/src/shared/notify-recv.h @@ -0,0 +1,8 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +#pragma once + +#include + +#include "pidref.h" + +int notify_recv(int fd, char **ret_text, struct ucred *ret_ucred, PidRef *ret_pidref); From 41e49f37247211f6bdaae7899692bffcf214ad9d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 14 Feb 2025 14:41:23 +0100 Subject: [PATCH 03/11] importd: port to notify_recv() --- src/import/importd.c | 50 +++++++++++--------------------------------- 1 file changed, 12 insertions(+), 38 deletions(-) diff --git a/src/import/importd.c b/src/import/importd.c index 414994ee300..8e1fe51bf8d 100644 --- a/src/import/importd.c +++ b/src/import/importd.c @@ -28,6 +28,7 @@ #include "main-func.h" #include "missing_capability.h" #include "mkdir-label.h" +#include "notify-recv.h" #include "os-util.h" #include "parse-util.h" #include "path-util.h" @@ -637,56 +638,25 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_unref); static int manager_on_notify(sd_event_source *s, int fd, uint32_t revents, void *userdata) { Manager *m = ASSERT_PTR(userdata); - char buf[NOTIFY_BUFFER_MAX+1]; - struct iovec iovec = { - .iov_base = buf, - .iov_len = sizeof(buf)-1, - }; - CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct ucred)) + - CMSG_SPACE(sizeof(int) * NOTIFY_FD_MAX)) control; - struct msghdr msghdr = { - .msg_iov = &iovec, - .msg_iovlen = 1, - .msg_control = &control, - .msg_controllen = sizeof(control), - }; - ssize_t n; int r; - n = recvmsg_safe(fd, &msghdr, MSG_DONTWAIT|MSG_CMSG_CLOEXEC); - if (ERRNO_IS_NEG_TRANSIENT(n)) + _cleanup_free_ char *buf = NULL; + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; + r = notify_recv(fd, &buf, /* ret_ucred= */ NULL, &pidref); + if (r == -EAGAIN) return 0; - if (n == -ECHRNG) { - log_warning_errno(n, "Got message with truncated control data (unexpected fds sent?), ignoring."); - return 0; - } - if (n == -EXFULL) { - log_warning_errno(n, "Got message with truncated payload data, ignoring."); - return 0; - } - if (n < 0) - return (int) n; - - cmsg_close_all(&msghdr); - - struct ucred *ucred = CMSG_FIND_DATA(&msghdr, SOL_SOCKET, SCM_CREDENTIALS, struct ucred); - if (!ucred || ucred->pid <= 0) { - log_warning("Got notification datagram lacking credential information, ignoring."); - return 0; - } + if (r < 0) + return log_warning_errno(r, "Failed to receive notification message: %m"); Transfer *t; HASHMAP_FOREACH(t, m->transfers) - if (ucred->pid == t->pidref.pid) + if (pidref_equal(&pidref, &t->pidref)) break; - if (!t) { log_warning("Got notification datagram from unexpected peer, ignoring."); return 0; } - buf[n] = 0; - char *p = find_line_startswith(buf, "X_IMPORT_PROGRESS="); if (!p) return 0; @@ -761,6 +731,10 @@ static int manager_new(Manager **ret) { if (r < 0) return r; + r = setsockopt_int(m->notify_fd, SOL_SOCKET, SO_PASSPIDFD, true); + if (r < 0) + log_debug_errno(r, "Failed to enable SO_PASSPIDFD on notification socket, ignoring. %m"); + r = sd_event_add_io(m->event, &m->notify_event_source, m->notify_fd, EPOLLIN, manager_on_notify, m); if (r < 0) From ee26ad44d742aa02f59ff26f7cf989236c25d249 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 17 Feb 2025 11:44:40 +0100 Subject: [PATCH 04/11] sysupdate: port to notify_recv() --- src/sysupdate/sysupdate-transfer.c | 64 ++++++++++-------------------- 1 file changed, 20 insertions(+), 44 deletions(-) diff --git a/src/sysupdate/sysupdate-transfer.c b/src/sysupdate/sysupdate-transfer.c index f0e8ad3b36c..f3dea04314b 100644 --- a/src/sysupdate/sysupdate-transfer.c +++ b/src/sysupdate/sysupdate-transfer.c @@ -15,6 +15,7 @@ #include "hexdecoct.h" #include "install-file.h" #include "mkdir.h" +#include "notify-recv.h" #include "parse-helpers.h" #include "parse-util.h" #include "percent-util.h" @@ -28,11 +29,11 @@ #include "stdio-util.h" #include "strv.h" #include "sync-util.h" +#include "sysupdate.h" #include "sysupdate-feature.h" #include "sysupdate-pattern.h" #include "sysupdate-resource.h" #include "sysupdate-transfer.h" -#include "sysupdate.h" #include "tmpfile-util.h" #include "web-util.h" @@ -977,56 +978,25 @@ static int helper_on_exit(sd_event_source *s, const siginfo_t *si, void *userdat } static int helper_on_notify(sd_event_source *s, int fd, uint32_t revents, void *userdata) { - char buf[NOTIFY_BUFFER_MAX+1]; - struct iovec iovec = { - .iov_base = buf, - .iov_len = sizeof(buf)-1, - }; - CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct ucred))) control; - struct msghdr msghdr = { - .msg_iov = &iovec, - .msg_iovlen = 1, - .msg_control = &control, - .msg_controllen = sizeof(control), - }; - struct ucred *ucred; CalloutContext *ctx = ASSERT_PTR(userdata); - char *progress_str, *errno_str; - int progress; - ssize_t n; int r; - n = recvmsg_safe(fd, &msghdr, MSG_DONTWAIT|MSG_CMSG_CLOEXEC); - if (ERRNO_IS_NEG_TRANSIENT(n)) - return 0; - if (n == -ECHRNG) { - log_warning_errno(n, "Got message with truncated control data (unexpected fds sent?), ignoring."); - return 0; - } - if (n == -EXFULL) { - log_warning_errno(n, "Got message with truncated payload data, ignoring."); - return 0; - } - if (n < 0) - return (int) n; + assert(fd >= 0); - cmsg_close_all(&msghdr); - - ucred = CMSG_FIND_DATA(&msghdr, SOL_SOCKET, SCM_CREDENTIALS, struct ucred); - if (!ucred || ucred->pid <= 0) { - log_warning("Got notification datagram lacking credential information, ignoring."); + _cleanup_free_ char *buf = NULL; + _cleanup_(pidref_done) PidRef sender_pid = PIDREF_NULL; + r = notify_recv(fd, &buf, /* ret_ucred= */ NULL, &sender_pid); + if (r == -EAGAIN) return 0; - } - if (ucred->pid != ctx->pid.pid) { + if (r < 0) + return log_warning_errno(r, "Failed to receive notification message: %m"); + + if (!pidref_equal(&ctx->pid, &sender_pid)) { log_warning("Got notification datagram from unexpected peer, ignoring."); return 0; } - buf[n] = 0; - - progress_str = find_line_startswith(buf, "X_IMPORT_PROGRESS="); - errno_str = find_line_startswith(buf, "ERRNO="); - + char *errno_str = find_line_startswith(buf, "ERRNO="); if (errno_str) { truncate_nl(errno_str); r = parse_errno(errno_str); @@ -1038,9 +1008,11 @@ static int helper_on_notify(sd_event_source *s, int fd, uint32_t revents, void * } } + char *progress_str = find_line_startswith(buf, "X_IMPORT_PROGRESS="); if (progress_str) { truncate_nl(progress_str); - progress = parse_percent(progress_str); + + int progress = parse_percent(progress_str); if (progress < 0) log_warning("Got invalid percent value '%s', ignoring.", progress_str); else { @@ -1105,7 +1077,11 @@ static int run_callout( r = setsockopt_int(fd, SOL_SOCKET, SO_PASSCRED, true); if (r < 0) - return log_error_errno(r, "Failed to set socket options: %m"); + return log_error_errno(r, "Failed to enable SO_PASSCRED: %m"); + + r = setsockopt_int(fd, SOL_SOCKET, SO_PASSPIDFD, true); + if (r < 0) + log_debug_errno(r, "Failed to enable SO_PASSPIDFD, ignoring: %m"); r = pidref_safe_fork(ctx->name, FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGTERM|FORK_LOG, &ctx->pid); if (r < 0) From cc19df7c76b16b6ac8fc95d7386b2441baf3ded7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 17 Feb 2025 11:50:15 +0100 Subject: [PATCH 05/11] sysupdated: port to notify_recv() --- src/libsystemd/sd-event/event-util.c | 25 ++++++++++ src/libsystemd/sd-event/event-util.h | 2 + src/sysupdate/sysupdated.c | 75 ++++++++++------------------ 3 files changed, 54 insertions(+), 48 deletions(-) diff --git a/src/libsystemd/sd-event/event-util.c b/src/libsystemd/sd-event/event-util.c index 42e973564e3..482df37225c 100644 --- a/src/libsystemd/sd-event/event-util.c +++ b/src/libsystemd/sd-event/event-util.c @@ -2,6 +2,7 @@ #include +#include "errno-util.h" #include "event-source.h" #include "event-util.h" #include "fd-util.h" @@ -172,6 +173,30 @@ int event_add_child_pidref( return sd_event_add_child(e, s, pid->pid, options, callback, userdata); } +int event_source_get_child_pidref(sd_event_source *s, PidRef *ret) { + int r; + + assert(s); + assert(ret); + + pid_t pid; + r = sd_event_source_get_child_pid(s, &pid); + if (r < 0) + return r; + + int pidfd = sd_event_source_get_child_pidfd(s); + if (pidfd < 0) + return pidfd; + + /* Note, we don't actually duplicate the fd here, i.e. we do not pass ownership of this PidRef to the caller */ + *ret = (PidRef) { + .pid = pid, + .fd = pidfd, + }; + + return 0; +} + dual_timestamp* event_dual_timestamp_now(sd_event *e, dual_timestamp *ts) { assert(e); assert(ts); diff --git a/src/libsystemd/sd-event/event-util.h b/src/libsystemd/sd-event/event-util.h index 383b4c82b7c..2c6fcccb908 100644 --- a/src/libsystemd/sd-event/event-util.h +++ b/src/libsystemd/sd-event/event-util.h @@ -37,6 +37,8 @@ int event_add_time_change(sd_event *e, sd_event_source **ret, sd_event_io_handle int event_add_child_pidref(sd_event *e, sd_event_source **s, const PidRef *pid, int options, sd_event_child_handler_t callback, void *userdata); +int event_source_get_child_pidref(sd_event_source *s, PidRef *ret); + dual_timestamp* event_dual_timestamp_now(sd_event *e, dual_timestamp *ts); void event_source_unref_many(sd_event_source **array, size_t n); diff --git a/src/sysupdate/sysupdated.c b/src/sysupdate/sysupdated.c index 58967930f77..1083345374c 100644 --- a/src/sysupdate/sysupdated.c +++ b/src/sysupdate/sysupdated.c @@ -24,6 +24,7 @@ #include "main-func.h" #include "memfd-util.h" #include "mkdir-label.h" +#include "notify-recv.h" #include "os-util.h" #include "process-util.h" #include "service-util.h" @@ -1645,65 +1646,39 @@ static Manager *manager_free(Manager *m) { DEFINE_TRIVIAL_CLEANUP_FUNC(Manager *, manager_free); static int manager_on_notify(sd_event_source *s, int fd, uint32_t revents, void *userdata) { - char buf[NOTIFY_BUFFER_MAX+1]; - struct iovec iovec = { - .iov_base = buf, - .iov_len = sizeof(buf)-1, - }; - CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct ucred))) control; - struct msghdr msghdr = { - .msg_iov = &iovec, - .msg_iovlen = 1, - .msg_control = &control, - .msg_controllen = sizeof(control), - }; - struct ucred *ucred; Manager *m = ASSERT_PTR(userdata); + int r; + + assert(fd >= 0); + + _cleanup_(pidref_done) PidRef sender_pidref = PIDREF_NULL; + _cleanup_free_ char *buf = NULL; + r = notify_recv(fd, &buf, /* ret_ucred= */ NULL, &sender_pidref); + if (r == -EAGAIN) + return 0; + if (r < 0) + return log_warning_errno(r, "Failed to receive notification message: %m"); + Job *j; - ssize_t n; - char *version, *progress, *errno_str, *ready; - - n = recvmsg_safe(fd, &msghdr, MSG_DONTWAIT|MSG_CMSG_CLOEXEC); - if (ERRNO_IS_NEG_TRANSIENT(n)) - return 0; - if (n == -ECHRNG) { - log_warning_errno(n, "Got message with truncated control data (unexpected fds sent?), ignoring."); - return 0; - } - if (n == -EXFULL) { - log_warning_errno(n, "Got message with truncated payload data, ignoring."); - return 0; - } - if (n < 0) - return (int) n; - - cmsg_close_all(&msghdr); - - ucred = CMSG_FIND_DATA(&msghdr, SOL_SOCKET, SCM_CREDENTIALS, struct ucred); - if (!ucred || ucred->pid <= 0) { - log_warning("Got notification datagram lacking credential information, ignoring."); - return 0; - } - HASHMAP_FOREACH(j, m->jobs) { - pid_t pid; - assert_se(sd_event_source_get_child_pid(j->child, &pid) >= 0); + PidRef child_pidref = PIDREF_NULL; - if (ucred->pid == pid) + r = event_source_get_child_pidref(j->child, &child_pidref); + if (r < 0) + return log_warning_errno(r, "Failed to get child pidref: %m"); + + if (pidref_equal(&sender_pidref, &child_pidref)) break; } - if (!j) { log_warning("Got notification datagram from unexpected peer, ignoring."); return 0; } - buf[n] = 0; - - version = find_line_startswith(buf, "X_SYSUPDATE_VERSION="); - progress = find_line_startswith(buf, "X_SYSUPDATE_PROGRESS="); - errno_str = find_line_startswith(buf, "ERRNO="); - ready = find_line_startswith(buf, "READY=1"); + char *version = find_line_startswith(buf, "X_SYSUPDATE_VERSION="); + char *progress = find_line_startswith(buf, "X_SYSUPDATE_PROGRESS="); + char *errno_str = find_line_startswith(buf, "ERRNO="); + const char *ready = find_line_startswith(buf, "READY=1"); if (version) job_on_version(j, truncate_nl(version)); @@ -1777,6 +1752,10 @@ static int manager_new(Manager **ret) { if (r < 0) return r; + r = setsockopt_int(notify_fd, SOL_SOCKET, SO_PASSPIDFD, true); + if (r < 0) + log_debug_errno(r, "Failed to enable SO_PASSPIDFD, ignoring: %m"); + r = sd_event_add_io(m->event, &m->notify_event, notify_fd, EPOLLIN, manager_on_notify, m); if (r < 0) return r; From 060c4b99bd76da157fa419f07af22ec1867ae17c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 17 Feb 2025 12:00:19 +0100 Subject: [PATCH 06/11] nspawn: port to notify_recv() --- src/nspawn/nspawn.c | 61 ++++++++++++--------------------------------- 1 file changed, 16 insertions(+), 45 deletions(-) diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index bcf0baaf7fb..4ba3694280b 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -68,6 +68,7 @@ #include "mount-util.h" #include "mountpoint-util.h" #include "namespace-util.h" +#include "notify-recv.h" #include "nspawn-bind-user.h" #include "nspawn-cgroup.h" #include "nspawn-expose-ports.h" @@ -3751,7 +3752,11 @@ static int setup_notify_child(const void *directory) { r = setsockopt_int(fd, SOL_SOCKET, SO_PASSCRED, true); if (r < 0) - return log_error_errno(r, "SO_PASSCRED failed: %m"); + return log_error_errno(r, "Failed to enable SO_PASSCRED: %m"); + + r = setsockopt_int(fd, SOL_SOCKET, SO_PASSPIDFD, true); + if (r < 0) + log_debug_errno(r, "Failed to enable SO_PASSPIDFD, ignoring: %m"); return TAKE_FD(fd); } @@ -4610,59 +4615,25 @@ static int setup_uid_map( } static int nspawn_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t revents, void *userdata) { - char buf[NOTIFY_BUFFER_MAX+1]; - char *p = NULL; - struct iovec iovec = { - .iov_base = buf, - .iov_len = sizeof(buf)-1, - }; - CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct ucred)) + - CMSG_SPACE(sizeof(int) * NOTIFY_FD_MAX)) control; - struct msghdr msghdr = { - .msg_iov = &iovec, - .msg_iovlen = 1, - .msg_control = &control, - .msg_controllen = sizeof(control), - }; - struct ucred *ucred; - ssize_t n; - pid_t inner_child_pid; - _cleanup_strv_free_ char **tags = NULL; + pid_t inner_child_pid = PTR_TO_PID(userdata); int r; assert(userdata); - inner_child_pid = PTR_TO_PID(userdata); - - if (revents != EPOLLIN) { - log_warning("Got unexpected poll event for notify fd."); + _cleanup_(pidref_done) PidRef sender_pid = PIDREF_NULL; + _cleanup_free_ char *buf = NULL; + r = notify_recv(fd, &buf, /* ret_ucred= */ NULL, &sender_pid); + if (r == -EAGAIN) return 0; - } + if (r < 0) + return log_error_errno(r, "Failed to receive notification message: %m"); - n = recvmsg_safe(fd, &msghdr, MSG_DONTWAIT|MSG_CMSG_CLOEXEC); - if (ERRNO_IS_NEG_TRANSIENT(n)) - return 0; - if (n == -ECHRNG) { - log_warning_errno(n, "Got message with truncated control data (too many fds sent?), ignoring."); - return 0; - } - if (n == -EXFULL) { - log_warning_errno(n, "Got message with truncated payload data, ignoring."); - return 0; - } - if (n < 0) - return log_warning_errno(n, "Couldn't read notification socket: %m"); - - cmsg_close_all(&msghdr); - - ucred = CMSG_FIND_DATA(&msghdr, SOL_SOCKET, SCM_CREDENTIALS, struct ucred); - if (!ucred || ucred->pid != inner_child_pid) { + if (sender_pid.pid != inner_child_pid) { log_debug("Received notify message from process that is not the payload's PID 1. Ignoring."); return 0; } - buf[n] = 0; - tags = strv_split(buf, "\n\r"); + _cleanup_strv_free_ char **tags = strv_split(buf, "\n\r"); if (!tags) return log_oom(); @@ -4683,7 +4654,7 @@ static int nspawn_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t r log_warning_errno(r, "Failed to send readiness notification, ignoring: %m"); } - p = strv_find_startswith(tags, "STATUS="); + char *p = strv_find_startswith(tags, "STATUS="); if (p) (void) sd_notifyf(false, "STATUS=Container running: %s", p); From 30999dd5cfe87d9b2d3e04d0e44849c5e717f4e8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 14 Feb 2025 14:43:02 +0100 Subject: [PATCH 07/11] notify: introduce arg_action enum --- src/notify/notify.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/notify/notify.c b/src/notify/notify.c index 1c5b5e7be3d..afa5923a729 100644 --- a/src/notify/notify.c +++ b/src/notify/notify.c @@ -25,12 +25,15 @@ #include "time-util.h" #include "user-util.h" +static enum { + ACTION_NOTIFY, + ACTION_BOOTED, +} arg_action = ACTION_NOTIFY; static bool arg_ready = false; static bool arg_reloading = false; static bool arg_stopping = false; static PidRef arg_pid = PIDREF_NULL; static const char *arg_status = NULL; -static bool arg_booted = false; static uid_t arg_uid = UID_INVALID; static gid_t arg_gid = GID_INVALID; static bool arg_no_block = false; @@ -231,7 +234,7 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_BOOTED: - arg_booted = true; + arg_action = ACTION_BOOTED; break; case ARG_UID: { @@ -338,7 +341,7 @@ static int parse_argv(int argc, char *argv[]) { have_env = have_env || n_arg_env > 0; - if (!have_env && !arg_booted) { + if (!have_env && arg_action != ACTION_BOOTED) { if (do_exec) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "No notify message specified while --exec, refusing."); @@ -347,7 +350,7 @@ static int parse_argv(int argc, char *argv[]) { return -EINVAL; } - if (have_env && arg_booted) + if (have_env && arg_action == ACTION_BOOTED) log_warning("Notify message specified along with --booted, ignoring."); if (n_arg_env > 0) { @@ -376,7 +379,7 @@ static int run(int argc, char* argv[]) { if (r <= 0) return r; - if (arg_booted) { + if (arg_action == ACTION_BOOTED) { r = sd_booted(); if (r < 0) log_debug_errno(r, "Failed to determine whether we are booted with systemd, assuming we aren't: %m"); From 4389e4c2aea36224e99dae616aff08c8560c94e8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 14 Feb 2025 14:43:22 +0100 Subject: [PATCH 08/11] notify: add a new --fork verb that implements a minimal receiver side for sd_notify() messages --- man/systemd-notify.xml | 53 ++++++- src/notify/notify.c | 311 ++++++++++++++++++++++++++++++++++++----- 2 files changed, 327 insertions(+), 37 deletions(-) diff --git a/man/systemd-notify.xml b/man/systemd-notify.xml index 9a66721a613..25b5e2d041f 100644 --- a/man/systemd-notify.xml +++ b/man/systemd-notify.xml @@ -26,7 +26,10 @@ systemd-notify OPTIONS VARIABLE=VALUE - systemd-notify --exec OPTIONS VARIABLE=VALUE ; CMDLINE + systemd-notify --exec OPTIONS VARIABLE=VALUE ; -- CMDLINE + + + systemd-notify --fork OPTIONS -- CMDLINE @@ -237,6 +240,54 @@ + + + + Instead of sending a notification message, fork off a command line and wait until a + READY=1 message is received from it. In other words: this makes + systemd-notify the receiver of notification messages instead of the sender, + swapping roles. This is useful to quickly fork off a process that implements the + sd_notify() protocol from a shell script. The invoked command line will have + standard input and standard output connected to /dev/null, but standard error + will be inherited from the invoking process. The numeric process ID is written to standard output by + systemd-notify (unless is specified), which may be used + to later terminate the forked off process. + + Note that processes forked off like this will likely remain running after + systemd-notify already returned, which hence will result in them being reparented + to the closest process reaper process, i.e. typically the per-user or system service manager. + + Note that this option should not be used to invoke full services ad-hoc, use + systemd-run for that. + + Also note that when invoked with this switch systemd-notify will exit + successfully under two distinction conditions: + + + systemd-notify received a READY=1 + notification from the child it just forked off. + + The child process exited cleanly (with exit status zero) before sending + READY=1. + + + Example use:# PID=$(systemd-notify --fork -- mycommand) +… +kill "$PID" +unset PID + + + + + + + + + Turn off output of the numeric process ID when is used. + + + + diff --git a/src/notify/notify.c b/src/notify/notify.c index afa5923a729..a2c53005aa9 100644 --- a/src/notify/notify.c +++ b/src/notify/notify.c @@ -7,18 +7,24 @@ #include #include "sd-daemon.h" +#include "sd-event.h" #include "alloc-util.h" #include "build.h" #include "env-util.h" +#include "escape.h" +#include "event-util.h" +#include "exit-status.h" #include "fd-util.h" #include "fdset.h" #include "format-util.h" #include "log.h" #include "main-func.h" +#include "notify-recv.h" #include "parse-util.h" #include "pretty-print.h" #include "process-util.h" +#include "socket-util.h" #include "string-util.h" #include "strv.h" #include "terminal-util.h" @@ -28,6 +34,7 @@ static enum { ACTION_NOTIFY, ACTION_BOOTED, + ACTION_FORK, } arg_action = ACTION_NOTIFY; static bool arg_ready = false; static bool arg_reloading = false; @@ -41,6 +48,7 @@ static char **arg_env = NULL; static char **arg_exec = NULL; static FDSet *arg_fds = NULL; static char *arg_fdname = NULL; +static bool arg_quiet = false; STATIC_DESTRUCTOR_REGISTER(arg_pid, pidref_done); STATIC_DESTRUCTOR_REGISTER(arg_env, strv_freep); @@ -57,7 +65,8 @@ static int help(void) { return log_oom(); printf("%s [OPTIONS...] [VARIABLE=VALUE...]\n" - "%s [OPTIONS...] --exec [VARIABLE=VALUE...] ; CMDLINE...\n" + "%s [OPTIONS...] --exec [VARIABLE=VALUE...] ; -- CMDLINE...\n" + "%s [OPTIONS...] --fork -- CMDLINE...\n" "\n%sNotify the init system about service status updates.%s\n\n" " -h --help Show this help\n" " --version Show package version\n" @@ -73,9 +82,12 @@ static int help(void) { " --exec Execute command line separated by ';' once done\n" " --fd=FD Pass specified file descriptor with along with message\n" " --fdname=NAME Name to assign to passed file descriptor(s)\n" + " --fork Receive notifications from child rather than sending them\n" + " -q --quiet Do not show PID of child when forking\n" "\nSee the %s for details.\n", program_invocation_short_name, program_invocation_short_name, + program_invocation_short_name, ansi_highlight(), ansi_normal(), link); @@ -165,6 +177,7 @@ static int parse_argv(int argc, char *argv[]) { ARG_EXEC, ARG_FD, ARG_FDNAME, + ARG_FORK, }; static const struct option options[] = { @@ -181,6 +194,8 @@ static int parse_argv(int argc, char *argv[]) { { "exec", no_argument, NULL, ARG_EXEC }, { "fd", required_argument, NULL, ARG_FD }, { "fdname", required_argument, NULL, ARG_FDNAME }, + { "fork", no_argument, NULL, ARG_FORK }, + { "quiet", no_argument, NULL, 'q' }, {} }; @@ -191,7 +206,7 @@ static int parse_argv(int argc, char *argv[]) { assert(argc >= 0); assert(argv); - while ((c = getopt_long(argc, argv, "h", options, NULL)) >= 0) { + while ((c = getopt_long(argc, argv, "hq", options, NULL)) >= 0) { switch (c) { @@ -305,6 +320,14 @@ static int parse_argv(int argc, char *argv[]) { break; + case ARG_FORK: + arg_action = ACTION_FORK; + break; + + case 'q': + arg_quiet = true; + break; + case '?': return -EINVAL; @@ -313,58 +336,271 @@ static int parse_argv(int argc, char *argv[]) { } } - if (arg_fdname && fdset_isempty(arg_fds)) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "No file descriptors passed, but --fdname= set, refusing."); - bool have_env = arg_ready || arg_stopping || arg_reloading || arg_status || pidref_is_set(&arg_pid) || !fdset_isempty(arg_fds); - size_t n_arg_env; - if (do_exec) { - int i; + switch (arg_action) { - for (i = optind; i < argc; i++) - if (streq(argv[i], ";")) - break; + case ACTION_NOTIFY: { + if (arg_fdname && fdset_isempty(arg_fds)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "No file descriptors passed, but --fdname= set, refusing."); - if (i >= argc) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "If --exec is used argument list must contain ';' separator, refusing."); - if (i+1 == argc) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Empty command line specified after ';' separator, refusing."); + size_t n_arg_env; - arg_exec = strv_copy_n(argv + i + 1, argc - i - 1); - if (!arg_exec) - return log_oom(); + if (do_exec) { + int i; - n_arg_env = i - optind; - } else - n_arg_env = argc - optind; + for (i = optind; i < argc; i++) + if (streq(argv[i], ";")) + break; - have_env = have_env || n_arg_env > 0; + if (i >= argc) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "If --exec is used argument list must contain ';' separator, refusing."); + if (i+1 == argc) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Empty command line specified after ';' separator, refusing."); - if (!have_env && arg_action != ACTION_BOOTED) { - if (do_exec) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "No notify message specified while --exec, refusing."); + arg_exec = strv_copy_n(argv + i + 1, argc - i - 1); + if (!arg_exec) + return log_oom(); - /* No argument at all? */ - help(); - return -EINVAL; + n_arg_env = i - optind; + } else + n_arg_env = argc - optind; + + have_env = have_env || n_arg_env > 0; + if (!have_env) { + if (do_exec) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "No notify message specified while --exec, refusing."); + + /* No argument at all? */ + help(); + return -EINVAL; + } + + if (n_arg_env > 0) { + arg_env = strv_copy_n(argv + optind, n_arg_env); + if (!arg_env) + return log_oom(); + } + + if (!fdset_isempty(passed)) + log_warning("Warning: %u more file descriptors passed than referenced with --fd=.", fdset_size(passed)); + + break; } - if (have_env && arg_action == ACTION_BOOTED) - log_warning("Notify message specified along with --booted, ignoring."); + case ACTION_BOOTED: + if (argc > optind) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "--booted takes no parameters, refusing."); - if (n_arg_env > 0) { - arg_env = strv_copy_n(argv + optind, n_arg_env); - if (!arg_env) - return log_oom(); + break; + + case ACTION_FORK: + if (optind >= argc) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "--fork requires a command to be specified, refusing."); + + break; + + default: + assert_not_reached(); } - if (!fdset_isempty(passed)) - log_warning("Warning: %u more file descriptors passed than referenced with --fd=.", fdset_size(passed)); + if (have_env && arg_action != ACTION_NOTIFY) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "--ready, --reloading, --stopping, --pid=, --status=, --fd= may not be combined with --fork or --booted, refusing."); return 1; } +static int on_notify_socket(sd_event_source *s, int fd, unsigned event, void *userdata) { + PidRef *child = ASSERT_PTR(userdata); + int r; + + assert(s); + assert(fd >= 0); + + _cleanup_free_ char *text = NULL; + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; + r = notify_recv(fd, &text, /* ret_ucred= */ NULL, &pidref); + if (r == -EAGAIN) + return 0; + if (r < 0) + return log_error_errno(r, "Failed to receive notification message: %m"); + + if (!pidref_equal(child, &pidref)) { + log_warning("Received notification message from unexpected process " PID_FMT " (expected " PID_FMT "), ignoring.", + pidref.pid, child->pid); + return 0; + } + + const char *p = find_line_startswith(text, "READY=1"); + if (!p || !IN_SET(*p, '\n', 0)) { + if (!DEBUG_LOGGING) + return 0; + + _cleanup_free_ char *escaped = cescape(text); + log_debug("Received notification message without READY=1, ignoring: %s", strna(escaped)); + return 0; + } + + log_debug("Received READY=1, exiting."); + return sd_event_exit(sd_event_source_get_event(s), EXIT_SUCCESS); +} + +static int on_child(sd_event_source *s, const siginfo_t *si, void *userdata) { + assert(s); + assert(si); + + int ret; + if (si->si_code == CLD_EXITED) { + if (si->si_status != EXIT_SUCCESS) + log_debug("Child failed with exit status %i.", si->si_status); + else + log_debug("Child exited successfully. (But no READY=1 message was sent!)"); + + /* NB: we propagate success here if the child exited cleanly but never sent us READY=1. We + * are not a service manager after all, where this would be a protocol violation. We are just + * a shell tool to fork off stuff in the background, where I think it makes sense to allow + * clean early exit of forked off processes. */ + ret = si->si_status; + + } else if (IN_SET(si->si_code, CLD_KILLED, CLD_DUMPED)) + ret = log_debug_errno(SYNTHETIC_ERRNO(EPROTO), + "Child terminated by signal %s.", signal_to_string(si->si_status)); + else + ret = log_debug_errno(SYNTHETIC_ERRNO(EPROTO), + "Child terminated due to unknown reason."); + + return sd_event_exit(sd_event_source_get_event(s), ret); +} + +static int action_fork(char *const *_command) { + + static const int forward_signals[] = { + SIGHUP, + SIGTERM, + SIGINT, + SIGQUIT, + SIGTSTP, + SIGCONT, + SIGUSR1, + SIGUSR2, + }; + + int r; + + assert(!strv_isempty(_command)); + + /* Make a copy, since pidref_safe_fork_full() will change argv[] further down. */ + _cleanup_strv_free_ char **command = strv_copy(_command); + if (!command) + return log_oom(); + + _cleanup_close_ int socket_fd = socket(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC|SOCK_NONBLOCK, 0); + if (socket_fd < 0) + return log_error_errno(errno, "Failed to allocate AF_UNIX socket for notifications: %m"); + + r = setsockopt_int(socket_fd, SOL_SOCKET, SO_PASSCRED, true); + if (r < 0) + return log_error_errno(r, "Failed to enable SO_PASSCRED: %m"); + + r = setsockopt_int(socket_fd, SOL_SOCKET, SO_PASSPIDFD, true); + if (r < 0) + log_debug_errno(r, "Failed to enable SO_PASSPIDFD, ignoring: %m"); + + /* Pick an address via auto-bind */ + union sockaddr_union sa = { + .sa.sa_family = AF_UNIX, + }; + if (bind(socket_fd, &sa.sa, offsetof(union sockaddr_union, un.sun_path)) < 0) + return log_error_errno(errno, "Failed to bind AF_UNIX socket: %m"); + + _cleanup_free_ char *addr_string = NULL; + r = getsockname_pretty(socket_fd, &addr_string); + if (r < 0) + return log_error_errno(r, "Failed to get socket name: %m"); + + _cleanup_free_ char *c = strv_join(command, " "); + if (!c) + return log_oom(); + + _cleanup_(pidref_done) PidRef child = PIDREF_NULL; + r = pidref_safe_fork_full( + "(notify)", + /* stdio_fds= */ (const int[]) { -EBADF, -EBADF, STDERR_FILENO }, + /* except_fds= */ NULL, + /* n_except_fds= */ 0, + /* flags= */ FORK_REARRANGE_STDIO, + &child); + if (r < 0) + return log_error_errno(r, "Failed to fork child in order to execute '%s': %m", c); + if (r == 0) { + /* Let's explicitly close the fds we just opened. Not because it was necessary (we should be + * setting O_CLOEXEC after all on all of them), but mostly to make debugging nice */ + socket_fd = safe_close(socket_fd); + pidref_done(&child); + + if (setenv("NOTIFY_SOCKET", addr_string, /* overwrite= */ true) < 0) { + log_error_errno(errno, "Failed to set $NOTIFY_SOCKET: %m"); + _exit(EXIT_MEMORY); + } + + log_debug("Executing: %s", c); + execvp(command[0], command); + log_error_errno(errno, "Failed to execute '%s': %m", c); + _exit(EXIT_EXEC); + } + + if (!arg_quiet) { + printf(PID_FMT "\n", child.pid); + fflush(stdout); + } + + BLOCK_SIGNALS(SIGCHLD); + + _cleanup_(sd_event_unrefp) sd_event *event = NULL; + r = sd_event_new(&event); + if (r < 0) + return log_error_errno(r, "Failed to allocate event loop: %m"); + + _cleanup_(sd_event_source_disable_unrefp) sd_event_source *socket_event_source = NULL; + r = sd_event_add_io(event, &socket_event_source, socket_fd, EPOLLIN, on_notify_socket, &child); + if (r < 0) + return log_error_errno(r, "Failed to allocate IO source: %m"); + + /* If we receive both the sd_notify() message and a SIGCHLD always process sd_notify() first, it's + * the more interesting, "positive" information. */ + r = sd_event_source_set_priority(socket_event_source, SD_EVENT_PRIORITY_NORMAL - 10); + if (r < 0) + return log_error_errno(r, "Failed to change child event source priority: %m"); + + _cleanup_(sd_event_source_disable_unrefp) sd_event_source *child_event_source = NULL; + r = event_add_child_pidref(event, &child_event_source, &child, WEXITED, on_child, /* userdata= */ NULL); + if (r < 0) + return log_error_errno(r, "Failed to allocate child source: %m"); + + /* Handle SIGCHLD before propagating the other signals below */ + r = sd_event_source_set_priority(child_event_source, SD_EVENT_PRIORITY_NORMAL - 5); + if (r < 0) + return log_error_errno(r, "Failed to change child event source priority: %m"); + + sd_event_source **forward_signal_sources = NULL; + size_t n_forward_signal_sources = 0; + CLEANUP_ARRAY(forward_signal_sources, n_forward_signal_sources, event_source_unref_many); + + r = event_forward_signals( + event, + child_event_source, + forward_signals, ELEMENTSOF(forward_signals), + &forward_signal_sources, &n_forward_signal_sources); + if (r < 0) + return log_error_errno(r, "Failed to set up signal forwarding: %m"); + + r = sd_event_loop(event); + if (r < 0) + return log_error_errno(r, "Failed to run event loop: %m"); + + return r; +} + static int run(int argc, char* argv[]) { _cleanup_free_ char *status = NULL, *main_pid = NULL, *main_pidfd_id = NULL, *msg = NULL, *monotonic_usec = NULL, *fdn = NULL; @@ -379,6 +615,9 @@ static int run(int argc, char* argv[]) { if (r <= 0) return r; + if (arg_action == ACTION_FORK) + return action_fork(argv + optind); + if (arg_action == ACTION_BOOTED) { r = sd_booted(); if (r < 0) From fd2d435d9782a70e67d93f3dd012e0ec09804249 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 14 Feb 2025 14:35:18 +0100 Subject: [PATCH 09/11] varlinkctl: introduce new --exec switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This new switch makes it possible to process fds attached to a varlink reply: if specified it will execute a command line specified by the caller, will pass the response json as stdin, and any fds acquired as fd3, fd4, … --- man/varlinkctl.xml | 35 ++++++++- src/varlinkctl/varlinkctl.c | 138 ++++++++++++++++++++++++++++++++++-- 2 files changed, 168 insertions(+), 5 deletions(-) diff --git a/man/varlinkctl.xml b/man/varlinkctl.xml index 41d911c5115..37d8b6731a1 100644 --- a/man/varlinkctl.xml +++ b/man/varlinkctl.xml @@ -61,6 +61,18 @@ ARGUMENTS + + varlinkctl + OPTIONS + --exec call + call + ADDRESS + METHOD + ARGUMENTS + -- + CMDLINE + + varlinkctl OPTIONS @@ -307,6 +319,25 @@ + + + + + Once the method call issued via call completed successfully, chainload the + specified command line, with the method call output parameters serialized to JSON passed into + standard input (and standard output and standard error inherited from the invoking + process). Moreover any file descriptors passed back on the underlying communication socket are + passed to the invoked process via the usual $LISTEN_FDS protocol. This + functionality may be used to consume replies that come with associated file descriptors in a + reasonable way. + + Now that if is specified the the third parameter to + call is not optional (i.e. the method call parameters). + + + + + @@ -349,7 +380,9 @@ type ResolvedAddress( Invoking a Method - The following command resolves a hostname via systemd-resolved.service8's ResolveHostname method call. + The following command resolves a hostname via + systemd-resolved.service8's + ResolveHostname method call. $ varlinkctl call /run/systemd/resolve/io.systemd.Resolve io.systemd.Resolve.ResolveHostname '{"name":"systemd.io","family":2}' -j { diff --git a/src/varlinkctl/varlinkctl.c b/src/varlinkctl/varlinkctl.c index cecd6c90df8..978bed5b7f0 100644 --- a/src/varlinkctl/varlinkctl.c +++ b/src/varlinkctl/varlinkctl.c @@ -5,11 +5,13 @@ #include "sd-varlink.h" #include "build.h" +#include "env-util.h" #include "fd-util.h" #include "fileio.h" #include "format-table.h" #include "io-util.h" #include "main-func.h" +#include "memfd-util.h" #include "pager.h" #include "parse-argument.h" #include "path-util.h" @@ -27,6 +29,7 @@ static bool arg_collect = false; static bool arg_quiet = false; static char **arg_graceful = NULL; static usec_t arg_timeout = 0; +static bool arg_exec = false; STATIC_DESTRUCTOR_REGISTER(arg_graceful, strv_freep); @@ -53,6 +56,8 @@ static int help(void) { " Show interface definition\n" " call ADDRESS METHOD [PARAMS]\n" " Invoke method\n" + " --exec call ADDRESS METHOD PARAMS -- CMDLINE…\n" + " Invoke method and pass response and fds to command\n" " validate-idl [FILE] Validate interface description\n" " help Show this help\n" "\n%3$sOptions:%4$s\n" @@ -94,6 +99,7 @@ static int parse_argv(int argc, char *argv[]) { ARG_COLLECT, ARG_GRACEFUL, ARG_TIMEOUT, + ARG_EXEC, }; static const struct option options[] = { @@ -107,6 +113,7 @@ static int parse_argv(int argc, char *argv[]) { { "quiet", no_argument, NULL, 'q' }, { "graceful", required_argument, NULL, ARG_GRACEFUL }, { "timeout", required_argument, NULL, ARG_TIMEOUT }, + { "exec", no_argument, NULL, ARG_EXEC }, {}, }; @@ -187,6 +194,10 @@ static int parse_argv(int argc, char *argv[]) { break; + case ARG_EXEC: + arg_exec = true; + break; + case '?': return -EINVAL; @@ -529,18 +540,34 @@ static int verb_call(int argc, char *argv[], void *userdata) { _cleanup_(sd_json_variant_unrefp) sd_json_variant *jp = NULL; _cleanup_(sd_varlink_unrefp) sd_varlink *vl = NULL; const char *url, *method, *parameter, *source; + char **cmdline; int r; assert(argc >= 3); - assert(argc <= 4); + + if (argc > 4 && !arg_exec) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Too many arguments."); + if (arg_exec && argc < 5) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Expected command line to execute."); + + if (arg_exec && (arg_collect || (arg_method_flags & (SD_VARLINK_METHOD_ONEWAY|SD_VARLINK_METHOD_MORE))) != 0) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "--exec and --collect/--more/--oneway may not be combined."); + url = argv[1]; method = argv[2]; parameter = argc > 3 && !streq(argv[3], "-") ? argv[3] : NULL; + cmdline = strv_skip(argv, 4); - /* No JSON mode explicitly configured? Then default to the same as -j */ + /* No JSON mode explicitly configured? Then default to the same as -j (except if --exec is used, in + * which case generate shortest possible JSON since we are going to pass it to a program rather than + * a user anyway) */ if (!sd_json_format_enabled(arg_json_format_flags)) { arg_json_format_flags &= ~SD_JSON_FORMAT_OFF; - arg_json_format_flags |= SD_JSON_FORMAT_PRETTY_AUTO|SD_JSON_FORMAT_COLOR_AUTO; + + if (arg_exec) + arg_json_format_flags |= SD_JSON_FORMAT_NEWLINE; + else + arg_json_format_flags |= SD_JSON_FORMAT_PRETTY_AUTO|SD_JSON_FORMAT_COLOR_AUTO; } /* For pipeable text tools it's kinda customary to finish output off in a newline character, and not @@ -648,6 +675,15 @@ static int verb_call(int argc, char *argv[], void *userdata) { } else { sd_json_variant *reply = NULL; const char *error = NULL; + bool process_fds = false; + + if (arg_exec) { + r = sd_varlink_set_allow_fd_passing_input(vl, true); + if (r < 0) + log_debug_errno(r, "Unable to enable file descriptor receiving, ignoring: %m"); + else + process_fds = true; + } r = sd_varlink_call(vl, method, jp, &reply, &error); if (r < 0) @@ -668,6 +704,100 @@ static int verb_call(int argc, char *argv[], void *userdata) { } else r = 0; + if (arg_exec && r == 0) { + (void) sd_notify(/* unset_environment= */ false, "READY=1"); + + _cleanup_free_ char *formatted = NULL; + r = sd_json_variant_format(reply, arg_json_format_flags, &formatted); + if (r < 0) + return log_error_errno(r, "Failed to format reply: %m"); + + _cleanup_close_ int mfd = memfd_new_and_seal_string("varlink-reply", formatted); + if (mfd < 0) + return log_error_errno(mfd, "Failed to allocate memfd for reply: %m"); + + _cleanup_free_ char *j = strv_join(cmdline, " "); + if (!j) + return log_oom(); + + int *fd_array = NULL, n = 0; + size_t m = 0; + CLEANUP_ARRAY(fd_array, m, close_many_and_free); + + if (process_fds) { + n = sd_varlink_get_n_fds(vl); + if (n < 0) + return log_error_errno(n, "Failed to determine how many file descriptors we received: %m"); + + fd_array = new(int, n); + if (!fd_array) + return log_oom(); + + for (int i = 0; i < n; i++) { + fd_array[m] = sd_varlink_take_fd(vl, i); + if (fd_array[m] < 0) + return log_error_errno(fd_array[m], "Failed to acquire fd we received: %m"); + + m++; + } + } + + /* We'll now close all remaining fds. This means we are stealing other code that + * lives in our process their fds. Hence we will now no longer bubble up any + * errors. */ + + log_close(); + log_set_open_when_needed(true); + + r = move_fd(mfd, STDIN_FILENO, /* cloexec= */ false); + if (r < 0) { + log_error_errno(r, "Failed to move reply to STDIN_FILENO: %m"); + _exit(EXIT_FAILURE); + } + + r = close_all_fds(fd_array, m); + if (r < 0) { + log_error_errno(r, "Failed to close all remaining file descriptors: %m"); + _exit(EXIT_FAILURE); + } + + r = pack_fds(fd_array, m); + if (r < 0) { + log_error_errno(r, "Failed to rearrange file descriptors: %m"); + _exit(EXIT_FAILURE); + } + + r = fd_cloexec_many(fd_array, m, false); + if (r < 0) { + log_error_errno(r, "Failed to disable O_CLOEXEC for file descriptors: %m"); + _exit(EXIT_FAILURE); + } + + if (m > 0) { + r = setenvf("LISTEN_FDS", /* overwrite= */ true, "%zu", m); + if (r < 0) { + log_error_errno(r, "Failed to set $LISTEN_FDS environment variable: %m"); + _exit(EXIT_FAILURE); + } + + r = setenvf("LISTEN_PID", /* overwrite= */ true, PID_FMT, getpid_cached()); + if (r < 0) { + log_error_errno(r, "Failed to set $LISTEN_PID environment variable: %m"); + _exit(EXIT_FAILURE); + } + } else { + (void) unsetenv("LISTEN_FDS"); + (void) unsetenv("LISTEN_PID"); + } + (void) unsetenv("LISTEN_FDNAMES"); + + log_debug("Executing: %s", j); + + execvp(cmdline[0], cmdline); + log_error_errno(errno, "Failed to execute '%s': %m", j); + _exit(EXIT_FAILURE); + } + if (arg_quiet) return r; @@ -735,7 +865,7 @@ static int varlinkctl_main(int argc, char *argv[]) { { "list-interfaces", 2, 2, 0, verb_info }, { "introspect", 2, VERB_ANY, 0, verb_introspect }, { "list-methods", 2, VERB_ANY, 0, verb_introspect }, - { "call", 3, 4, 0, verb_call }, + { "call", 3, VERB_ANY, 0, verb_call }, { "validate-idl", 1, 2, 0, verb_validate_idl }, { "help", VERB_ANY, VERB_ANY, 0, verb_help }, {} From a932d2f23e3d9f6d5093e645c3b06718e943a48d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 17 Feb 2025 11:34:27 +0100 Subject: [PATCH 10/11] test: fix racy machined test Previously, one of the io.systemd.Machine.Open() tests would invoke a command line via machined, and then check if it ran properly. This was implemented in a racy fashion: the client side would immediately close the pty fd allocated for the operation, thus triggering an immediate SIGHUP on the other side. Now, depending whether this client was quicker in closing or the server was quicker in executing the command line this was a race. Fix this comprehensively: let's first wait for the varlink operation to complete via the new "systemd-notify --fork" logic (because varlinkctl sends out READY=1 once handing off to --exec). Secondly let's use varlinkctl's --exec logic to invoke a process which keeps open the open pty until we kill it (we just use sleep for that). (Also add some more tests for the varlinkctl --exec stuff) --- test/units/TEST-13-NSPAWN.machined.sh | 11 ++++++++++- test/units/TEST-74-AUX-UTILS.varlinkctl.sh | 6 ++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/test/units/TEST-13-NSPAWN.machined.sh b/test/units/TEST-13-NSPAWN.machined.sh index 5bc24236747..4875e48f809 100755 --- a/test/units/TEST-13-NSPAWN.machined.sh +++ b/test/units/TEST-13-NSPAWN.machined.sh @@ -388,9 +388,18 @@ varlinkctl call /run/systemd/machine/io.systemd.Machine io.systemd.Machine.Open varlinkctl call /run/systemd/machine/io.systemd.Machine io.systemd.Machine.Open '{"name": ".host", "mode": "shell"}' rm -f /tmp/none-existent-file -varlinkctl call /run/systemd/machine/io.systemd.Machine io.systemd.Machine.Open '{"name": ".host", "mode": "shell", "user": "root", "path": "/bin/sh", "args": ["/bin/sh", "-c", "echo $FOO > /tmp/none-existent-file"], "environment": ["FOO=BAR"]}' + +# We need to make sure the acquired pty fd stays open if we invoke a command +# server side, to not generate early SIGHUP. Hence, let's just invoke "sleep +# infinity" client side, once we acquired the fd (passing it to it), and kill +# it once we verified everything worked. +PID=$(systemd-notify --fork -- varlinkctl --exec call /run/systemd/machine/io.systemd.Machine io.systemd.Machine.Open '{"name": ".host", "mode": "shell", "user": "root", "path": "/bin/bash", "args": ["/bin/bash", "-c", "echo $FOO > /tmp/none-existent-file"], "environment": ["FOO=BAR"]}' -- sleep infinity) timeout 30 bash -c "until test -e /tmp/none-existent-file; do sleep .5; done" grep -q "BAR" /tmp/none-existent-file +kill "$PID" + +# Test varlinkctl's --exec fd passing logic properly +assert_eq "$(varlinkctl --exec call /run/systemd/machine/io.systemd.Machine io.systemd.Machine.Open '{"name": ".host", "mode": "shell", "user": "root", "path": "/bin/bash", "args": ["/bin/bash", "-c", "echo $((7 + 8))"]}' -- bash -c 'read -r -N 2 x <&3 ; echo "$x"')" 15 # test io.systemd.Machine.MapFrom varlinkctl call /run/systemd/machine/io.systemd.Machine io.systemd.Machine.MapFrom '{"name": "long-running", "uid":0, "gid": 0}' diff --git a/test/units/TEST-74-AUX-UTILS.varlinkctl.sh b/test/units/TEST-74-AUX-UTILS.varlinkctl.sh index a0797469a46..921fadf998c 100755 --- a/test/units/TEST-74-AUX-UTILS.varlinkctl.sh +++ b/test/units/TEST-74-AUX-UTILS.varlinkctl.sh @@ -160,3 +160,9 @@ done varlinkctl info /run/systemd/io.systemd.Hostname varlinkctl introspect /run/systemd/io.systemd.Hostname io.systemd.Hostname varlinkctl call /run/systemd/io.systemd.Hostname io.systemd.Hostname.Describe '{}' + +# Validate that --exec results in the very same values +varlinkctl call /run/systemd/io.systemd.Hostname io.systemd.Hostname.Describe '{}' | jq > /tmp/describe1.json +varlinkctl --exec call /run/systemd/io.systemd.Hostname io.systemd.Hostname.Describe '{}' -- jq > /tmp/describe2.json +cmp /tmp/describe1.json /tmp/describe2.json +rm /tmp/describe1.json /tmp/describe2.json From 40e22609e8dad046a984eaa84dba2a26b16d5a9c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 17 Feb 2025 17:14:30 +0100 Subject: [PATCH 11/11] ptyfwd-tool: don't segfault if called without arguments --- src/ptyfwd/ptyfwd-tool.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/ptyfwd/ptyfwd-tool.c b/src/ptyfwd/ptyfwd-tool.c index c6ae362e3d3..f62a4dc767b 100644 --- a/src/ptyfwd/ptyfwd-tool.c +++ b/src/ptyfwd/ptyfwd-tool.c @@ -108,6 +108,9 @@ static int parse_argv(int argc, char *argv[]) { assert_not_reached(); } + if (optind >= argc) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Expected command line, refusing."); + return 1; } @@ -156,6 +159,8 @@ static int run(int argc, char *argv[]) { if (!l) return log_oom(); + assert_se(!strv_isempty(l)); + r = sd_event_default(&event); if (r < 0) return log_error_errno(r, "Failed to get event loop: %m");