From 3bda3f17fa84557eeb28fa7c330cbd3a3f876d47 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 9 Sep 2023 09:29:27 +0200 Subject: [PATCH 1/9] pidref: add structure that can reference a pid via both pidfd and pid_t Let's start with the conversion of PID 1 to pidfds. Let's add a simple structure with just two fields that can be used to maintain a reference to arbitrary processes via both pid_t and pidfd. This is an embeddable struct, to keep it in line with where we previously used a pid_t directly to track a process. Of course, since this might contain an fd on systems where we have pidfd this structure has a proper lifecycle. (Note that this is quite different from sd_event_add_child() event source objects as that one is only for child processes and collects process results, while this infra is much simpler and more generic and can be used to reference any process, anywhere in the tree.) --- src/basic/meson.build | 1 + src/basic/pidref.c | 145 ++++++++++++++++++++++++++++++++++++++++++ src/basic/pidref.h | 29 +++++++++ 3 files changed, 175 insertions(+) create mode 100644 src/basic/pidref.c create mode 100644 src/basic/pidref.h diff --git a/src/basic/meson.build b/src/basic/meson.build index 77ce2cf2621..0e4c5584da1 100644 --- a/src/basic/meson.build +++ b/src/basic/meson.build @@ -65,6 +65,7 @@ basic_sources = files( 'path-lookup.c', 'path-util.c', 'percent-util.c', + 'pidref.c', 'prioq.c', 'proc-cmdline.c', 'process-util.c', diff --git a/src/basic/pidref.c b/src/basic/pidref.c new file mode 100644 index 00000000000..f41460938c1 --- /dev/null +++ b/src/basic/pidref.c @@ -0,0 +1,145 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "errno-util.h" +#include "fd-util.h" +#include "missing_syscall.h" +#include "parse-util.h" +#include "pidref.h" +#include "process-util.h" + +int pidref_set_pid(PidRef *pidref, pid_t pid) { + int fd; + + assert(pidref); + + if (pid < 0) + return -ESRCH; + if (pid == 0) + pid = getpid_cached(); + + fd = pidfd_open(pid, 0); + if (fd < 0) { + /* Graceful fallback in case the kernel doesn't support pidfds or is out of fds */ + if (!ERRNO_IS_NOT_SUPPORTED(errno) && !ERRNO_IS_PRIVILEGE(errno) && !ERRNO_IS_RESOURCE(errno)) + return -errno; + + fd = -EBADF; + } + + *pidref = (PidRef) { + .fd = fd, + .pid = pid, + }; + + return 0; +} + +int pidref_set_pidstr(PidRef *pidref, const char *pid) { + pid_t nr; + int r; + + assert(pidref); + + r = parse_pid(pid, &nr); + if (r < 0) + return r; + + return pidref_set_pid(pidref, nr); +} + +int pidref_set_pidfd(PidRef *pidref, int fd) { + int r; + + assert(pidref); + + if (fd < 0) + return -EBADF; + + int fd_copy = fcntl(fd, F_DUPFD_CLOEXEC, 3); + if (fd_copy < 0) { + pid_t pid; + + if (!ERRNO_IS_RESOURCE(errno)) + return -errno; + + /* Graceful fallback if we are out of fds */ + r = pidfd_get_pid(fd, &pid); + if (r < 0) + return r; + + *pidref = (PidRef) { + .fd = -EBADF, + .pid = pid, + }; + + return 0; + } + + return pidref_set_pidfd_consume(pidref, fd_copy); +} + +int pidref_set_pidfd_take(PidRef *pidref, int fd) { + pid_t pid; + int r; + + assert(pidref); + + if (fd < 0) + return -EBADF; + + r = pidfd_get_pid(fd, &pid); + if (r < 0) + return r; + + *pidref = (PidRef) { + .fd = fd, + .pid = pid, + }; + + return 0; +} + +int pidref_set_pidfd_consume(PidRef *pidref, int fd) { + int r; + + r = pidref_set_pidfd_take(pidref, fd); + if (r < 0) + safe_close(fd); + + return r; +} + +void pidref_done(PidRef *pidref) { + assert(pidref); + + *pidref = (PidRef) { + .fd = safe_close(pidref->fd), + }; +} + +int pidref_kill(PidRef *pidref, int sig) { + + if (!pidref) + return -ESRCH; + + if (pidref->fd >= 0) + return RET_NERRNO(pidfd_send_signal(pidref->fd, sig, NULL, 0)); + + if (pidref->pid > 0) + return RET_NERRNO(kill(pidref->pid, sig)); + + return -ESRCH; +} + +int pidref_kill_and_sigcont(PidRef *pidref, int sig) { + int r; + + r = pidref_kill(pidref, sig); + if (r < 0) + return r; + + if (!IN_SET(sig, SIGCONT, SIGKILL)) + (void) pidref_kill(pidref, SIGCONT); + + return 0; +} diff --git a/src/basic/pidref.h b/src/basic/pidref.h new file mode 100644 index 00000000000..2411e510f13 --- /dev/null +++ b/src/basic/pidref.h @@ -0,0 +1,29 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +#pragma once + +#include "macro.h" + +/* An embeddable structure carrying a reference to a process. Supposed to be used when tracking processes continously. */ +typedef struct PidRef { + pid_t pid; /* always valid */ + int fd; /* only valid if pidfd are available in the kernel, and we manage to get an fd */ +} PidRef; + +#define PIDREF_NULL (PidRef) { .fd = -EBADF } + +static inline bool pidref_is_set(const PidRef *pidref) { + return pidref && pidref->pid > 0; +} + +int pidref_set_pid(PidRef *pidref, pid_t pid); +int pidref_set_pidstr(PidRef *pidref, const char *pid); +int pidref_set_pidfd(PidRef *pidref, int fd); +int pidref_set_pidfd_take(PidRef *pidref, int fd); /* takes ownership of the passed pidfd on success*/ +int pidref_set_pidfd_consume(PidRef *pidref, int fd); /* takes ownership of the passed pidfd in both success and failure */ + +void pidref_done(PidRef *pidref); + +int pidref_kill(PidRef *pidref, int sig); +int pidref_kill_and_sigcont(PidRef *pidref, int sig); + +#define TAKE_PIDREF(p) TAKE_GENERIC((p), PidRef, PIDREF_NULL) From c79ab77cd31a718be88345565ba1657c36f7b880 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 9 Sep 2023 09:49:22 +0200 Subject: [PATCH 2/9] core: reference main/control pid of .service units via PidRef The first conversion to PidRef. It's mostly an excercise of search/replace, but with some special care taken for life-cycle (i.e. we need to destroy the structure properly once done, to release the pidfd). It also uses pidfd based killing for some of the killing but leaves most as it is to make the conversion minimal. --- src/core/dbus-service.c | 4 +- src/core/service.c | 207 ++++++++++++++++++++++------------------ src/core/service.h | 3 +- 3 files changed, 117 insertions(+), 97 deletions(-) diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c index 6068c742cad..cc65a559903 100644 --- a/src/core/dbus-service.c +++ b/src/core/dbus-service.c @@ -346,8 +346,8 @@ const sd_bus_vtable bus_service_vtable[] = { SD_BUS_PROPERTY("RestartPreventExitStatus", "(aiai)", property_get_exit_status_set, offsetof(Service, restart_prevent_status), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("RestartForceExitStatus", "(aiai)", property_get_exit_status_set, offsetof(Service, restart_force_status), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("SuccessExitStatus", "(aiai)", property_get_exit_status_set, offsetof(Service, success_status), SD_BUS_VTABLE_PROPERTY_CONST), - SD_BUS_PROPERTY("MainPID", "u", bus_property_get_pid, offsetof(Service, main_pid), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), - SD_BUS_PROPERTY("ControlPID", "u", bus_property_get_pid, offsetof(Service, control_pid), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), + SD_BUS_PROPERTY("MainPID", "u", bus_property_get_pid, offsetof(Service, main_pid.pid), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), + SD_BUS_PROPERTY("ControlPID", "u", bus_property_get_pid, offsetof(Service, control_pid.pid), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("BusName", "s", NULL, offsetof(Service, bus_name), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("FileDescriptorStoreMax", "u", bus_property_get_unsigned, offsetof(Service, n_fd_store_max), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("NFileDescriptorStore", "u", property_get_size_as_uint32, offsetof(Service, n_fd_store), 0), diff --git a/src/core/service.c b/src/core/service.c index 2a02279f565..9aee6bca079 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -131,7 +131,8 @@ static void service_init(Unit *u) { s->socket_fd = -EBADF; s->stdin_fd = s->stdout_fd = s->stderr_fd = -EBADF; s->guess_main_pid = true; - + s->main_pid = PIDREF_NULL; + s->control_pid = PIDREF_NULL; s->control_command_id = _SERVICE_EXEC_COMMAND_INVALID; s->exec_context.keyring_mode = MANAGER_IS_SYSTEM(u->manager) ? @@ -151,19 +152,21 @@ static void service_init(Unit *u) { static void service_unwatch_control_pid(Service *s) { assert(s); - if (s->control_pid <= 0) + if (!pidref_is_set(&s->control_pid)) return; - unit_unwatch_pid(UNIT(s), TAKE_PID(s->control_pid)); + unit_unwatch_pid(UNIT(s), s->control_pid.pid); + pidref_done(&s->control_pid); } static void service_unwatch_main_pid(Service *s) { assert(s); - if (s->main_pid <= 0) + if (!pidref_is_set(&s->main_pid)) return; - unit_unwatch_pid(UNIT(s), TAKE_PID(s->main_pid)); + unit_unwatch_pid(UNIT(s), s->main_pid.pid); + pidref_done(&s->main_pid); } static void service_unwatch_pid_file(Service *s) { @@ -177,6 +180,9 @@ static void service_unwatch_pid_file(Service *s) { } static int service_set_main_pid(Service *s, pid_t pid) { + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; + int r; + assert(s); if (pid <= 1) @@ -185,15 +191,19 @@ static int service_set_main_pid(Service *s, pid_t pid) { if (pid == getpid_cached()) return -EINVAL; - if (s->main_pid == pid && s->main_pid_known) + if (s->main_pid.pid == pid && s->main_pid_known) return 0; - if (s->main_pid != pid) { + r = pidref_set_pid(&pidref, pid); + if (r < 0) + return r; + + if (s->main_pid.pid != pid) { service_unwatch_main_pid(s); exec_status_start(&s->main_exec_status, pid); } - s->main_pid = pid; + s->main_pid = TAKE_PIDREF(pidref); s->main_pid_known = true; s->main_pid_alien = pid_is_my_child(pid) == 0; @@ -446,8 +456,7 @@ static void service_done(Unit *u) { exit_status_set_free(&s->restart_force_status); exit_status_set_free(&s->success_status); - /* This will leak a process, but at least no memory or any of - * our resources */ + /* This will leak a process, but at least no memory or any of our resources */ service_unwatch_main_pid(s); service_unwatch_control_pid(s); service_unwatch_pid_file(s); @@ -964,17 +973,17 @@ static void service_dump(Unit *u, FILE *f, const char *prefix) { prefix, oom_policy_to_string(s->oom_policy), prefix, signal_to_string(s->reload_signal)); - if (s->control_pid > 0) + if (pidref_is_set(&s->control_pid)) fprintf(f, "%sControl PID: "PID_FMT"\n", - prefix, s->control_pid); + prefix, s->control_pid.pid); - if (s->main_pid > 0) + if (pidref_is_set(&s->main_pid)) fprintf(f, "%sMain PID: "PID_FMT"\n" "%sMain PID Known: %s\n" "%sMain PID Alien: %s\n", - prefix, s->main_pid, + prefix, s->main_pid.pid, prefix, yes_no(s->main_pid_known), prefix, yes_no(s->main_pid_alien)); @@ -1083,7 +1092,7 @@ static int service_is_suitable_main_pid(Service *s, pid_t pid, int prio) { if (pid == getpid_cached() || pid == 1) return log_unit_full_errno(UNIT(s), prio, SYNTHETIC_ERRNO(EPERM), "New main PID "PID_FMT" is the manager, refusing.", pid); - if (pid == s->control_pid) + if (pid == s->control_pid.pid) return log_unit_full_errno(UNIT(s), prio, SYNTHETIC_ERRNO(EPERM), "New main PID "PID_FMT" is the control process, refusing.", pid); if (!pid_is_alive(pid)) @@ -1137,7 +1146,7 @@ static int service_load_pid_file(Service *s, bool may_warn) { if (r < 0) return log_unit_full_errno(UNIT(s), prio, r, "Failed to parse PID from file %s: %m", s->pid_file); - if (s->main_pid_known && pid == s->main_pid) + if (s->main_pid_known && pid == s->main_pid.pid) return 0; r = service_is_suitable_main_pid(s, pid, prio); @@ -1163,7 +1172,7 @@ static int service_load_pid_file(Service *s, bool may_warn) { } if (s->main_pid_known) { - log_unit_debug(UNIT(s), "Main PID changing: "PID_FMT" -> "PID_FMT, s->main_pid, pid); + log_unit_debug(UNIT(s), "Main PID changing: "PID_FMT" -> "PID_FMT, s->main_pid.pid, pid); service_unwatch_main_pid(s); s->main_pid_known = false; @@ -1174,7 +1183,7 @@ static int service_load_pid_file(Service *s, bool may_warn) { if (r < 0) return r; - r = unit_watch_pid(UNIT(s), pid, false); + r = unit_watch_pid(UNIT(s), pid, /* exclusive= */ false); if (r < 0) /* FIXME: we need to do something here */ return log_unit_warning_errno(UNIT(s), r, "Failed to watch PID "PID_FMT" for service: %m", pid); @@ -1187,15 +1196,14 @@ static void service_search_main_pid(Service *s) { assert(s); - /* If we know it anyway, don't ever fall back to unreliable - * heuristics */ + /* If we know it anyway, don't ever fall back to unreliable heuristics */ if (s->main_pid_known) return; if (!s->guess_main_pid) return; - assert(s->main_pid <= 0); + assert(!pidref_is_set(&s->main_pid)); if (unit_search_main_pid(UNIT(s), &pid) < 0) return; @@ -1204,7 +1212,7 @@ static void service_search_main_pid(Service *s) { if (service_set_main_pid(s, pid) < 0) return; - r = unit_watch_pid(UNIT(s), pid, false); + r = unit_watch_pid(UNIT(s), pid, /* exclusive= */ false); if (r < 0) /* FIXME: we need to do something here */ log_unit_warning_errno(UNIT(s), r, "Failed to watch PID "PID_FMT" from: %m", pid); @@ -1336,28 +1344,28 @@ static int service_coldplug(Unit *u) { if (r < 0) return r; - if (s->main_pid > 0 && - pid_is_unwaited(s->main_pid) && + if (pidref_is_set(&s->main_pid) && + pid_is_unwaited(s->main_pid.pid) && (IN_SET(s->deserialized_state, SERVICE_START, SERVICE_START_POST, SERVICE_RUNNING, SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, SERVICE_STOP, SERVICE_STOP_WATCHDOG, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL, SERVICE_STOP_POST, SERVICE_FINAL_WATCHDOG, SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL))) { - r = unit_watch_pid(UNIT(s), s->main_pid, false); + r = unit_watch_pid(UNIT(s), s->main_pid.pid, /* exclusive= */ false); if (r < 0) return r; } - if (s->control_pid > 0 && - pid_is_unwaited(s->control_pid) && + if (pidref_is_set(&s->control_pid) && + pid_is_unwaited(s->control_pid.pid) && IN_SET(s->deserialized_state, SERVICE_CONDITION, SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST, SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, SERVICE_STOP, SERVICE_STOP_WATCHDOG, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL, SERVICE_STOP_POST, SERVICE_FINAL_WATCHDOG, SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL, SERVICE_CLEANING)) { - r = unit_watch_pid(UNIT(s), s->control_pid, false); + r = unit_watch_pid(UNIT(s), s->control_pid.pid, /* exclusive= */ false); if (r < 0) return r; } @@ -1620,7 +1628,7 @@ static int service_spawn_internal( ExecCommand *c, usec_t timeout, ExecFlags flags, - pid_t *ret_pid) { + PidRef *ret_pid) { _cleanup_(exec_params_clear) ExecParameters exec_params = { .flags = flags, @@ -1631,6 +1639,7 @@ static int service_spawn_internal( }; _cleanup_(sd_event_source_unrefp) sd_event_source *exec_fd_source = NULL; _cleanup_strv_free_ char **final_env = NULL, **our_env = NULL; + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; size_t n_env = 0; pid_t pid; int r; @@ -1699,8 +1708,8 @@ static int service_spawn_internal( return -ENOMEM; } - if (s->main_pid > 0) - if (asprintf(our_env + n_env++, "MAINPID="PID_FMT, s->main_pid) < 0) + if (pidref_is_set(&s->main_pid)) + if (asprintf(our_env + n_env++, "MAINPID="PID_FMT, s->main_pid.pid) < 0) return -ENOMEM; if (MANAGER_IS_USER(UNIT(s)->manager)) @@ -1830,12 +1839,15 @@ static int service_spawn_internal( s->exec_fd_event_source = TAKE_PTR(exec_fd_source); s->exec_fd_hot = false; - r = unit_watch_pid(UNIT(s), pid, true); + r = pidref_set_pid(&pidref, pid); if (r < 0) return r; - *ret_pid = pid; + r = unit_watch_pid(UNIT(s), pidref.pid, /* exclusive= */ true); + if (r < 0) + return r; + *ret_pid = TAKE_PIDREF(pidref); return 0; } @@ -1844,19 +1856,16 @@ static int main_pid_good(Service *s) { /* Returns 0 if the pid is dead, > 0 if it is good, < 0 if we don't know */ - /* If we know the pid file, then let's just check if it is - * still valid */ + /* If we know the pid file, then let's just check if it is still valid */ if (s->main_pid_known) { - /* If it's an alien child let's check if it is still - * alive ... */ - if (s->main_pid_alien && s->main_pid > 0) - return pid_is_alive(s->main_pid); + /* If it's an alien child let's check if it is still alive ... */ + if (s->main_pid_alien && pidref_is_set(&s->main_pid)) + return pid_is_alive(s->main_pid.pid); - /* .. otherwise assume we'll get a SIGCHLD for it, - * which we really should wait for to collect exit - * status and code */ - return s->main_pid > 0; + /* .. otherwise assume we'll get a SIGCHLD for it, which we really should wait for to collect + * exit status and code */ + return pidref_is_set(&s->main_pid); } /* We don't know the pid */ @@ -1870,7 +1879,7 @@ static int control_pid_good(Service *s) { * make this function as similar as possible to main_pid_good() and cgroup_good(), we pretend that < 0 also * means: we can't figure it out. */ - return s->control_pid > 0; + return pidref_is_set(&s->control_pid); } static int cgroup_good(Service *s) { @@ -2074,6 +2083,7 @@ static void service_enter_stop_post(Service *s, ServiceResult f) { s->control_command = s->exec_command[SERVICE_EXEC_STOP_POST]; if (s->control_command) { s->control_command_id = SERVICE_EXEC_STOP_POST; + pidref_done(&s->control_pid); r = service_spawn(s, s->control_command, @@ -2138,8 +2148,8 @@ static void service_enter_signal(Service *s, ServiceState state, ServiceResult f UNIT(s), &s->kill_context, kill_operation, - s->main_pid, - s->control_pid, + s->main_pid.pid, + s->control_pid.pid, s->main_pid_alien); if (r < 0) goto fail; @@ -2196,6 +2206,7 @@ static void service_enter_stop(Service *s, ServiceResult f) { s->control_command = s->exec_command[SERVICE_EXEC_STOP]; if (s->control_command) { s->control_command_id = SERVICE_EXEC_STOP; + pidref_done(&s->control_pid); r = service_spawn(s, s->control_command, @@ -2274,6 +2285,7 @@ static void service_enter_start_post(Service *s) { s->control_command = s->exec_command[SERVICE_EXEC_START_POST]; if (s->control_command) { s->control_command_id = SERVICE_EXEC_START_POST; + pidref_done(&s->control_pid); r = service_spawn(s, s->control_command, @@ -2299,17 +2311,17 @@ static void service_kill_control_process(Service *s) { assert(s); - if (s->control_pid <= 0) + if (!pidref_is_set(&s->control_pid)) return; - r = kill_and_sigcont(s->control_pid, SIGKILL); + r = pidref_kill_and_sigcont(&s->control_pid, SIGKILL); if (r < 0) { _cleanup_free_ char *comm = NULL; - (void) get_process_comm(s->control_pid, &comm); + (void) get_process_comm(s->control_pid.pid, &comm); log_unit_debug_errno(UNIT(s), r, "Failed to kill control process " PID_FMT " (%s), ignoring: %m", - s->control_pid, strna(comm)); + s->control_pid.pid, strna(comm)); } } @@ -2336,9 +2348,9 @@ static int service_adverse_to_leftover_processes(Service *s) { } static void service_enter_start(Service *s) { + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; ExecCommand *c; usec_t timeout; - pid_t pid; int r; assert(s); @@ -2392,7 +2404,7 @@ static void service_enter_start(Service *s) { c, timeout, EXEC_PASS_FDS|EXEC_APPLY_SANDBOXING|EXEC_APPLY_CHROOT|EXEC_APPLY_TTY_STDIN|EXEC_SET_WATCHDOG|EXEC_WRITE_CREDENTIALS|EXEC_SETENV_MONITOR_RESULT, - &pid); + &pidref); if (r < 0) goto fail; @@ -2400,7 +2412,7 @@ static void service_enter_start(Service *s) { /* For simple services we immediately start * the START_POST binaries. */ - (void) service_set_main_pid(s, pid); + (void) service_set_main_pid(s, pidref.pid); service_enter_start_post(s); } else if (s->type == SERVICE_FORKING) { @@ -2408,7 +2420,8 @@ static void service_enter_start(Service *s) { /* For forking services we wait until the start * process exited. */ - s->control_pid = pid; + pidref_done(&s->control_pid); + s->control_pid = TAKE_PIDREF(pidref); service_set_state(s, SERVICE_START); } else if (IN_SET(s->type, SERVICE_ONESHOT, SERVICE_DBUS, SERVICE_NOTIFY, SERVICE_NOTIFY_RELOAD, SERVICE_EXEC)) { @@ -2418,7 +2431,7 @@ static void service_enter_start(Service *s) { /* For D-Bus services we know the main pid right away, but wait for the bus name to appear on the * bus. 'notify' and 'exec' services are similar. */ - (void) service_set_main_pid(s, pid); + (void) service_set_main_pid(s, pidref.pid); service_set_state(s, SERVICE_START); } else assert_not_reached(); @@ -2480,6 +2493,7 @@ static void service_enter_condition(Service *s) { goto fail; s->control_command_id = SERVICE_EXEC_CONDITION; + pidref_done(&s->control_pid); r = service_spawn(s, s->control_command, @@ -2571,8 +2585,8 @@ static void service_enter_reload(Service *s) { usec_t ts = now(CLOCK_MONOTONIC); - if (s->type == SERVICE_NOTIFY_RELOAD && s->main_pid > 0) { - r = kill_and_sigcont(s->main_pid, s->reload_signal); + if (s->type == SERVICE_NOTIFY_RELOAD && pidref_is_set(&s->main_pid)) { + r = pidref_kill_and_sigcont(&s->main_pid, s->reload_signal); if (r < 0) { log_unit_warning_errno(UNIT(s), r, "Failed to send reload signal: %m"); goto fail; @@ -2584,6 +2598,7 @@ static void service_enter_reload(Service *s) { s->control_command = s->exec_command[SERVICE_EXEC_RELOAD]; if (s->control_command) { s->control_command_id = SERVICE_EXEC_RELOAD; + pidref_done(&s->control_pid); r = service_spawn(s, s->control_command, @@ -2634,6 +2649,8 @@ static void service_run_next_control(Service *s) { else timeout = s->timeout_stop_usec; + pidref_done(&s->control_pid); + r = service_spawn(s, s->control_command, timeout, @@ -2664,7 +2681,7 @@ fail: } static void service_run_next_main(Service *s) { - pid_t pid; + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; int r; assert(s); @@ -2679,11 +2696,11 @@ static void service_run_next_main(Service *s) { s->main_command, s->timeout_start_usec, EXEC_PASS_FDS|EXEC_APPLY_SANDBOXING|EXEC_APPLY_CHROOT|EXEC_APPLY_TTY_STDIN|EXEC_SET_WATCHDOG|EXEC_SETENV_MONITOR_RESULT|EXEC_WRITE_CREDENTIALS, - &pid); + &pidref); if (r < 0) goto fail; - (void) service_set_main_pid(s, pid); + (void) service_set_main_pid(s, pidref.pid); return; @@ -2930,11 +2947,11 @@ static int service_serialize(Unit *u, FILE *f, FDSet *fds) { (void) serialize_item(f, "result", service_result_to_string(s->result)); (void) serialize_item(f, "reload-result", service_result_to_string(s->reload_result)); - if (s->control_pid > 0) - (void) serialize_item_format(f, "control-pid", PID_FMT, s->control_pid); + if (pidref_is_set(&s->control_pid)) + (void) serialize_item_format(f, "control-pid", PID_FMT, s->control_pid.pid); - if (s->main_pid_known && s->main_pid > 0) - (void) serialize_item_format(f, "main-pid", PID_FMT, s->main_pid); + if (s->main_pid_known && pidref_is_set(&s->main_pid)) + (void) serialize_item_format(f, "main-pid", PID_FMT, s->main_pid.pid); (void) serialize_bool(f, "main-pid-known", s->main_pid_known); (void) serialize_bool(f, "bus-name-good", s->bus_name_good); @@ -3168,12 +3185,10 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value, s->reload_result = f; } else if (streq(key, "control-pid")) { - pid_t pid; - - if (parse_pid(value, &pid) < 0) - log_unit_debug(u, "Failed to parse control-pid value: %s", value); - else - s->control_pid = pid; + pidref_done(&s->control_pid); + r = pidref_set_pidstr(&s->control_pid, value); + if (r < 0) + log_unit_debug_errno(u, r, "Failed to initialize control PID '%s' from serialization, ignoring.", value); } else if (streq(key, "main-pid")) { pid_t pid; @@ -3723,7 +3738,7 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) { /* Oneshot services and non-SERVICE_EXEC_START commands should not be * considered daemons as they are typically not long running. */ - if (s->type == SERVICE_ONESHOT || (s->control_pid == pid && s->control_command_id != SERVICE_EXEC_START)) + if (s->type == SERVICE_ONESHOT || (s->control_pid.pid == pid && s->control_command_id != SERVICE_EXEC_START)) clean_mode = EXIT_CLEAN_COMMAND; else clean_mode = EXIT_CLEAN_DAEMON; @@ -3739,7 +3754,7 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) { else assert_not_reached(); - if (s->main_pid == pid) { + if (s->main_pid.pid == pid) { /* Clean up the exec_fd event source. We want to do this here, not later in * service_set_state(), because service_enter_stop_post() calls service_spawn(). * The source owns its end of the pipe, so this will close that too. */ @@ -3751,7 +3766,7 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) { if (service_load_pid_file(s, false) > 0) return; - s->main_pid = 0; + pidref_done(&s->main_pid); exec_status_exit(&s->main_exec_status, &s->exec_context, pid, code, status); if (s->main_command) { @@ -3882,11 +3897,11 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) { service_enter_start_post(s); } - } else if (s->control_pid == pid) { + } else if (s->control_pid.pid == pid) { const char *kind; bool success; - s->control_pid = 0; + pidref_done(&s->control_pid); if (s->control_command) { exec_status_exit(&s->control_command->exec_status, &s->exec_context, pid, code, status); @@ -4314,23 +4329,23 @@ static bool service_notify_message_authorized(Service *s, pid_t pid, FDSet *fds) return false; } - if (notify_access == NOTIFY_MAIN && pid != s->main_pid) { - if (s->main_pid != 0) - log_unit_warning(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID "PID_FMT, pid, s->main_pid); + if (notify_access == NOTIFY_MAIN && pid != s->main_pid.pid) { + if (pidref_is_set(&s->main_pid)) + log_unit_warning(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID "PID_FMT, pid, s->main_pid.pid); else log_unit_warning(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID which is currently not known", pid); return false; } - if (notify_access == NOTIFY_EXEC && pid != s->main_pid && pid != s->control_pid) { - if (s->main_pid != 0 && s->control_pid != 0) + if (notify_access == NOTIFY_EXEC && pid != s->main_pid.pid && pid != s->control_pid.pid) { + if (pidref_is_set(&s->main_pid) && pidref_is_set(&s->control_pid)) log_unit_warning(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID "PID_FMT" and control PID "PID_FMT, - pid, s->main_pid, s->control_pid); - else if (s->main_pid != 0) - log_unit_warning(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID "PID_FMT, pid, s->main_pid); - else if (s->control_pid != 0) - log_unit_warning(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for control PID "PID_FMT, pid, s->control_pid); + pid, s->main_pid.pid, s->control_pid.pid); + else if (pidref_is_set(&s->main_pid)) + log_unit_warning(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID "PID_FMT, pid, s->main_pid.pid); + else if (pidref_is_set(&s->control_pid)) + log_unit_warning(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for control PID "PID_FMT, pid, s->control_pid.pid); else log_unit_warning(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID and control PID which are currently not known", pid); @@ -4382,7 +4397,7 @@ static void service_notify_message( if (parse_pid(e, &new_main_pid) < 0) log_unit_warning(u, "Failed to parse MAINPID= field in notification message, ignoring: %s", e); - else if (!s->main_pid_known || new_main_pid != s->main_pid) { + else if (!s->main_pid_known || new_main_pid != s->main_pid.pid) { r = service_is_suitable_main_pid(s, new_main_pid, LOG_WARNING); if (r == 0) { @@ -4397,7 +4412,7 @@ static void service_notify_message( if (r > 0) { (void) service_set_main_pid(s, new_main_pid); - r = unit_watch_pid(UNIT(s), new_main_pid, false); + r = unit_watch_pid(UNIT(s), new_main_pid, /* exclusive= */ false); if (r < 0) log_unit_warning_errno(UNIT(s), r, "Failed to watch new main PID "PID_FMT" for service: %m", new_main_pid); @@ -4621,7 +4636,7 @@ static bool pick_up_pid_from_bus_name(Service *s) { /* If the service is running but we have no main PID yet, get it from the owner of the D-Bus name */ - return !pid_is_valid(s->main_pid) && + return !pidref_is_set(&s->main_pid) && IN_SET(s->state, SERVICE_START, SERVICE_START_POST, @@ -4667,7 +4682,7 @@ static int bus_name_pid_lookup_callback(sd_bus_message *reply, void *userdata, s log_unit_debug(u, "D-Bus name %s is now owned by process " PID_FMT, s->bus_name, (pid_t) pid); (void) service_set_main_pid(s, pid); - (void) unit_watch_pid(UNIT(s), pid, false); + (void) unit_watch_pid(UNIT(s), pid, /* exclusive= */ false); return 1; } @@ -4798,7 +4813,7 @@ static int service_kill(Unit *u, KillWho who, int signo, int code, int value, sd assert(s); - return unit_kill_common(u, who, signo, code, value, s->main_pid, s->control_pid, error); + return unit_kill_common(u, who, signo, code, value, s->main_pid.pid, s->control_pid.pid, error); } static int service_main_pid(Unit *u) { @@ -4806,7 +4821,7 @@ static int service_main_pid(Unit *u) { assert(s); - return s->main_pid; + return s->main_pid.pid; } static int service_control_pid(Unit *u) { @@ -4814,7 +4829,7 @@ static int service_control_pid(Unit *u) { assert(s); - return s->control_pid; + return s->control_pid.pid; } static bool service_needs_console(Unit *u) { @@ -4874,6 +4889,7 @@ static int service_clean(Unit *u, ExecCleanMask mask) { _cleanup_strv_free_ char **l = NULL; bool may_clean_fdstore = false; Service *s = SERVICE(u); + pid_t pid; int r; assert(s); @@ -4914,12 +4930,15 @@ static int service_clean(Unit *u, ExecCleanMask mask) { if (r < 0) goto fail; - r = unit_fork_and_watch_rm_rf(u, l, &s->control_pid); + r = unit_fork_and_watch_rm_rf(u, l, &pid); + if (r < 0) + goto fail; + + r = pidref_set_pid(&s->control_pid, pid); if (r < 0) goto fail; service_set_state(s, SERVICE_CLEANING); - return 0; fail: diff --git a/src/core/service.h b/src/core/service.h index 3bf5fb1e141..e85302e6166 100644 --- a/src/core/service.h +++ b/src/core/service.h @@ -8,6 +8,7 @@ typedef struct ServiceFDStore ServiceFDStore; #include "kill.h" #include "open-file.h" #include "path.h" +#include "pidref.h" #include "ratelimit.h" #include "socket.h" #include "unit.h" @@ -167,7 +168,7 @@ struct Service { /* Runtime data of the execution context */ ExecRuntime *exec_runtime; - pid_t main_pid, control_pid; + PidRef main_pid, control_pid; /* if we are a socket activated service instance, store information of the connection/peer/socket */ int socket_fd; From 43a19142cbdff996299617d72a924e0dd3d25868 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 9 Sep 2023 09:55:50 +0200 Subject: [PATCH 3/9] core: convert Socket logic to PidRef too Just like the previous commit, but for socket rather than service units. --- src/core/socket.c | 83 ++++++++++++++++++++++++++++++----------------- src/core/socket.h | 3 +- 2 files changed, 56 insertions(+), 30 deletions(-) diff --git a/src/core/socket.c b/src/core/socket.c index 88e45c7385f..3e569327c85 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -97,6 +97,7 @@ static void socket_init(Unit *u) { s->exec_context.std_output = u->manager->default_std_output; s->exec_context.std_error = u->manager->default_std_error; + s->control_pid = PIDREF_NULL; s->control_command_id = _SOCKET_EXEC_COMMAND_INVALID; s->trigger_limit.interval = USEC_INFINITY; @@ -106,10 +107,11 @@ static void socket_init(Unit *u) { static void socket_unwatch_control_pid(Socket *s) { assert(s); - if (s->control_pid <= 0) + if (!pidref_is_set(&s->control_pid)) return; - unit_unwatch_pid(UNIT(s), TAKE_PID(s->control_pid)); + unit_unwatch_pid(UNIT(s), s->control_pid.pid); + pidref_done(&s->control_pid); } static void socket_cleanup_fd_list(SocketPort *p) { @@ -616,10 +618,10 @@ static void socket_dump(Unit *u, FILE *f, const char *prefix) { "%sTimestamping: %s\n", prefix, socket_timestamping_to_string(s->timestamping)); - if (s->control_pid > 0) + if (pidref_is_set(&s->control_pid)) fprintf(f, "%sControl PID: "PID_FMT"\n", - prefix, s->control_pid); + prefix, s->control_pid.pid); if (s->bind_to_device) fprintf(f, @@ -1854,8 +1856,8 @@ static int socket_coldplug(Unit *u) { if (s->deserialized_state == s->state) return 0; - if (s->control_pid > 0 && - pid_is_unwaited(s->control_pid) && + if (pidref_is_set(&s->control_pid) && + pid_is_unwaited(s->control_pid.pid) && IN_SET(s->deserialized_state, SOCKET_START_PRE, SOCKET_START_CHOWN, @@ -1868,7 +1870,7 @@ static int socket_coldplug(Unit *u) { SOCKET_FINAL_SIGKILL, SOCKET_CLEANING)) { - r = unit_watch_pid(UNIT(s), s->control_pid, false); + r = unit_watch_pid(UNIT(s), s->control_pid.pid, /* exclusive= */ false); if (r < 0) return r; @@ -1914,7 +1916,7 @@ static int socket_coldplug(Unit *u) { return 0; } -static int socket_spawn(Socket *s, ExecCommand *c, pid_t *_pid) { +static int socket_spawn(Socket *s, ExecCommand *c, PidRef *ret_pid) { _cleanup_(exec_params_clear) ExecParameters exec_params = { .flags = EXEC_APPLY_SANDBOXING|EXEC_APPLY_CHROOT|EXEC_APPLY_TTY_STDIN, @@ -1923,12 +1925,13 @@ static int socket_spawn(Socket *s, ExecCommand *c, pid_t *_pid) { .stderr_fd = -EBADF, .exec_fd = -EBADF, }; + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; pid_t pid; int r; assert(s); assert(c); - assert(_pid); + assert(ret_pid); r = unit_prepare_exec(UNIT(s)); if (r < 0) @@ -1952,19 +1955,25 @@ static int socket_spawn(Socket *s, ExecCommand *c, pid_t *_pid) { if (r < 0) return r; - r = unit_watch_pid(UNIT(s), pid, true); + r = pidref_set_pid(&pidref, pid); if (r < 0) return r; - *_pid = pid; + r = unit_watch_pid(UNIT(s), pidref.pid, /* exclusive= */ true); + if (r < 0) + return r; + *ret_pid = TAKE_PIDREF(pidref); return 0; } -static int socket_chown(Socket *s, pid_t *_pid) { +static int socket_chown(Socket *s, PidRef *ret_pid) { + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; pid_t pid; int r; + assert(s); + r = socket_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_usec)); if (r < 0) goto fail; @@ -2021,11 +2030,15 @@ static int socket_chown(Socket *s, pid_t *_pid) { _exit(EXIT_SUCCESS); } - r = unit_watch_pid(UNIT(s), pid, true); + r = pidref_set_pid(&pidref, pid); if (r < 0) goto fail; - *_pid = pid; + r = unit_watch_pid(UNIT(s), pidref.pid, /* exclusive= */ true); + if (r < 0) + goto fail; + + *ret_pid = TAKE_PIDREF(pidref); return 0; fail: @@ -2069,6 +2082,8 @@ static void socket_enter_stop_post(Socket *s, SocketResult f) { s->control_command = s->exec_command[SOCKET_EXEC_STOP_POST]; if (s->control_command) { + pidref_done(&s->control_pid); + r = socket_spawn(s, s->control_command, &s->control_pid); if (r < 0) goto fail; @@ -2107,7 +2122,7 @@ static void socket_enter_signal(Socket *s, SocketState state, SocketResult f) { &s->kill_context, state_to_kill_operation(s, state), -1, - s->control_pid, + s->control_pid.pid, false); if (r < 0) goto fail; @@ -2150,6 +2165,8 @@ static void socket_enter_stop_pre(Socket *s, SocketResult f) { s->control_command = s->exec_command[SOCKET_EXEC_STOP_PRE]; if (s->control_command) { + pidref_done(&s->control_pid); + r = socket_spawn(s, s->control_command, &s->control_pid); if (r < 0) goto fail; @@ -2196,6 +2213,8 @@ static void socket_enter_start_post(Socket *s) { s->control_command = s->exec_command[SOCKET_EXEC_START_POST]; if (s->control_command) { + pidref_done(&s->control_pid); + r = socket_spawn(s, s->control_command, &s->control_pid); if (r < 0) { log_unit_warning_errno(UNIT(s), r, "Failed to run 'start-post' task: %m"); @@ -2257,6 +2276,8 @@ static void socket_enter_start_pre(Socket *s) { s->control_command = s->exec_command[SOCKET_EXEC_START_PRE]; if (s->control_command) { + pidref_done(&s->control_pid); + r = socket_spawn(s, s->control_command, &s->control_pid); if (r < 0) { log_unit_warning_errno(UNIT(s), r, "Failed to run 'start-pre' task: %m"); @@ -2436,6 +2457,8 @@ static void socket_run_next(Socket *s) { s->control_command = s->control_command->command_next; + pidref_done(&s->control_pid); + r = socket_spawn(s, s->control_command, &s->control_pid); if (r < 0) goto fail; @@ -2562,8 +2585,8 @@ static int socket_serialize(Unit *u, FILE *f, FDSet *fds) { (void) serialize_item_format(f, "n-accepted", "%u", s->n_accepted); (void) serialize_item_format(f, "n-refused", "%u", s->n_refused); - if (s->control_pid > 0) - (void) serialize_item_format(f, "control-pid", PID_FMT, s->control_pid); + if (pidref_is_set(&s->control_pid)) + (void) serialize_item_format(f, "control-pid", PID_FMT, s->control_pid.pid); if (s->control_command_id >= 0) (void) serialize_item(f, "control-command", socket_exec_command_to_string(s->control_command_id)); @@ -2644,12 +2667,10 @@ static int socket_deserialize_item(Unit *u, const char *key, const char *value, else s->n_refused += k; } else if (streq(key, "control-pid")) { - pid_t pid; - - if (parse_pid(value, &pid) < 0) - log_unit_debug(u, "Failed to parse control-pid value: %s", value); - else - s->control_pid = pid; + pidref_done(&s->control_pid); + r = pidref_set_pidstr(&s->control_pid, value); + if (r < 0) + log_debug_errno(r, "Failed to pin control PID '%s', ignoring: %m", value); } else if (streq(key, "control-command")) { SocketExecCommand id; @@ -3089,10 +3110,10 @@ static void socket_sigchld_event(Unit *u, pid_t pid, int code, int status) { assert(s); assert(pid >= 0); - if (pid != s->control_pid) + if (pid != s->control_pid.pid) return; - s->control_pid = 0; + pidref_done(&s->control_pid); if (is_clean_exit(code, status, EXIT_CLEAN_COMMAND, NULL)) f = SOCKET_SUCCESS; @@ -3366,7 +3387,7 @@ static void socket_trigger_notify(Unit *u, Unit *other) { } static int socket_kill(Unit *u, KillWho who, int signo, int code, int value, sd_bus_error *error) { - return unit_kill_common(u, who, signo, code, value, -1, SOCKET(u)->control_pid, error); + return unit_kill_common(u, who, signo, code, value, -1, SOCKET(u)->control_pid.pid, error); } static int socket_get_timeout(Unit *u, usec_t *timeout) { @@ -3402,12 +3423,13 @@ static int socket_control_pid(Unit *u) { assert(s); - return s->control_pid; + return s->control_pid.pid; } static int socket_clean(Unit *u, ExecCleanMask mask) { _cleanup_strv_free_ char **l = NULL; Socket *s = SOCKET(u); + pid_t pid; int r; assert(s); @@ -3432,12 +3454,15 @@ static int socket_clean(Unit *u, ExecCleanMask mask) { if (r < 0) goto fail; - r = unit_fork_and_watch_rm_rf(u, l, &s->control_pid); + r = unit_fork_and_watch_rm_rf(u, l, &pid); + if (r < 0) + goto fail; + + r = pidref_set_pid(&s->control_pid, pid); if (r < 0) goto fail; socket_set_state(s, SOCKET_CLEANING); - return 0; fail: diff --git a/src/core/socket.h b/src/core/socket.h index 191d27f46d1..03b11c1a692 100644 --- a/src/core/socket.h +++ b/src/core/socket.h @@ -5,6 +5,7 @@ typedef struct Socket Socket; typedef struct SocketPeer SocketPeer; #include "mount.h" +#include "pidref.h" #include "socket-util.h" #include "unit.h" @@ -103,7 +104,7 @@ struct Socket { ExecCommand* control_command; SocketExecCommand control_command_id; - pid_t control_pid; + PidRef control_pid; mode_t directory_mode; mode_t socket_mode; From 360f384fd98c4f1b0c8937fd2bb2b0daf122c04d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 9 Sep 2023 11:52:14 +0200 Subject: [PATCH 4/9] core: also port mount units to PidRef --- src/core/dbus-mount.c | 2 +- src/core/mount.c | 62 ++++++++++++++++++++++++++----------------- src/core/mount.h | 5 ++-- 3 files changed, 41 insertions(+), 28 deletions(-) diff --git a/src/core/dbus-mount.c b/src/core/dbus-mount.c index 55ad4f2c982..89f7d4fa0f9 100644 --- a/src/core/dbus-mount.c +++ b/src/core/dbus-mount.c @@ -89,7 +89,7 @@ const sd_bus_vtable bus_mount_vtable[] = { SD_BUS_PROPERTY("Options","s", property_get_options, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("Type", "s", property_get_type, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("TimeoutUSec", "t", bus_property_get_usec, offsetof(Mount, timeout_usec), SD_BUS_VTABLE_PROPERTY_CONST), - SD_BUS_PROPERTY("ControlPID", "u", bus_property_get_pid, offsetof(Mount, control_pid), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), + SD_BUS_PROPERTY("ControlPID", "u", bus_property_get_pid, offsetof(Mount, control_pid.pid), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("DirectoryMode", "u", bus_property_get_mode, offsetof(Mount, directory_mode), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("SloppyOptions", "b", bus_property_get_bool, offsetof(Mount, sloppy_options), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("LazyUnmount", "b", bus_property_get_bool, offsetof(Mount, lazy_unmount), SD_BUS_VTABLE_PROPERTY_CONST), diff --git a/src/core/mount.c b/src/core/mount.c index 931075a1ee7..5347d73f36b 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -185,6 +185,7 @@ static void mount_init(Unit *u) { * already trying to comply its last one. */ m->exec_context.same_pgrp = true; + m->control_pid = PIDREF_NULL; m->control_command_id = _MOUNT_EXEC_COMMAND_INVALID; u->ignore_on_isolate = true; @@ -223,10 +224,11 @@ static int mount_arm_timer(Mount *m, usec_t usec) { static void mount_unwatch_control_pid(Mount *m) { assert(m); - if (m->control_pid <= 0) + if (!pidref_is_set(&m->control_pid)) return; - unit_unwatch_pid(UNIT(m), TAKE_PID(m->control_pid)); + unit_unwatch_pid(UNIT(m), m->control_pid.pid); + pidref_done(&m->control_pid); } static void mount_parameters_done(MountParameters *p) { @@ -799,11 +801,11 @@ static int mount_coldplug(Unit *u) { if (m->deserialized_state == m->state) return 0; - if (m->control_pid > 0 && - pid_is_unwaited(m->control_pid) && + if (pidref_is_set(&m->control_pid) && + pid_is_unwaited(m->control_pid.pid) && MOUNT_STATE_WITH_PROCESS(m->deserialized_state)) { - r = unit_watch_pid(UNIT(m), m->control_pid, false); + r = unit_watch_pid(UNIT(m), m->control_pid.pid, /* exclusive= */ false); if (r < 0) return r; @@ -829,13 +831,13 @@ static void mount_catchup(Unit *u) { switch (m->state) { case MOUNT_DEAD: case MOUNT_FAILED: - assert(m->control_pid == 0); + assert(!pidref_is_set(&m->control_pid)); (void) unit_acquire_invocation_id(u); mount_cycle_clear(m); mount_enter_mounted(m, MOUNT_SUCCESS); break; case MOUNT_MOUNTING: - assert(m->control_pid > 0); + assert(pidref_is_set(&m->control_pid)); mount_set_state(m, MOUNT_MOUNTING_DONE); break; default: @@ -844,11 +846,11 @@ static void mount_catchup(Unit *u) { else switch (m->state) { case MOUNT_MOUNTING_DONE: - assert(m->control_pid > 0); + assert(pidref_is_set(&m->control_pid)); mount_set_state(m, MOUNT_MOUNTING); break; case MOUNT_MOUNTED: - assert(m->control_pid == 0); + assert(!pidref_is_set(&m->control_pid)); mount_enter_dead(m, MOUNT_SUCCESS); break; default: @@ -899,17 +901,17 @@ static void mount_dump(Unit *u, FILE *f, const char *prefix) { prefix, yes_no(m->read_write_only), prefix, FORMAT_TIMESPAN(m->timeout_usec, USEC_PER_SEC)); - if (m->control_pid > 0) + if (pidref_is_set(&m->control_pid)) fprintf(f, "%sControl PID: "PID_FMT"\n", - prefix, m->control_pid); + prefix, m->control_pid.pid); exec_context_dump(&m->exec_context, f, prefix); kill_context_dump(&m->kill_context, f, prefix); cgroup_context_dump(UNIT(m), f, prefix); } -static int mount_spawn(Mount *m, ExecCommand *c, pid_t *ret_pid) { +static int mount_spawn(Mount *m, ExecCommand *c, PidRef *ret_pid) { _cleanup_(exec_params_clear) ExecParameters exec_params = { .flags = EXEC_APPLY_SANDBOXING|EXEC_APPLY_CHROOT|EXEC_APPLY_TTY_STDIN, @@ -918,6 +920,7 @@ static int mount_spawn(Mount *m, ExecCommand *c, pid_t *ret_pid) { .stderr_fd = -EBADF, .exec_fd = -EBADF, }; + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; pid_t pid; int r; @@ -947,11 +950,15 @@ static int mount_spawn(Mount *m, ExecCommand *c, pid_t *ret_pid) { if (r < 0) return r; - r = unit_watch_pid(UNIT(m), pid, true); + r = pidref_set_pid(&pidref, pid); if (r < 0) return r; - *ret_pid = pid; + r = unit_watch_pid(UNIT(m), pidref.pid, /* exclusive= */ true); + if (r < 0) + return r; + + *ret_pid = TAKE_PIDREF(pidref); return 0; } @@ -1030,7 +1037,7 @@ static void mount_enter_signal(Mount *m, MountState state, MountResult f) { &m->kill_context, state_to_kill_operation(state), -1, - m->control_pid, + m->control_pid.pid, false); if (r < 0) goto fail; @@ -1356,8 +1363,8 @@ static int mount_serialize(Unit *u, FILE *f, FDSet *fds) { (void) serialize_item(f, "reload-result", mount_result_to_string(m->reload_result)); (void) serialize_item_format(f, "n-retry-umount", "%u", m->n_retry_umount); - if (m->control_pid > 0) - (void) serialize_item_format(f, "control-pid", PID_FMT, m->control_pid); + if (pidref_is_set(&m->control_pid)) + (void) serialize_item_format(f, "control-pid", PID_FMT, m->control_pid.pid); if (m->control_command_id >= 0) (void) serialize_item(f, "control-command", mount_exec_command_to_string(m->control_command_id)); @@ -1410,9 +1417,10 @@ static int mount_deserialize_item(Unit *u, const char *key, const char *value, F } else if (streq(key, "control-pid")) { - r = parse_pid(value, &m->control_pid); + pidref_done(&m->control_pid); + r = pidref_set_pidstr(&m->control_pid, value); if (r < 0) - log_unit_debug_errno(u, r, "Failed to parse control-pid value: %s", value); + log_debug_errno(r, "Failed to set control PID to '%s': %m", value); } else if (streq(key, "control-command")) { MountExecCommand id; @@ -1460,7 +1468,7 @@ static void mount_sigchld_event(Unit *u, pid_t pid, int code, int status) { assert(m); assert(pid >= 0); - if (pid != m->control_pid) + if (pid != m->control_pid.pid) return; /* So here's the thing, we really want to know before /usr/bin/mount or /usr/bin/umount exit whether @@ -1479,7 +1487,7 @@ static void mount_sigchld_event(Unit *u, pid_t pid, int code, int status) { * /proc/self/mountinfo changes before our mount/umount exits. */ (void) mount_process_proc_self_mountinfo(u->manager); - m->control_pid = 0; + pidref_done(&m->control_pid); if (is_clean_exit(code, status, EXIT_CLEAN_COMMAND, NULL)) f = MOUNT_SUCCESS; @@ -2210,7 +2218,7 @@ static int mount_kill(Unit *u, KillWho who, int signo, int code, int value, sd_b assert(m); - return unit_kill_common(u, who, signo, code, value, -1, m->control_pid, error); + return unit_kill_common(u, who, signo, code, value, -1, m->control_pid.pid, error); } static int mount_control_pid(Unit *u) { @@ -2218,12 +2226,13 @@ static int mount_control_pid(Unit *u) { assert(m); - return m->control_pid; + return m->control_pid.pid; } static int mount_clean(Unit *u, ExecCleanMask mask) { _cleanup_strv_free_ char **l = NULL; Mount *m = MOUNT(u); + pid_t pid; int r; assert(m); @@ -2248,12 +2257,15 @@ static int mount_clean(Unit *u, ExecCleanMask mask) { if (r < 0) goto fail; - r = unit_fork_and_watch_rm_rf(u, l, &m->control_pid); + r = unit_fork_and_watch_rm_rf(u, l, &pid); + if (r < 0) + goto fail; + + r = pidref_set_pid(&m->control_pid, pid); if (r < 0) goto fail; mount_set_state(m, MOUNT_CLEANING); - return 0; fail: diff --git a/src/core/mount.h b/src/core/mount.h index 25606747196..2c48d32b07d 100644 --- a/src/core/mount.h +++ b/src/core/mount.h @@ -3,8 +3,9 @@ typedef struct Mount Mount; -#include "kill.h" #include "dynamic-user.h" +#include "kill.h" +#include "pidref.h" #include "unit.h" typedef enum MountExecCommand { @@ -83,7 +84,7 @@ struct Mount { ExecCommand* control_command; MountExecCommand control_command_id; - pid_t control_pid; + PidRef control_pid; sd_event_source *timer_event_source; From 436cdf0a077e16daa5f1617460613a8a7924ccfd Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 9 Sep 2023 11:57:39 +0200 Subject: [PATCH 5/9] core: also port swap units to PidRef --- src/core/dbus-swap.c | 2 +- src/core/swap.c | 61 +++++++++++++++++++++++++------------------- src/core/swap.h | 4 ++- 3 files changed, 39 insertions(+), 28 deletions(-) diff --git a/src/core/dbus-swap.c b/src/core/dbus-swap.c index 0fa8dd10e2c..443f84aab40 100644 --- a/src/core/dbus-swap.c +++ b/src/core/dbus-swap.c @@ -42,7 +42,7 @@ const sd_bus_vtable bus_swap_vtable[] = { SD_BUS_PROPERTY("Priority", "i", property_get_priority, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("Options", "s", property_get_options, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("TimeoutUSec", "t", bus_property_get_usec, offsetof(Swap, timeout_usec), SD_BUS_VTABLE_PROPERTY_CONST), - SD_BUS_PROPERTY("ControlPID", "u", bus_property_get_pid, offsetof(Swap, control_pid), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), + SD_BUS_PROPERTY("ControlPID", "u", bus_property_get_pid, offsetof(Swap, control_pid.pid), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("Result", "s", property_get_result, offsetof(Swap, result), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("UID", "u", bus_property_get_uid, offsetof(Unit, ref_uid), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("GID", "u", bus_property_get_gid, offsetof(Unit, ref_gid), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), diff --git a/src/core/swap.c b/src/core/swap.c index c35545e031d..386725c8471 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -144,6 +144,7 @@ static void swap_init(Unit *u) { s->exec_context.std_output = u->manager->default_std_output; s->exec_context.std_error = u->manager->default_std_error; + s->control_pid = PIDREF_NULL; s->control_command_id = _SWAP_EXEC_COMMAND_INVALID; u->ignore_on_isolate = true; @@ -152,10 +153,11 @@ static void swap_init(Unit *u) { static void swap_unwatch_control_pid(Swap *s) { assert(s); - if (s->control_pid <= 0) + if (!pidref_is_set(&s->control_pid)) return; - unit_unwatch_pid(UNIT(s), TAKE_PID(s->control_pid)); + unit_unwatch_pid(UNIT(s), s->control_pid.pid); + pidref_done(&s->control_pid); } static void swap_done(Unit *u) { @@ -578,11 +580,11 @@ static int swap_coldplug(Unit *u) { if (new_state == s->state) return 0; - if (s->control_pid > 0 && - pid_is_unwaited(s->control_pid) && + if (pidref_is_set(&s->control_pid) && + pid_is_unwaited(s->control_pid.pid) && SWAP_STATE_WITH_PROCESS(new_state)) { - r = unit_watch_pid(UNIT(s), s->control_pid, false); + r = unit_watch_pid(UNIT(s), s->control_pid.pid, /* exclusive= */ false); if (r < 0) return r; @@ -642,17 +644,17 @@ static void swap_dump(Unit *u, FILE *f, const char *prefix) { "%sTimeoutSec: %s\n", prefix, FORMAT_TIMESPAN(s->timeout_usec, USEC_PER_SEC)); - if (s->control_pid > 0) + if (pidref_is_set(&s->control_pid)) fprintf(f, "%sControl PID: "PID_FMT"\n", - prefix, s->control_pid); + prefix, s->control_pid.pid); exec_context_dump(&s->exec_context, f, prefix); kill_context_dump(&s->kill_context, f, prefix); cgroup_context_dump(UNIT(s), f, prefix); } -static int swap_spawn(Swap *s, ExecCommand *c, pid_t *_pid) { +static int swap_spawn(Swap *s, ExecCommand *c, PidRef *ret_pid) { _cleanup_(exec_params_clear) ExecParameters exec_params = { .flags = EXEC_APPLY_SANDBOXING|EXEC_APPLY_CHROOT|EXEC_APPLY_TTY_STDIN, @@ -661,12 +663,13 @@ static int swap_spawn(Swap *s, ExecCommand *c, pid_t *_pid) { .stderr_fd = -EBADF, .exec_fd = -EBADF, }; + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; pid_t pid; int r; assert(s); assert(c); - assert(_pid); + assert(ret_pid); r = unit_prepare_exec(UNIT(s)); if (r < 0) @@ -690,17 +693,19 @@ static int swap_spawn(Swap *s, ExecCommand *c, pid_t *_pid) { if (r < 0) goto fail; - r = unit_watch_pid(UNIT(s), pid, true); + r = pidref_set_pid(&pidref, pid); if (r < 0) goto fail; - *_pid = pid; + r = unit_watch_pid(UNIT(s), pidref.pid, /* exclusive= */ true); + if (r < 0) + goto fail; + *ret_pid = TAKE_PIDREF(pidref); return 0; fail: s->timer_event_source = sd_event_source_disable_unref(s->timer_event_source); - return r; } @@ -766,7 +771,7 @@ static void swap_enter_signal(Swap *s, SwapState state, SwapResult f) { &s->kill_context, state_to_kill_operation(s, state), -1, - s->control_pid, + s->control_pid.pid, false); if (r < 0) goto fail; @@ -973,8 +978,8 @@ static int swap_serialize(Unit *u, FILE *f, FDSet *fds) { (void) serialize_item(f, "state", swap_state_to_string(s->state)); (void) serialize_item(f, "result", swap_result_to_string(s->result)); - if (s->control_pid > 0) - (void) serialize_item_format(f, "control-pid", PID_FMT, s->control_pid); + if (pidref_is_set(&s->control_pid)) + (void) serialize_item_format(f, "control-pid", PID_FMT, s->control_pid.pid); if (s->control_command_id >= 0) (void) serialize_item(f, "control-command", swap_exec_command_to_string(s->control_command_id)); @@ -984,6 +989,7 @@ static int swap_serialize(Unit *u, FILE *f, FDSet *fds) { static int swap_deserialize_item(Unit *u, const char *key, const char *value, FDSet *fds) { Swap *s = SWAP(u); + int r; assert(s); assert(fds); @@ -1005,12 +1011,11 @@ static int swap_deserialize_item(Unit *u, const char *key, const char *value, FD else if (f != SWAP_SUCCESS) s->result = f; } else if (streq(key, "control-pid")) { - pid_t pid; - if (parse_pid(value, &pid) < 0) - log_unit_debug(u, "Failed to parse control-pid value: %s", value); - else - s->control_pid = pid; + pidref_done(&s->control_pid); + r = pidref_set_pidstr(&s->control_pid, value); + if (r < 0) + log_debug_errno(r, "Failed to pin control PID '%s', ignoring: %m", value); } else if (streq(key, "control-command")) { SwapExecCommand id; @@ -1035,14 +1040,14 @@ static void swap_sigchld_event(Unit *u, pid_t pid, int code, int status) { assert(s); assert(pid >= 0); - if (pid != s->control_pid) + if (pid != s->control_pid.pid) return; /* Let's scan /proc/swaps before we process SIGCHLD. For the reasoning see the similar code in * mount.c */ (void) swap_process_proc_swaps(u->manager); - s->control_pid = 0; + pidref_done(&s->control_pid); if (is_clean_exit(code, status, EXIT_CLEAN_COMMAND, NULL)) f = SWAP_SUCCESS; @@ -1475,7 +1480,7 @@ static void swap_reset_failed(Unit *u) { } static int swap_kill(Unit *u, KillWho who, int signo, int code, int value, sd_bus_error *error) { - return unit_kill_common(u, who, signo, code, value, -1, SWAP(u)->control_pid, error); + return unit_kill_common(u, who, signo, code, value, -1, SWAP(u)->control_pid.pid, error); } static int swap_get_timeout(Unit *u, usec_t *timeout) { @@ -1519,12 +1524,13 @@ static int swap_control_pid(Unit *u) { assert(s); - return s->control_pid; + return s->control_pid.pid; } static int swap_clean(Unit *u, ExecCleanMask mask) { _cleanup_strv_free_ char **l = NULL; Swap *s = SWAP(u); + pid_t pid; int r; assert(s); @@ -1549,12 +1555,15 @@ static int swap_clean(Unit *u, ExecCleanMask mask) { if (r < 0) goto fail; - r = unit_fork_and_watch_rm_rf(u, l, &s->control_pid); + r = unit_fork_and_watch_rm_rf(u, l, &pid); + if (r < 0) + goto fail; + + r = pidref_set_pid(&s->control_pid, pid); if (r < 0) goto fail; swap_set_state(s, SWAP_CLEANING); - return 0; fail: diff --git a/src/core/swap.h b/src/core/swap.h index d61c7112cf6..f2cae6766b8 100644 --- a/src/core/swap.h +++ b/src/core/swap.h @@ -6,6 +6,8 @@ ***/ #include "sd-device.h" + +#include "pidref.h" #include "unit.h" typedef struct Swap Swap; @@ -73,7 +75,7 @@ struct Swap { ExecCommand* control_command; SwapExecCommand control_command_id; - pid_t control_pid; + PidRef control_pid; sd_event_source *timer_event_source; From 894a30ef3f40b3f6c97f221a30e024264ef53cbf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 9 Sep 2023 12:09:53 +0200 Subject: [PATCH 6/9] core: don't manually destroy timer when we can't spawn a child Let's stop manually destroying the timers when we fail to spawn a child. We don't do this in any of the similar codepaths in any of the unit types, only in two specific ones in socket/swap. Destroying the timer is unnecessary, since this is done anyway in the _set_state() call of each unit type if not appropriate, and every failure path here runs through that anyway. This brings all these similar codepaths into sync. --- src/core/socket.c | 10 +++------- src/core/swap.c | 14 +++++--------- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/src/core/socket.c b/src/core/socket.c index 3e569327c85..a485db67c53 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -1976,7 +1976,7 @@ static int socket_chown(Socket *s, PidRef *ret_pid) { r = socket_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_usec)); if (r < 0) - goto fail; + return r; /* We have to resolve the user names out-of-process, hence * let's fork here. It's messy, but well, what can we do? */ @@ -2032,18 +2032,14 @@ static int socket_chown(Socket *s, PidRef *ret_pid) { r = pidref_set_pid(&pidref, pid); if (r < 0) - goto fail; + return r; r = unit_watch_pid(UNIT(s), pidref.pid, /* exclusive= */ true); if (r < 0) - goto fail; + return r; *ret_pid = TAKE_PIDREF(pidref); return 0; - -fail: - s->timer_event_source = sd_event_source_disable_unref(s->timer_event_source); - return r; } static void socket_enter_dead(Socket *s, SocketResult f) { diff --git a/src/core/swap.c b/src/core/swap.c index 386725c8471..c467008fb2f 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -677,11 +677,11 @@ static int swap_spawn(Swap *s, ExecCommand *c, PidRef *ret_pid) { r = swap_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_usec)); if (r < 0) - goto fail; + return r; r = unit_set_exec_params(UNIT(s), &exec_params); if (r < 0) - goto fail; + return r; r = exec_spawn(UNIT(s), c, @@ -691,22 +691,18 @@ static int swap_spawn(Swap *s, ExecCommand *c, PidRef *ret_pid) { &s->cgroup_context, &pid); if (r < 0) - goto fail; + return r; r = pidref_set_pid(&pidref, pid); if (r < 0) - goto fail; + return r; r = unit_watch_pid(UNIT(s), pidref.pid, /* exclusive= */ true); if (r < 0) - goto fail; + return r; *ret_pid = TAKE_PIDREF(pidref); return 0; - -fail: - s->timer_event_source = sd_event_source_disable_unref(s->timer_event_source); - return r; } static void swap_enter_dead(Swap *s, SwapResult f) { From 89bad70f712a6c931b3143ff4504ac2327b569c7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 9 Sep 2023 12:35:07 +0200 Subject: [PATCH 7/9] logind: also port session leader tracking over to PidRef --- src/login/logind-action.c | 4 ++-- src/login/logind-dbus.c | 13 ++++++---- src/login/logind-inhibit.c | 14 +++++++---- src/login/logind-inhibit.h | 4 +++- src/login/logind-session-dbus.c | 2 +- src/login/logind-session.c | 42 ++++++++++++++++++++------------- src/login/logind-session.h | 3 ++- 7 files changed, 52 insertions(+), 30 deletions(-) diff --git a/src/login/logind-action.c b/src/login/logind-action.c index 42dc36b6708..650fce6a7af 100644 --- a/src/login/logind-action.c +++ b/src/login/logind-action.c @@ -239,7 +239,7 @@ int manager_handle_action( manager_is_inhibited(m, inhibit_operation, INHIBIT_BLOCK, NULL, false, false, 0, &offending)) { _cleanup_free_ char *comm = NULL, *u = NULL; - (void) get_process_comm(offending->pid, &comm); + (void) get_process_comm(offending->pid.pid, &comm); u = uid_to_name(offending->uid); /* If this is just a recheck of the lid switch then don't warn about anything */ @@ -248,7 +248,7 @@ int manager_handle_action( handle_action_to_string(handle), inhibit_what_to_string(inhibit_operation), offending->uid, strna(u), - offending->pid, strna(comm)); + offending->pid.pid, strna(comm)); return is_edge ? -EPERM : 0; } diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index dcbd195fe2d..1e453205aaa 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -665,7 +665,7 @@ static int method_list_inhibitors(sd_bus_message *message, void *userdata, sd_bu strempty(inhibitor->why), strempty(inhibit_mode_to_string(inhibitor->mode)), (uint32_t) inhibitor->uid, - (uint32_t) inhibitor->pid); + (uint32_t) inhibitor->pid.pid); if (r < 0) return r; } @@ -1625,12 +1625,12 @@ int manager_dispatch_delayed(Manager *manager, bool timeout) { if (!timeout) return 0; - (void) get_process_comm(offending->pid, &comm); + (void) get_process_comm(offending->pid.pid, &comm); u = uid_to_name(offending->uid); log_notice("Delay lock is active (UID "UID_FMT"/%s, PID "PID_FMT"/%s) but inhibitor timeout is reached.", offending->uid, strna(u), - offending->pid, strna(comm)); + offending->pid.pid, strna(comm)); } /* Actually do the operation */ @@ -3207,6 +3207,7 @@ static int method_set_wall_message( static int method_inhibit(sd_bus_message *message, void *userdata, sd_bus_error *error) { _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL; + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; const char *who, *why, *what, *mode; _cleanup_free_ char *id = NULL; _cleanup_close_ int fifo_fd = -EBADF; @@ -3279,6 +3280,10 @@ static int method_inhibit(sd_bus_message *message, void *userdata, sd_bus_error if (r < 0) return r; + r = pidref_set_pid(&pidref, pid); + if (r < 0) + return sd_bus_error_set_errnof(error, r, "Failed pin source process "PID_FMT": %m", pid); + if (hashmap_size(m->inhibitors) >= m->inhibitors_max) return sd_bus_error_setf(error, SD_BUS_ERROR_LIMITS_EXCEEDED, "Maximum number of inhibitors (%" PRIu64 ") reached, refusing further inhibitors.", @@ -3299,7 +3304,7 @@ static int method_inhibit(sd_bus_message *message, void *userdata, sd_bus_error i->what = w; i->mode = mm; - i->pid = pid; + i->pid = TAKE_PIDREF(pidref); i->uid = uid; i->why = strdup(why); i->who = strdup(who); diff --git a/src/login/logind-inhibit.c b/src/login/logind-inhibit.c index 12cd854706d..81a791cfbb9 100644 --- a/src/login/logind-inhibit.c +++ b/src/login/logind-inhibit.c @@ -47,6 +47,7 @@ int inhibitor_new(Inhibitor **ret, Manager *m, const char* id) { .mode = _INHIBIT_MODE_INVALID, .uid = UID_INVALID, .fifo_fd = -EBADF, + .pid = PIDREF_NULL, }; i->state_file = path_join("/run/systemd/inhibit", id); @@ -81,6 +82,8 @@ Inhibitor* inhibitor_free(Inhibitor *i) { free(i->state_file); free(i->fifo_path); + pidref_done(&i->pid); + return mfree(i); } @@ -110,7 +113,7 @@ static int inhibitor_save(Inhibitor *i) { inhibit_what_to_string(i->what), inhibit_mode_to_string(i->mode), i->uid, - i->pid); + i->pid.pid); if (i->who) { _cleanup_free_ char *cc = NULL; @@ -177,7 +180,7 @@ int inhibitor_start(Inhibitor *i) { log_debug("Inhibitor %s (%s) pid="PID_FMT" uid="UID_FMT" mode=%s started.", strna(i->who), strna(i->why), - i->pid, i->uid, + i->pid.pid, i->uid, inhibit_mode_to_string(i->mode)); i->started = true; @@ -195,7 +198,7 @@ void inhibitor_stop(Inhibitor *i) { if (i->started) log_debug("Inhibitor %s (%s) pid="PID_FMT" uid="UID_FMT" mode=%s stopped.", strna(i->who), strna(i->why), - i->pid, i->uid, + i->pid.pid, i->uid, inhibit_mode_to_string(i->mode)); inhibitor_remove_fifo(i); @@ -242,7 +245,8 @@ int inhibitor_load(Inhibitor *i) { } if (pid) { - r = parse_pid(pid, &i->pid); + pidref_done(&i->pid); + r = pidref_set_pidstr(&i->pid, pid); if (r < 0) log_debug_errno(r, "Failed to parse PID of inhibitor: %s", pid); } @@ -417,7 +421,7 @@ bool manager_is_inhibited( if (i->mode != mm) continue; - if (ignore_inactive && pid_is_active(m, i->pid) <= 0) + if (ignore_inactive && pid_is_active(m, i->pid.pid) <= 0) continue; if (ignore_uid && i->uid == uid) diff --git a/src/login/logind-inhibit.h b/src/login/logind-inhibit.h index 871e69a03a8..6435d41f819 100644 --- a/src/login/logind-inhibit.h +++ b/src/login/logind-inhibit.h @@ -1,6 +1,8 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once +#include "pidref.h" + typedef struct Inhibitor Inhibitor; typedef enum InhibitWhat { @@ -40,7 +42,7 @@ struct Inhibitor { char *why; InhibitMode mode; - pid_t pid; + PidRef pid; uid_t uid; dual_timestamp since; diff --git a/src/login/logind-session-dbus.c b/src/login/logind-session-dbus.c index 53a96731331..fe52cfa64e1 100644 --- a/src/login/logind-session-dbus.c +++ b/src/login/logind-session-dbus.c @@ -877,7 +877,7 @@ static const sd_bus_vtable session_vtable[] = { SD_BUS_PROPERTY("Service", "s", NULL, offsetof(Session, service), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("Desktop", "s", NULL, offsetof(Session, desktop), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("Scope", "s", NULL, offsetof(Session, scope), SD_BUS_VTABLE_PROPERTY_CONST), - SD_BUS_PROPERTY("Leader", "u", bus_property_get_pid, offsetof(Session, leader), SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("Leader", "u", bus_property_get_pid, offsetof(Session, leader.pid), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("Audit", "u", NULL, offsetof(Session, audit_id), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("Type", "s", property_get_type, offsetof(Session, type), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("Class", "s", property_get_class, offsetof(Session, class), SD_BUS_VTABLE_PROPERTY_CONST), diff --git a/src/login/logind-session.c b/src/login/logind-session.c index 4e885ce03bd..c957c93a46a 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -66,6 +66,7 @@ int session_new(Session **ret, Manager *m, const char *id) { .vtfd = -EBADF, .audit_id = AUDIT_SESSION_INVALID, .tty_validity = _TTY_VALIDITY_INVALID, + .leader = PIDREF_NULL, }; s->state_file = path_join("/run/systemd/sessions", id); @@ -128,8 +129,10 @@ Session* session_free(Session *s) { free(s->scope); } - if (pid_is_valid(s->leader)) - (void) hashmap_remove_value(s->manager->sessions_by_leader, PID_TO_PTR(s->leader), s); + if (pidref_is_set(&s->leader)) { + (void) hashmap_remove_value(s->manager->sessions_by_leader, PID_TO_PTR(s->leader.pid), s); + pidref_done(&s->leader); + } free(s->scope_job); @@ -168,6 +171,7 @@ void session_set_user(Session *s, User *u) { } int session_set_leader(Session *s, pid_t pid) { + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; int r; assert(s); @@ -175,18 +179,24 @@ int session_set_leader(Session *s, pid_t pid) { if (!pid_is_valid(pid)) return -EINVAL; - if (s->leader == pid) + if (s->leader.pid == pid) return 0; - r = hashmap_put(s->manager->sessions_by_leader, PID_TO_PTR(pid), s); + r = pidref_set_pid(&pidref, pid); if (r < 0) return r; - if (pid_is_valid(s->leader)) - (void) hashmap_remove_value(s->manager->sessions_by_leader, PID_TO_PTR(s->leader), s); + r = hashmap_put(s->manager->sessions_by_leader, PID_TO_PTR(pidref.pid), s); + if (r < 0) + return r; - s->leader = pid; - (void) audit_session_from_pid(pid, &s->audit_id); + if (pidref_is_set(&s->leader)) { + (void) hashmap_remove_value(s->manager->sessions_by_leader, PID_TO_PTR(s->leader.pid), s); + pidref_done(&s->leader); + } + + s->leader = TAKE_PIDREF(pidref); + (void) audit_session_from_pid(s->leader.pid, &s->audit_id); return 1; } @@ -323,8 +333,8 @@ int session_save(Session *s) { if (!s->vtnr) fprintf(f, "POSITION=%u\n", s->position); - if (pid_is_valid(s->leader)) - fprintf(f, "LEADER="PID_FMT"\n", s->leader); + if (pidref_is_set(&s->leader)) + fprintf(f, "LEADER="PID_FMT"\n", s->leader.pid); if (audit_session_is_valid(s->audit_id)) fprintf(f, "AUDIT=%"PRIu32"\n", s->audit_id); @@ -674,7 +684,7 @@ static int session_start_scope(Session *s, sd_bus_message *properties, sd_bus_er r = manager_start_scope( s->manager, scope, - s->leader, + s->leader.pid, s->user->slice, description, /* These two have StopWhenUnneeded= set, hence add a dep towards them */ @@ -779,7 +789,7 @@ int session_start(Session *s, sd_bus_message *properties, sd_bus_error *error) { "MESSAGE_ID=" SD_MESSAGE_SESSION_START_STR, "SESSION_ID=%s", s->id, "USER_ID=%s", s->user->user_record->user_name, - "LEADER="PID_FMT, s->leader, + "LEADER="PID_FMT, s->leader.pid, LOG_MESSAGE("New session %s of user %s.", s->id, s->user->user_record->user_name)); if (!dual_timestamp_is_set(&s->timestamp)) @@ -849,7 +859,7 @@ static int session_stop_scope(Session *s, bool force) { log_struct(s->class == SESSION_BACKGROUND ? LOG_DEBUG : LOG_INFO, "SESSION_ID=%s", s->id, "USER_ID=%s", s->user->user_record->user_name, - "LEADER="PID_FMT, s->leader, + "LEADER="PID_FMT, s->leader.pid, LOG_MESSAGE("Session %s logged out. Waiting for processes to exit.", s->id)); } @@ -907,7 +917,7 @@ int session_finalize(Session *s) { "MESSAGE_ID=" SD_MESSAGE_SESSION_STOP_STR, "SESSION_ID=%s", s->id, "USER_ID=%s", s->user->user_record->user_name, - "LEADER="PID_FMT, s->leader, + "LEADER="PID_FMT, s->leader.pid, LOG_MESSAGE("Removed session %s.", s->id)); s->timer_event_source = sd_event_source_unref(s->timer_event_source); @@ -1036,8 +1046,8 @@ int session_get_idle_hint(Session *s, dual_timestamp *t) { /* For sessions with a leader but no explicitly configured tty, let's check the controlling tty of * the leader */ - if (pid_is_valid(s->leader)) { - r = get_process_ctty_atime(s->leader, &atime); + if (pidref_is_set(&s->leader)) { + r = get_process_ctty_atime(s->leader.pid, &atime); if (r >= 0) goto found_atime; } diff --git a/src/login/logind-session.h b/src/login/logind-session.h index bf45301705a..7a4e66ff153 100644 --- a/src/login/logind-session.h +++ b/src/login/logind-session.h @@ -7,6 +7,7 @@ typedef enum KillWho KillWho; #include "list.h" #include "login-util.h" #include "logind-user.h" +#include "pidref.h" #include "string-util.h" typedef enum SessionState { @@ -87,7 +88,7 @@ struct Session { unsigned vtnr; int vtfd; - pid_t leader; + PidRef leader; uint32_t audit_id; int fifo_fd; From d8854ff1aca4434db0d7d6dcaf9fcf2f38105fb4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 9 Sep 2023 12:57:46 +0200 Subject: [PATCH 8/9] machined: port over to PidRef too --- src/machine/machine-dbus.c | 25 +++++++++-------- src/machine/machine.c | 55 +++++++++++++++++++++---------------- src/machine/machine.h | 3 +- src/machine/machined-dbus.c | 11 ++++++-- src/machine/machined.c | 7 ++++- 5 files changed, 62 insertions(+), 39 deletions(-) diff --git a/src/machine/machine-dbus.c b/src/machine/machine-dbus.c index effee717b65..6341335c4dd 100644 --- a/src/machine/machine-dbus.c +++ b/src/machine/machine-dbus.c @@ -233,7 +233,7 @@ int bus_machine_method_get_addresses(sd_bus_message *message, void *userdata, sd if (r < 0) return r; - p = procfs_file_alloca(m->leader, "ns/net"); + p = procfs_file_alloca(m->leader.pid, "ns/net"); r = readlink_malloc(p, &them); if (r < 0) return r; @@ -241,7 +241,7 @@ int bus_machine_method_get_addresses(sd_bus_message *message, void *userdata, sd if (streq(us, them)) return sd_bus_error_setf(error, BUS_ERROR_NO_PRIVATE_NETWORKING, "Machine %s does not use private networking", m->name); - r = namespace_open(m->leader, NULL, NULL, &netns_fd, NULL, NULL); + r = namespace_open(m->leader.pid, NULL, NULL, &netns_fd, NULL, NULL); if (r < 0) return r; @@ -375,7 +375,7 @@ int bus_machine_method_get_os_release(sd_bus_message *message, void *userdata, s _cleanup_fclose_ FILE *f = NULL; pid_t child; - r = namespace_open(m->leader, &pidns_fd, &mntns_fd, NULL, NULL, &root_fd); + r = namespace_open(m->leader.pid, &pidns_fd, &mntns_fd, NULL, NULL, &root_fd); if (r < 0) return r; @@ -496,7 +496,7 @@ static int container_bus_new(Machine *m, sd_bus_error *error, sd_bus **ret) { if (r < 0) return r; - if (asprintf(&address, "x-machine-unix:pid=%" PID_PRI, m->leader) < 0) + if (asprintf(&address, "x-machine-unix:pid=%" PID_PRI, m->leader.pid) < 0) return -ENOMEM; bus->address = address; @@ -880,10 +880,13 @@ int bus_machine_method_bind_mount(sd_bus_message *message, void *userdata, sd_bu return sd_bus_error_set(error, SD_BUS_ERROR_NOT_SUPPORTED, "Can't bind mount on container with user namespacing applied."); propagate_directory = strjoina("/run/systemd/nspawn/propagate/", m->name); - r = bind_mount_in_namespace(m->leader, - propagate_directory, - "/run/host/incoming/", - src, dest, read_only, make_file_or_directory); + r = bind_mount_in_namespace( + m->leader.pid, + propagate_directory, + "/run/host/incoming/", + src, dest, + read_only, + make_file_or_directory); if (r < 0) return sd_bus_error_set_errnof(error, r, "Failed to mount %s on %s in machine's namespace: %m", src, dest); @@ -997,7 +1000,7 @@ int bus_machine_method_copy(sd_bus_message *message, void *userdata, sd_bus_erro errno_pipe_fd[0] = safe_close(errno_pipe_fd[0]); - q = procfs_file_alloca(m->leader, "ns/mnt"); + q = procfs_file_alloca(m->leader.pid, "ns/mnt"); mntfd = open(q, O_RDONLY|O_NOCTTY|O_CLOEXEC); if (mntfd < 0) { r = log_error_errno(errno, "Failed to open mount namespace of leader: %m"); @@ -1093,7 +1096,7 @@ int bus_machine_method_open_root_directory(sd_bus_message *message, void *userda _cleanup_close_pair_ int pair[2] = PIPE_EBADF; pid_t child; - r = namespace_open(m->leader, NULL, &mntns_fd, NULL, NULL, &root_fd); + r = namespace_open(m->leader.pid, NULL, &mntns_fd, NULL, NULL, &root_fd); if (r < 0) return r; @@ -1266,7 +1269,7 @@ static const sd_bus_vtable machine_vtable[] = { SD_BUS_PROPERTY("Service", "s", NULL, offsetof(Machine, service), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("Unit", "s", NULL, offsetof(Machine, unit), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("Scope", "s", NULL, offsetof(Machine, unit), SD_BUS_VTABLE_PROPERTY_CONST|SD_BUS_VTABLE_HIDDEN), - SD_BUS_PROPERTY("Leader", "u", NULL, offsetof(Machine, leader), SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("Leader", "u", NULL, offsetof(Machine, leader.pid), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("Class", "s", property_get_class, offsetof(Machine, class), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("RootDirectory", "s", NULL, offsetof(Machine, root_directory), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("NetworkInterfaces", "ai", property_get_netif, 0, SD_BUS_VTABLE_PROPERTY_CONST), diff --git a/src/machine/machine.c b/src/machine/machine.c index acb86c1a83f..6f0ef3da4a6 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -49,10 +49,14 @@ int machine_new(Manager *manager, MachineClass class, const char *name, Machine * means as much as "we don't know yet", and that we'll figure * it out later when loading the state file. */ - m = new0(Machine, 1); + m = new(Machine, 1); if (!m) return -ENOMEM; + *m = (Machine) { + .leader = PIDREF_NULL, + }; + m->name = strdup(name); if (!m->name) return -ENOMEM; @@ -94,8 +98,10 @@ Machine* machine_free(Machine *m) { if (m->manager->host_machine == m) m->manager->host_machine = NULL; - if (m->leader > 0) - (void) hashmap_remove_value(m->manager->machine_leaders, PID_TO_PTR(m->leader), m); + if (pidref_is_set(&m->leader)) { + (void) hashmap_remove_value(m->manager->machine_leaders, PID_TO_PTR(m->leader.pid), m); + pidref_done(&m->leader); + } sd_bus_message_unref(m->create_message); @@ -175,8 +181,8 @@ int machine_save(Machine *m) { if (!sd_id128_is_null(m->id)) fprintf(f, "ID=" SD_ID128_FORMAT_STR "\n", SD_ID128_FORMAT_VAL(m->id)); - if (m->leader != 0) - fprintf(f, "LEADER="PID_FMT"\n", m->leader); + if (pidref_is_set(&m->leader)) + fprintf(f, "LEADER="PID_FMT"\n", m->leader.pid); if (m->class != _MACHINE_CLASS_INVALID) fprintf(f, "CLASS=%s\n", machine_class_to_string(m->class)); @@ -272,10 +278,14 @@ int machine_load(Machine *m) { return log_error_errno(r, "Failed to read %s: %m", m->state_file); if (id) - sd_id128_from_string(id, &m->id); + (void) sd_id128_from_string(id, &m->id); - if (leader) - parse_pid(leader, &m->leader); + if (leader) { + pidref_done(&m->leader); + r = pidref_set_pidstr(&m->leader, leader); + if (r < 0) + log_debug_errno(r, "Failed to set leader PID to '%s', ignoring: %m", leader); + } if (class) { MachineClass c; @@ -337,7 +347,7 @@ static int machine_start_scope( int r; assert(machine); - assert(machine->leader > 0); + assert(pidref_is_set(&machine->leader)); assert(!machine->unit); escaped = unit_name_escape(machine->name); @@ -374,7 +384,7 @@ static int machine_start_scope( return r; r = sd_bus_message_append(m, "(sv)(sv)(sv)(sv)(sv)", - "PIDs", "au", 1, machine->leader, + "PIDs", "au", 1, machine->leader.pid, "Delegate", "b", 1, "CollectMode", "s", "inactive-or-failed", "AddRef", "b", 1, @@ -440,7 +450,7 @@ int machine_start(Machine *m, sd_bus_message *properties, sd_bus_error *error) { if (m->started) return 0; - r = hashmap_put(m->manager->machine_leaders, PID_TO_PTR(m->leader), m); + r = hashmap_put(m->manager->machine_leaders, PID_TO_PTR(m->leader.pid), m); if (r < 0) return r; @@ -452,7 +462,7 @@ int machine_start(Machine *m, sd_bus_message *properties, sd_bus_error *error) { log_struct(LOG_INFO, "MESSAGE_ID=" SD_MESSAGE_MACHINE_START_STR, "NAME=%s", m->name, - "LEADER="PID_FMT, m->leader, + "LEADER="PID_FMT, m->leader.pid, LOG_MESSAGE("New machine %s.", m->name)); if (!dual_timestamp_is_set(&m->timestamp)) @@ -503,7 +513,7 @@ int machine_finalize(Machine *m) { log_struct(LOG_INFO, "MESSAGE_ID=" SD_MESSAGE_MACHINE_STOP_STR, "NAME=%s", m->name, - "LEADER="PID_FMT, m->leader, + "LEADER="PID_FMT, m->leader.pid, LOG_MESSAGE("Machine %s terminated.", m->name)); m->stopping = true; /* The machine is supposed to be going away. Don't try to kill it. */ @@ -573,7 +583,7 @@ int machine_kill(Machine *m, KillWho who, int signo) { return -ESRCH; if (who == KILL_LEADER) /* If we shall simply kill the leader, do so directly */ - return RET_NERRNO(kill(m->leader, signo)); + return pidref_kill(&m->leader, signo); /* Otherwise, make PID 1 do it for us, for the entire cgroup */ return manager_kill_unit(m->manager, m->unit, signo, NULL); @@ -585,14 +595,13 @@ int machine_openpt(Machine *m, int flags, char **ret_slave) { switch (m->class) { case MACHINE_HOST: - return openpt_allocate(flags, ret_slave); case MACHINE_CONTAINER: - if (m->leader <= 0) + if (!pidref_is_set(&m->leader)) return -EINVAL; - return openpt_allocate_in_namespace(m->leader, flags, ret_slave); + return openpt_allocate_in_namespace(m->leader.pid, flags, ret_slave); default: return -EOPNOTSUPP; @@ -608,10 +617,10 @@ int machine_open_terminal(Machine *m, const char *path, int mode) { return open_terminal(path, mode); case MACHINE_CONTAINER: - if (m->leader <= 0) + if (!pidref_is_set(&m->leader)) return -EINVAL; - return open_terminal_in_namespace(m->leader, path, mode); + return open_terminal_in_namespace(m->leader.pid, path, mode); default: return -EOPNOTSUPP; @@ -664,7 +673,7 @@ int machine_get_uid_shift(Machine *m, uid_t *ret) { if (m->class != MACHINE_CONTAINER) return -EOPNOTSUPP; - xsprintf(p, "/proc/" PID_FMT "/uid_map", m->leader); + xsprintf(p, "/proc/" PID_FMT "/uid_map", m->leader.pid); f = fopen(p, "re"); if (!f) { if (errno == ENOENT) { @@ -702,7 +711,7 @@ int machine_get_uid_shift(Machine *m, uid_t *ret) { fclose(f); - xsprintf(p, "/proc/" PID_FMT "/gid_map", m->leader); + xsprintf(p, "/proc/" PID_FMT "/gid_map", m->leader.pid); f = fopen(p, "re"); if (!f) return -errno; @@ -756,7 +765,7 @@ static int machine_owns_uid_internal( if (machine->class != MACHINE_CONTAINER) goto negative; - p = procfs_file_alloca(machine->leader, map_file); + p = procfs_file_alloca(machine->leader.pid, map_file); f = fopen(p, "re"); if (!f) { log_debug_errno(errno, "Failed to open %s, ignoring.", p); @@ -830,7 +839,7 @@ static int machine_translate_uid_internal( /* Translates a machine UID into a host UID */ - p = procfs_file_alloca(machine->leader, map_file); + p = procfs_file_alloca(machine->leader.pid, map_file); f = fopen(p, "re"); if (!f) return -errno; diff --git a/src/machine/machine.h b/src/machine/machine.h index 54ebcb3b26c..30ef93b36e7 100644 --- a/src/machine/machine.h +++ b/src/machine/machine.h @@ -7,6 +7,7 @@ typedef enum KillWho KillWho; #include "list.h" #include "machined.h" #include "operation.h" +#include "pidref.h" #include "time-util.h" typedef enum MachineState { @@ -47,7 +48,7 @@ struct Machine { char *unit; char *scope_job; - pid_t leader; + PidRef leader; dual_timestamp timestamp; diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c index 0c157a981af..979eb3cee78 100644 --- a/src/machine/machined-dbus.c +++ b/src/machine/machined-dbus.c @@ -219,6 +219,7 @@ static int method_list_machines(sd_bus_message *message, void *userdata, sd_bus_ } static int method_create_or_register_machine(Manager *manager, sd_bus_message *message, bool read_network, Machine **_m, sd_bus_error *error) { + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; const char *name, *service, *class, *root_directory; const int32_t *netif = NULL; MachineClass c; @@ -294,6 +295,10 @@ static int method_create_or_register_machine(Manager *manager, sd_bus_message *m return r; } + r = pidref_set_pid(&pidref, leader); + if (r < 0) + return sd_bus_error_set_errnof(error, r, "Failed to pin process " PID_FMT ": %m", pidref.pid); + if (hashmap_get(manager->machines, name)) return sd_bus_error_setf(error, BUS_ERROR_MACHINE_EXISTS, "Machine '%s' already exists", name); @@ -301,7 +306,7 @@ static int method_create_or_register_machine(Manager *manager, sd_bus_message *m if (r < 0) return r; - m->leader = leader; + m->leader = TAKE_PIDREF(pidref); m->class = c; m->id = id; @@ -388,11 +393,11 @@ static int method_register_machine_internal(sd_bus_message *message, bool read_n if (r < 0) return r; - r = cg_pid_get_unit(m->leader, &m->unit); + r = cg_pid_get_unit(m->leader.pid, &m->unit); if (r < 0) { r = sd_bus_error_set_errnof(error, r, "Failed to determine unit of process "PID_FMT" : %m", - m->leader); + m->leader.pid); goto fail; } diff --git a/src/machine/machined.c b/src/machine/machined.c index 926a8a7f2b6..58a407d4513 100644 --- a/src/machine/machined.c +++ b/src/machine/machined.c @@ -107,6 +107,7 @@ static Manager* manager_unref(Manager *m) { } static int manager_add_host_machine(Manager *m) { + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; _cleanup_free_ char *rd = NULL, *unit = NULL; sd_id128_t mid; Machine *t; @@ -127,11 +128,15 @@ static int manager_add_host_machine(Manager *m) { if (!unit) return log_oom(); + r = pidref_set_pid(&pidref, 1); + if (r < 0) + return log_error_errno(r, "Failed to open reference to PID 1: %m"); + r = machine_new(m, MACHINE_HOST, ".host", &t); if (r < 0) return log_error_errno(r, "Failed to create machine: %m"); - t->leader = 1; + t->leader = TAKE_PIDREF(pidref); t->id = mid; t->root_directory = TAKE_PTR(rd); From a1f7cdc636ac46ad1489f8557f2a4d945f98ed15 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 9 Sep 2023 09:56:29 +0200 Subject: [PATCH 9/9] update TODO --- TODO | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/TODO b/TODO index 3e66361eebb..cf2d13688e4 100644 --- a/TODO +++ b/TODO @@ -122,9 +122,6 @@ Deprecations and removals: * drop fd_is_mount_point() fallback mess once we can rely on STATX_ATTR_MOUNT_ROOT to exist i.e. kernel baseline 5.8 -* rework our PID tracking in services and so on, to be strictly based on pidfd, - once kernel baseline is 5.13. - * Remove /dev/mem ACPI FPDT parsing when /sys/firmware/acpi/fpdt is ubiquitous. That requires distros to enable CONFIG_ACPI_FPDT, and have kernels v5.12 for x86 and v6.2 for arm. @@ -136,6 +133,22 @@ Deprecations and removals: Features: +* PidRef conversion work: + - pid_is_unwaited() → pidref_is_unwaited() + - pid_is_alive() → pidref_is_alive() + - unit_watch_pid() → unit_watch_pidref() + - unit_kill_common() + - unit_kill_context() + - service_set_main_pid() + - actually wait for POLLIN on piref's pidfd in service logic + - unit_main_pid() + unit_control_pid() + - exec_spawn() + - serialization of control/main pid in service, socket, mount, swap units + - unit_fork_and_watch_rm_rf() + - cg_pid_get_unit() + - openpt_allocate_in_namespace() + - scope dbus PIDs property needs to gain PIDFDs companion + * ddi must be listed as block device fstype * measure some string via pcrphase whenever we end up booting into emergency @@ -1281,8 +1294,6 @@ Features: * if /usr/bin/swapoff fails due to OOM, log a friendly explanatory message about it -* pid1: Move to tracking of main pid/control pid of units per pidfd - * pid1: support new clone3() fork-into-cgroup feature * pid1: also remove PID files of a service when the service starts, not just