diff --git a/src/basic/pidref.c b/src/basic/pidref.c index 9b4922b160..d8c61a17c0 100644 --- a/src/basic/pidref.c +++ b/src/basic/pidref.c @@ -457,12 +457,36 @@ bool pidref_is_automatic(const PidRef *pidref) { return pidref && pid_is_automatic(pidref->pid); } -static void pidref_hash_func(const PidRef *pidref, struct siphash *state) { +void pidref_hash_func(const PidRef *pidref, struct siphash *state) { siphash24_compress_typesafe(pidref->pid, state); } -static int pidref_compare_func(const PidRef *a, const PidRef *b) { - return CMP(a->pid, b->pid); +int pidref_compare_func(const PidRef *a, const PidRef *b) { + int r; + + assert(a); + assert(b); + + r = CMP(pidref_is_set(a), pidref_is_set(b)); + if (r != 0) + return r; + + r = CMP(pidref_is_automatic(a), pidref_is_automatic(b)); + if (r != 0) + return r; + + r = CMP(pidref_is_remote(a), pidref_is_remote(b)); + if (r != 0) + return r; + + r = CMP(a->pid, b->pid); + if (r != 0) + return r; + + if (a->fd_id != 0 && b->fd_id != 0) + return CMP(a->fd_id, b->fd_id); + + return 0; } DEFINE_HASH_OPS(pidref_hash_ops, PidRef, pidref_hash_func, pidref_compare_func); diff --git a/src/basic/pidref.h b/src/basic/pidref.h index 0198db8f8e..b71c3bd9d0 100644 --- a/src/basic/pidref.h +++ b/src/basic/pidref.h @@ -108,5 +108,9 @@ int pidref_verify(const PidRef *pidref); #define TAKE_PIDREF(p) TAKE_GENERIC((p), PidRef, PIDREF_NULL) +struct siphash; +void pidref_hash_func(const PidRef *pidref, struct siphash *state); +int pidref_compare_func(const PidRef *a, const PidRef *b); + extern const struct hash_ops pidref_hash_ops; extern const struct hash_ops pidref_hash_ops_free; /* Has destructor call for pidref_free(), i.e. expects heap allocated PidRef as keys */ diff --git a/src/core/manager.c b/src/core/manager.c index 90d741e563..36217ead14 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -2798,7 +2798,6 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t Manager *m = ASSERT_PTR(userdata); _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; struct ucred ucred; - _cleanup_free_ char *buf = NULL; _cleanup_(fdset_free_asyncp) FDSet *fds = NULL; int r; @@ -2809,7 +2808,8 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t return 0; } - r = notify_recv_with_fds(m->notify_fd, &buf, &ucred, &pidref, &fds); + _cleanup_strv_free_ char **tags = NULL; + r = notify_recv_with_fds_strv(m->notify_fd, &tags, &ucred, &pidref, &fds); if (r == -EAGAIN) return 0; if (r < 0) @@ -2819,12 +2819,6 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t * socket. */ return r; - _cleanup_strv_free_ char **tags = strv_split_newlines(buf); - if (!tags) { - log_oom_warning(); - return 0; - } - /* Possibly a barrier fd, let's see. */ if (manager_process_barrier_fd(tags, fds)) { log_debug("Received barrier notification message from PID " PID_FMT ".", pidref.pid); diff --git a/src/home/homed-home.c b/src/home/homed-home.c index eba7be9b6a..871ccdffe9 100644 --- a/src/home/homed-home.c +++ b/src/home/homed-home.c @@ -12,6 +12,7 @@ #include "env-util.h" #include "errno-list.h" #include "errno-util.h" +#include "event-util.h" #include "fd-util.h" #include "fileio.h" #include "filesystems.h" @@ -153,6 +154,7 @@ int home_new(Manager *m, UserRecord *hr, const char *sysfs, Home **ret) { .aliases = TAKE_PTR(aliases), .uid = hr->uid, .state = _HOME_STATE_INVALID, + .worker_pid = PIDREF_NULL, .worker_stdout_fd = -EBADF, .sysfs = TAKE_PTR(ns), .signed_locally = -1, @@ -224,8 +226,8 @@ Home *home_free(Home *h) { if (h->sysfs) (void) hashmap_remove_value(h->manager->homes_by_sysfs, h->sysfs, h); - if (h->worker_pid > 0) - (void) hashmap_remove_value(h->manager->homes_by_worker_pid, PID_TO_PTR(h->worker_pid), h); + if (pidref_is_set(&h->worker_pid)) + (void) hashmap_remove_value(h->manager->homes_by_worker_pid, &h->worker_pid, h); if (h->manager->gc_focus == h) h->manager->gc_focus = NULL; @@ -236,6 +238,7 @@ Home *home_free(Home *h) { user_record_unref(h->record); user_record_unref(h->secret); + pidref_done_sigkill_wait(&h->worker_pid); h->worker_event_source = sd_event_source_disable_unref(h->worker_event_source); safe_close(h->worker_stdout_fd); free(h->user_name); @@ -1137,13 +1140,13 @@ static int home_on_worker_process(sd_event_source *s, const siginfo_t *si, void assert(s); assert(si); - assert(h->worker_pid == si->si_pid); + assert(h->worker_pid.pid == si->si_pid); assert(h->worker_event_source); assert(h->worker_stdout_fd >= 0); - (void) hashmap_remove_value(h->manager->homes_by_worker_pid, PID_TO_PTR(h->worker_pid), h); + (void) hashmap_remove_value(h->manager->homes_by_worker_pid, &h->worker_pid, h); - h->worker_pid = 0; + pidref_done(&h->worker_pid); h->worker_event_source = sd_event_source_disable_unref(h->worker_event_source); if (si->si_code != CLD_EXITED) { @@ -1227,14 +1230,13 @@ static int home_start_work( _cleanup_(erase_and_freep) char *formatted = NULL; _cleanup_close_ int stdin_fd = -EBADF, stdout_fd = -EBADF; _cleanup_free_ int *blob_fds = NULL; - pid_t pid = 0; int r; assert(h); assert(verb); assert(hr); - if (h->worker_pid != 0) + if (pidref_is_set(&h->worker_pid)) return -EBUSY; assert(h->worker_stdout_fd < 0); @@ -1299,7 +1301,8 @@ static int home_start_work( if (stdout_fd < 0) return stdout_fd; - r = safe_fork_full("(sd-homework)", + _cleanup_(pidref_done_sigkill_wait) PidRef pid = PIDREF_NULL; + r = pidref_safe_fork_full("(sd-homework)", (int[]) { stdin_fd, stdout_fd, STDERR_FILENO }, blob_fds, hashmap_size(blobs), FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_CLOEXEC_OFF|FORK_PACK_FDS|FORK_DEATHSIG_SIGTERM| @@ -1362,20 +1365,21 @@ static int home_start_work( _exit(EXIT_FAILURE); } - r = sd_event_add_child(h->manager->event, &h->worker_event_source, pid, WEXITED, home_on_worker_process, h); + r = event_add_child_pidref(h->manager->event, &h->worker_event_source, &pid, WEXITED, home_on_worker_process, h); if (r < 0) return r; (void) sd_event_source_set_description(h->worker_event_source, "worker"); - r = hashmap_put(h->manager->homes_by_worker_pid, PID_TO_PTR(pid), h); + h->worker_pid = TAKE_PIDREF(pid); + r = hashmap_put(h->manager->homes_by_worker_pid, &h->worker_pid, h); if (r < 0) { + pidref_done_sigkill_wait(&h->worker_pid); h->worker_event_source = sd_event_source_disable_unref(h->worker_event_source); return r; } h->worker_stdout_fd = TAKE_FD(stdout_fd); - h->worker_pid = pid; h->worker_error_code = 0; return 0; @@ -3278,19 +3282,19 @@ int home_wait_for_worker(Home *h) { assert(h); - if (h->worker_pid <= 0) + if (!pidref_is_set(&h->worker_pid)) return 0; log_info("Worker process for home %s is still running while exiting. Waiting for it to finish.", h->user_name); - r = wait_for_terminate_with_timeout(h->worker_pid, 30 * USEC_PER_SEC); + r = wait_for_terminate_with_timeout(h->worker_pid.pid, 30 * USEC_PER_SEC); if (r == -ETIMEDOUT) log_warning_errno(r, "Waiting for worker process for home %s timed out. Ignoring.", h->user_name); else if (r < 0) log_warning_errno(r, "Failed to wait for worker process for home %s. Ignoring.", h->user_name); - (void) hashmap_remove_value(h->manager->homes_by_worker_pid, PID_TO_PTR(h->worker_pid), h); - h->worker_pid = 0; + (void) hashmap_remove_value(h->manager->homes_by_worker_pid, &h->worker_pid, h); + pidref_done(&h->worker_pid); return 1; } diff --git a/src/home/homed-home.h b/src/home/homed-home.h index 93689563d3..dfd64461fc 100644 --- a/src/home/homed-home.h +++ b/src/home/homed-home.h @@ -8,6 +8,7 @@ typedef struct Home Home; #include "homed-operation.h" #include "list.h" #include "ordered-set.h" +#include "pidref.h" #include "stat-util.h" #include "user-record.h" @@ -128,7 +129,7 @@ struct Home { UserRecord *record; - pid_t worker_pid; + PidRef worker_pid; int worker_stdout_fd; sd_event_source *worker_event_source; int worker_error_code; diff --git a/src/home/homed-manager.c b/src/home/homed-manager.c index 7121833660..e628e1a389 100644 --- a/src/home/homed-manager.c +++ b/src/home/homed-manager.c @@ -37,6 +37,7 @@ #include "homed-varlink.h" #include "io-util.h" #include "mkdir.h" +#include "notify-recv.h" #include "openssl-util.h" #include "process-util.h" #include "quota-util.h" @@ -76,7 +77,7 @@ static bool uid_is_home(uid_t uid) { #define UID_CLAMP_INTO_HOME_RANGE(rnd) (((uid_t) (rnd) % (HOME_UID_MAX - HOME_UID_MIN + 1)) + HOME_UID_MIN) DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(homes_by_uid_hash_ops, void, trivial_hash_func, trivial_compare_func, Home, home_free); -DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(homes_by_worker_pid_hash_ops, void, trivial_hash_func, trivial_compare_func, Home, home_free); +DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(homes_by_worker_pid_hash_ops, PidRef, pidref_hash_func, pidref_compare_func, Home, home_free); DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(homes_by_sysfs_hash_ops, char, path_hash_func, path_compare, Home, home_free); static int on_home_inotify(sd_event_source *s, const struct inotify_event *event, void *userdata); @@ -1068,123 +1069,33 @@ static int manager_bind_varlink(Manager *m) { return 0; } -static ssize_t read_datagram( - int fd, - struct ucred *ret_sender, - void **ret, - int *ret_passed_fd) { - - CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct ucred)) + CMSG_SPACE(sizeof(int))) control; - _cleanup_free_ void *buffer = NULL; - _cleanup_close_ int passed_fd = -EBADF; - struct ucred *sender = NULL; - struct cmsghdr *cmsg; - struct msghdr mh; - struct iovec iov; - ssize_t n, m; - - assert(fd >= 0); - assert(ret_sender); - assert(ret); - assert(ret_passed_fd); - - n = next_datagram_size_fd(fd); - if (n < 0) - return n; - - buffer = malloc(n + 2); - if (!buffer) - return -ENOMEM; - - /* Pass one extra byte, as a size check */ - iov = IOVEC_MAKE(buffer, n + 1); - - mh = (struct msghdr) { - .msg_iov = &iov, - .msg_iovlen = 1, - .msg_control = &control, - .msg_controllen = sizeof(control), - }; - - m = recvmsg_safe(fd, &mh, MSG_DONTWAIT|MSG_CMSG_CLOEXEC); - if (m < 0) - return m; - - /* Ensure the size matches what we determined before */ - if (m != n) { - cmsg_close_all(&mh); - return -EMSGSIZE; - } - - CMSG_FOREACH(cmsg, &mh) { - if (cmsg->cmsg_level == SOL_SOCKET && - cmsg->cmsg_type == SCM_CREDENTIALS && - cmsg->cmsg_len == CMSG_LEN(sizeof(struct ucred))) { - assert(!sender); - sender = CMSG_TYPED_DATA(cmsg, struct ucred); - } - - if (cmsg->cmsg_level == SOL_SOCKET && - cmsg->cmsg_type == SCM_RIGHTS) { - - if (cmsg->cmsg_len != CMSG_LEN(sizeof(int))) { - cmsg_close_all(&mh); - return -EMSGSIZE; - } - - assert(passed_fd < 0); - passed_fd = *CMSG_TYPED_DATA(cmsg, int); - } - } - - if (sender) - *ret_sender = *sender; - else - *ret_sender = (struct ucred) UCRED_INVALID; - - *ret_passed_fd = TAKE_FD(passed_fd); - - /* For safety reasons: let's always NUL terminate. */ - ((char*) buffer)[n] = 0; - *ret = TAKE_PTR(buffer); - - return 0; -} - static int on_notify_socket(sd_event_source *s, int fd, uint32_t revents, void *userdata) { - _cleanup_strv_free_ char **l = NULL; - _cleanup_free_ void *datagram = NULL; - _cleanup_close_ int passed_fd = -EBADF; - struct ucred sender = UCRED_INVALID; Manager *m = ASSERT_PTR(userdata); - ssize_t n; - Home *h; + int r; assert(s); - n = read_datagram(fd, &sender, &datagram, &passed_fd); - if (n < 0) { - if (ERRNO_IS_TRANSIENT(n)) - return 0; - return log_error_errno(n, "Failed to read notify datagram: %m"); - } + _cleanup_(fdset_free_asyncp) FDSet *passed_fds = NULL; + _cleanup_(pidref_done) PidRef sender = PIDREF_NULL; + _cleanup_strv_free_ char **l = NULL; + r = notify_recv_with_fds_strv(fd, &l, /* ret_ucred= */ NULL, &sender, &passed_fds); + if (r == -EAGAIN) + return 0; + if (r < 0) + return r; - if (sender.pid <= 0) { - log_warning("Received notify datagram without valid sender PID, ignoring."); + if (fdset_size(passed_fds) > 1) { + log_warning("Received notify datagram with multiple fds, ignoring."); return 0; } - h = hashmap_get(m->homes_by_worker_pid, PID_TO_PTR(sender.pid)); + Home *h = hashmap_get(m->homes_by_worker_pid, &sender); if (!h) { log_warning("Received notify datagram of unknown process, ignoring."); return 0; } - l = strv_split(datagram, "\n"); - if (!l) - return log_oom(); - - home_process_notify(h, l, TAKE_FD(passed_fd)); + home_process_notify(h, l, fdset_steal_first(passed_fds)); return 0; } @@ -1224,7 +1135,11 @@ static int manager_listen_notify(Manager *m) { r = setsockopt_int(fd, SOL_SOCKET, SO_PASSCRED, true); if (r < 0) - return r; + return log_error_errno(r, "Failed to enable SO_PASSCRED on notify socket: %m"); + + r = setsockopt_int(fd, SOL_SOCKET, SO_PASSPIDFD, true); + if (r < 0) + log_warning_errno(r, "Failed to enable SO_PASSPIDFD on notify socket, ignoring: %m"); r = sd_event_add_io(m->event, &m->notify_socket_event_source, fd, EPOLLIN, on_notify_socket, m); if (r < 0) diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 91bc8e8b0e..92724e53f8 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -4622,8 +4622,8 @@ static int nspawn_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t r assert(userdata); _cleanup_(pidref_done) PidRef sender_pid = PIDREF_NULL; - _cleanup_free_ char *buf = NULL; - r = notify_recv(fd, &buf, /* ret_ucred= */ NULL, &sender_pid); + _cleanup_strv_free_ char **tags = NULL; + r = notify_recv_strv(fd, &tags, /* ret_ucred= */ NULL, &sender_pid); if (r == -EAGAIN) return 0; if (r < 0) @@ -4634,10 +4634,6 @@ static int nspawn_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t r return 0; } - _cleanup_strv_free_ char **tags = strv_split(buf, "\n\r"); - if (!tags) - return log_oom(); - if (DEBUG_LOGGING) { _cleanup_free_ char *joined = strv_join(tags, " "); diff --git a/src/shared/notify-recv.c b/src/shared/notify-recv.c index ce12ec8e01..08719a20bb 100644 --- a/src/shared/notify-recv.c +++ b/src/shared/notify-recv.c @@ -4,6 +4,8 @@ #include "fd-util.h" #include "notify-recv.h" #include "socket-util.h" +#include "strv.h" +#include "user-util.h" int notify_recv_with_fds( int fd, @@ -140,3 +142,45 @@ int notify_recv_with_fds( return 0; } + +int notify_recv_with_fds_strv( + int fd, + char ***ret_list, + struct ucred *ret_ucred, + PidRef *ret_pidref, + FDSet **ret_fds) { + + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; + _cleanup_(fdset_free_asyncp) FDSet *fds = NULL; + struct ucred ucred = UCRED_INVALID; + _cleanup_free_ char *text = NULL; + int r; + + r = notify_recv_with_fds( + fd, + ret_list ? &text : NULL, + ret_ucred ? &ucred : NULL, + ret_pidref ? &pidref : NULL, + ret_fds ? &fds : NULL); + if (r < 0) + return r; + + if (ret_list) { + char **l = strv_split_newlines(text); + if (!l) { + log_oom_warning(); + return -EAGAIN; + } + + *ret_list = l; + } + + if (ret_ucred) + *ret_ucred = ucred; + if (ret_pidref) + *ret_pidref = TAKE_PIDREF(pidref); + if (ret_fds) + *ret_fds = TAKE_PTR(fds); + + return 0; +} diff --git a/src/shared/notify-recv.h b/src/shared/notify-recv.h index 0a38fd28b4..f4739d8c12 100644 --- a/src/shared/notify-recv.h +++ b/src/shared/notify-recv.h @@ -16,3 +16,14 @@ int notify_recv_with_fds( static inline int notify_recv(int fd, char **ret_text, struct ucred *ret_ucred, PidRef *ret_pidref) { return notify_recv_with_fds(fd, ret_text, ret_ucred, ret_pidref, NULL); } + +int notify_recv_with_fds_strv( + int fd, + char ***ret_list, + struct ucred *ret_ucred, + PidRef *ret_pidref, + FDSet **ret_fds); + +static inline int notify_recv_strv(int fd, char ***ret_list, struct ucred *ret_ucred, PidRef *ret_pidref) { + return notify_recv_with_fds_strv(fd, ret_list, ret_ucred, ret_pidref, NULL); +}