diff --git a/TODO b/TODO index 3e66361eeb..cf2d13688e 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 diff --git a/src/basic/meson.build b/src/basic/meson.build index 77ce2cf262..0e4c5584da 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 0000000000..f41460938c --- /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 0000000000..2411e510f1 --- /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) diff --git a/src/core/dbus-mount.c b/src/core/dbus-mount.c index 55ad4f2c98..89f7d4fa0f 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/dbus-service.c b/src/core/dbus-service.c index 6068c742ca..cc65a55990 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/dbus-swap.c b/src/core/dbus-swap.c index 0fa8dd10e2..443f84aab4 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/mount.c b/src/core/mount.c index 931075a1ee..5347d73f36 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 2560674719..2c48d32b07 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; diff --git a/src/core/service.c b/src/core/service.c index 2a02279f56..9aee6bca07 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 3bf5fb1e14..e85302e616 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; diff --git a/src/core/socket.c b/src/core/socket.c index 88e45c7385..a485db67c5 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,22 +1955,28 @@ 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; + 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? */ @@ -2021,16 +2030,16 @@ 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; + 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; - -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) { @@ -2069,6 +2078,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 +2118,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 +2161,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 +2209,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 +2272,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 +2453,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 +2581,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 +2663,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 +3106,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 +3383,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 +3419,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 +3450,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 191d27f46d..03b11c1a69 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; diff --git a/src/core/swap.c b/src/core/swap.c index c35545e031..c467008fb2 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) @@ -674,11 +677,11 @@ static int swap_spawn(Swap *s, ExecCommand *c, pid_t *_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, @@ -688,20 +691,18 @@ static int swap_spawn(Swap *s, ExecCommand *c, pid_t *_pid) { &s->cgroup_context, &pid); if (r < 0) - goto fail; + return r; - r = unit_watch_pid(UNIT(s), pid, true); + r = pidref_set_pid(&pidref, pid); if (r < 0) - goto fail; + 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; - -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) { @@ -766,7 +767,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 +974,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 +985,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 +1007,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 +1036,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 +1476,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 +1520,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 +1551,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 d61c7112cf..f2cae6766b 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; diff --git a/src/login/logind-action.c b/src/login/logind-action.c index 42dc36b670..650fce6a7a 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 dcbd195fe2..1e453205aa 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 12cd854706..81a791cfbb 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 871e69a03a..6435d41f81 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 53a9673133..fe52cfa64e 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 4e885ce03b..c957c93a46 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 bf45301705..7a4e66ff15 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; diff --git a/src/machine/machine-dbus.c b/src/machine/machine-dbus.c index effee717b6..6341335c4d 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 acb86c1a83..6f0ef3da4a 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 54ebcb3b26..30ef93b36e 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 0c157a981a..979eb3cee7 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 926a8a7f2b..58a407d451 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);