diff --git a/TODO b/TODO index bc02f7bacf7..d2c451fb990 100644 --- a/TODO +++ b/TODO @@ -218,9 +218,9 @@ Features: be established on top of dm-crypt or dm-verity devices, or an allowlist of file systems (which should probably include vfat, for compat with the ESP) -* $LISTEN_PID, $MAINPID and $SYSTEMD_EXECPID env vars that the service manager - sets should be augmented with $LISTEN_PIDFDID, $MAINPIDFDID and - $SYSTEMD_EXECPIDFD (and similar for other env vars we might send). +* $LISTEN_PID, $SYSTEMD_EXECPID env vars that the service manager sets should + be augmented with $LISTEN_PIDFDID, and $SYSTEMD_EXECPIDFD (and similar for + other env vars we might send). * port copy.c over to use LabelOps for all labelling. diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml index 8a2fa448dc7..29445946409 100644 --- a/man/systemd.exec.xml +++ b/man/systemd.exec.xml @@ -3886,22 +3886,40 @@ StandardInputData=V2XigLJyZSBubyBzdHJhbmdlcnMgdG8gbG92ZQpZb3Uga25vdyB0aGUgcnVsZX $MAINPID - The PID of the unit's main process if it is - known. This is only set for control processes as invoked by - ExecReload= and similar. + The UNIX process ID (PID) of the unit's main process if it is known. This is only + set for control processes as invoked by ExecReload= and similar. + + $MAINPIDFDID + + The 64bit inode ID of the file descriptor returned by pidfd_open3 + for the main process (if supported). This is only set for control processes as invoked by + ExecReload= and similar. + + + + $MANAGERPID - The PID of the user systemd - instance, set for processes spawned by it. - + The PID of the per-user systemd service manager instance, set + for processes spawned by it. + + $MANAGERPIDFDID + + The pidfd_open() inode ID (see above) of the per-user + systemd service manager instance, set for processes spawned by it. + + + + $LISTEN_FDS $LISTEN_PID diff --git a/meson.build b/meson.build index d567f255664..4298805365a 100644 --- a/meson.build +++ b/meson.build @@ -377,6 +377,7 @@ possible_common_cc_flags = [ '-Warray-bounds=2', '-Wdate-time', '-Wendif-labels', + '-Werror=discarded-qualifiers', '-Werror=format=2', '-Werror=format-signedness', '-Werror=implicit-function-declaration', diff --git a/src/basic/pidfd-util.c b/src/basic/pidfd-util.c index c90699d066e..36fe224324a 100644 --- a/src/basic/pidfd-util.c +++ b/src/basic/pidfd-util.c @@ -9,6 +9,7 @@ #include "macro.h" #include "memory-util.h" #include "missing_magic.h" +#include "missing_threads.h" #include "parse-util.h" #include "path-util.h" #include "pidfd-util.h" @@ -18,16 +19,14 @@ static int have_pidfs = -1; -static int pidfd_check_pidfs(void) { +static int pidfd_check_pidfs(int pid_fd) { + + /* NB: the passed fd *must* be acquired via pidfd_open(), i.e. must be a true pidfd! */ if (have_pidfs >= 0) return have_pidfs; - _cleanup_close_ int fd = pidfd_open(getpid_cached(), 0); - if (fd < 0) - return -errno; - - return (have_pidfs = fd_is_fs_type(fd, PID_FS_MAGIC)); + return (have_pidfs = fd_is_fs_type(pid_fd, PID_FS_MAGIC)); } int pidfd_get_namespace(int fd, unsigned long ns_type_cmd) { @@ -231,7 +230,7 @@ int pidfd_get_inode_id(int fd, uint64_t *ret) { assert(fd >= 0); - r = pidfd_check_pidfs(); + r = pidfd_check_pidfs(fd); if (r < 0) return r; if (r == 0) @@ -245,3 +244,32 @@ int pidfd_get_inode_id(int fd, uint64_t *ret) { *ret = (uint64_t) st.st_ino; return 0; } + +int pidfd_get_inode_id_self_cached(uint64_t *ret) { + static thread_local uint64_t cached = 0; + static thread_local pid_t initialized = 0; /* < 0: cached error; == 0: invalid; > 0: valid and pid that was current */ + int r; + + assert(ret); + + if (initialized == getpid_cached()) { + *ret = cached; + return 0; + } + if (initialized < 0) + return initialized; + + _cleanup_close_ int fd = pidfd_open(getpid_cached(), 0); + if (fd < 0) + return -errno; + + r = pidfd_get_inode_id(fd, &cached); + if (ERRNO_IS_NEG_NOT_SUPPORTED(r)) + return (initialized = -EOPNOTSUPP); + if (r < 0) + return r; + + *ret = cached; + initialized = getpid_cached(); + return 0; +} diff --git a/src/basic/pidfd-util.h b/src/basic/pidfd-util.h index 374e96261b0..c20de6df676 100644 --- a/src/basic/pidfd-util.h +++ b/src/basic/pidfd-util.h @@ -17,3 +17,5 @@ int pidfd_get_uid(int fd, uid_t *ret); int pidfd_get_cgroupid(int fd, uint64_t *ret); int pidfd_get_inode_id(int fd, uint64_t *ret); + +int pidfd_get_inode_id_self_cached(uint64_t *ret); diff --git a/src/basic/pidref.c b/src/basic/pidref.c index ccfa2903b60..9b4922b1607 100644 --- a/src/basic/pidref.c +++ b/src/basic/pidref.c @@ -83,14 +83,17 @@ bool pidref_equal(PidRef *a, PidRef *b) { } int pidref_set_pid(PidRef *pidref, pid_t pid) { + uint64_t pidfdid = 0; int fd; assert(pidref); if (pid < 0) return -ESRCH; - if (pid == 0) + if (pid == 0) { pid = getpid_cached(); + (void) pidfd_get_inode_id_self_cached(&pidfdid); + } fd = pidfd_open(pid, 0); if (fd < 0) { @@ -104,6 +107,7 @@ int pidref_set_pid(PidRef *pidref, pid_t pid) { *pidref = (PidRef) { .fd = fd, .pid = pid, + .fd_id = pidfdid, }; return 0; @@ -388,17 +392,32 @@ int pidref_verify(const PidRef *pidref) { return 1; /* We have a pidfd and it still points to the PID we have, hence all is *really* OK → return 1 */ } -bool pidref_is_self(const PidRef *pidref) { - if (!pidref) +bool pidref_is_self(PidRef *pidref) { + if (!pidref_is_set(pidref)) return false; if (pidref_is_remote(pidref)) return false; - return pidref->pid == getpid_cached(); + if (pidref->pid != getpid_cached()) + return false; + + /* PID1 cannot exit, hence no point in comparing pidfd IDs, they can never change */ + if (pidref->pid == 1) + return true; + + /* Also compare pidfd ID if we can get it */ + if (pidref_acquire_pidfd_id(pidref) < 0) + return true; + + uint64_t self_id; + if (pidfd_get_inode_id_self_cached(&self_id) < 0) + return true; + + return pidref->fd_id == self_id; } -int pidref_wait(const PidRef *pidref, siginfo_t *ret, int options) { +int pidref_wait(PidRef *pidref, siginfo_t *ret, int options) { int r; if (!pidref_is_set(pidref)) @@ -424,7 +443,7 @@ int pidref_wait(const PidRef *pidref, siginfo_t *ret, int options) { return 0; } -int pidref_wait_for_terminate(const PidRef *pidref, siginfo_t *ret) { +int pidref_wait_for_terminate(PidRef *pidref, siginfo_t *ret) { int r; for (;;) { diff --git a/src/basic/pidref.h b/src/basic/pidref.h index a268af46037..9e8a39ecfbb 100644 --- a/src/basic/pidref.h +++ b/src/basic/pidref.h @@ -39,7 +39,7 @@ struct PidRef { their own pidfs and each process comes with a unique inode number */ }; -#define PIDREF_NULL (const PidRef) { .fd = -EBADF } +#define PIDREF_NULL (PidRef) { .fd = -EBADF } /* A special pidref value that we are using when a PID shall be automatically acquired from some surrounding * context, for example connection peer. Much like PIDREF_NULL it will be considered unset by @@ -77,7 +77,7 @@ static inline int pidref_set_self(PidRef *pidref) { return pidref_set_pid(pidref, 0); } -bool pidref_is_self(const PidRef *pidref); +bool pidref_is_self(PidRef *pidref); void pidref_done(PidRef *pidref); PidRef* pidref_free(PidRef *pidref); @@ -92,8 +92,8 @@ int pidref_kill(const PidRef *pidref, int sig); int pidref_kill_and_sigcont(const PidRef *pidref, int sig); int pidref_sigqueue(const PidRef *pidref, int sig, int value); -int pidref_wait(const PidRef *pidref, siginfo_t *siginfo, int options); -int pidref_wait_for_terminate(const PidRef *pidref, siginfo_t *ret); +int pidref_wait(PidRef *pidref, siginfo_t *siginfo, int options); +int pidref_wait_for_terminate(PidRef *pidref, siginfo_t *ret); static inline void pidref_done_sigkill_wait(PidRef *pidref) { if (!pidref_is_set(pidref)) diff --git a/src/basic/process-util.c b/src/basic/process-util.c index a6ea346f786..fbde9c35e67 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -1120,7 +1120,7 @@ int getenv_for_pid(pid_t pid, const char *field, char **ret) { return 0; } -int pidref_is_my_child(const PidRef *pid) { +int pidref_is_my_child(PidRef *pid) { int r; if (!pidref_is_set(pid)) @@ -1150,7 +1150,7 @@ int pid_is_my_child(pid_t pid) { return pidref_is_my_child(&PIDREF_MAKE_FROM_PID(pid)); } -int pidref_is_unwaited(const PidRef *pid) { +int pidref_is_unwaited(PidRef *pid) { int r; /* Checks whether a PID is still valid at all, including a zombie */ diff --git a/src/basic/process-util.h b/src/basic/process-util.h index cfb967c3bc4..58fff2b1740 100644 --- a/src/basic/process-util.h +++ b/src/basic/process-util.h @@ -90,9 +90,9 @@ int getenv_for_pid(pid_t pid, const char *field, char **_value); int pid_is_alive(pid_t pid); int pidref_is_alive(const PidRef *pidref); int pid_is_unwaited(pid_t pid); -int pidref_is_unwaited(const PidRef *pidref); +int pidref_is_unwaited(PidRef *pidref); int pid_is_my_child(pid_t pid); -int pidref_is_my_child(const PidRef *pidref); +int pidref_is_my_child(PidRef *pidref); int pidref_from_same_root_fs(PidRef *a, PidRef *b); bool is_main_thread(void); diff --git a/src/basic/random-util.c b/src/basic/random-util.c index 866f0ba5ed7..fec4f810358 100644 --- a/src/basic/random-util.c +++ b/src/basic/random-util.c @@ -23,6 +23,7 @@ #include "missing_syscall.h" #include "missing_threads.h" #include "parse-util.h" +#include "pidfd-util.h" #include "process-util.h" #include "random-util.h" #include "sha256.h" @@ -39,6 +40,7 @@ static void fallback_random_bytes(void *p, size_t n) { uint64_t call_id, block_id; usec_t stamp_mono, stamp_real; pid_t pid, tid; + uint64_t pidfdid; uint8_t auxval[16]; } state = { /* Arbitrary domain separation to prevent other usage of AT_RANDOM from clashing. */ @@ -51,6 +53,7 @@ static void fallback_random_bytes(void *p, size_t n) { memcpy(state.label, "systemd fallback random bytes v1", sizeof(state.label)); memcpy(state.auxval, ULONG_TO_PTR(getauxval(AT_RANDOM)), sizeof(state.auxval)); + (void) pidfd_get_inode_id_self_cached(&state.pidfdid); while (n > 0) { struct sha256_ctx ctx; diff --git a/src/core/cgroup.c b/src/core/cgroup.c index e959b307c67..8e197a0303f 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -4482,7 +4482,7 @@ Unit *manager_get_unit_by_pidref_watching(Manager *m, const PidRef *pid) { return NULL; } -Unit *manager_get_unit_by_pidref(Manager *m, const PidRef *pid) { +Unit* manager_get_unit_by_pidref(Manager *m, PidRef *pid) { Unit *u; assert(m); diff --git a/src/core/cgroup.h b/src/core/cgroup.h index 807e56c6210..08289163196 100644 --- a/src/core/cgroup.h +++ b/src/core/cgroup.h @@ -466,7 +466,7 @@ unsigned manager_dispatch_cgroup_realize_queue(Manager *m); Unit *manager_get_unit_by_cgroup(Manager *m, const char *cgroup); Unit *manager_get_unit_by_pidref_cgroup(Manager *m, const PidRef *pid); Unit *manager_get_unit_by_pidref_watching(Manager *m, const PidRef *pid); -Unit* manager_get_unit_by_pidref(Manager *m, const PidRef *pid); +Unit* manager_get_unit_by_pidref(Manager *m, PidRef *pid); Unit* manager_get_unit_by_pid(Manager *m, pid_t pid); uint64_t unit_get_ancestor_memory_min(Unit *u); diff --git a/src/core/service.c b/src/core/service.c index e5d23f87dd0..626a47f7a8f 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -36,6 +36,7 @@ #include "open-file.h" #include "parse-util.h" #include "path-util.h" +#include "pidfd-util.h" #include "process-util.h" #include "random-util.h" #include "selinux-util.h" @@ -1769,7 +1770,7 @@ static int service_spawn_internal( if (r < 0) return r; - our_env = new0(char*, 13); + our_env = new0(char*, 15); if (!our_env) return -ENOMEM; @@ -1781,14 +1782,25 @@ static int service_spawn_internal( return -ENOMEM; } - if (pidref_is_set(&s->main_pid)) + 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)) + if (pidref_acquire_pidfd_id(&s->main_pid) >= 0) + if (asprintf(our_env + n_env++, "MAINPIDFDID=%" PRIu64, s->main_pid.fd_id) < 0) + return -ENOMEM; + } + + if (MANAGER_IS_USER(UNIT(s)->manager)) { if (asprintf(our_env + n_env++, "MANAGERPID="PID_FMT, getpid_cached()) < 0) return -ENOMEM; + uint64_t pidfdid; + if (pidfd_get_inode_id_self_cached(&pidfdid) >= 0) + if (asprintf(our_env + n_env++, "MANAGERPIDFDID=%" PRIu64, pidfdid) < 0) + return -ENOMEM; + } + if (s->pid_file) if (asprintf(our_env + n_env++, "PIDFILE=%s", s->pid_file) < 0) return -ENOMEM; diff --git a/src/core/unit.c b/src/core/unit.c index 99e36fe4558..7685ab1996b 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -6007,7 +6007,7 @@ bool unit_needs_console(Unit *u) { return exec_context_may_touch_console(ec); } -int unit_pid_attachable(Unit *u, const PidRef *pid, sd_bus_error *error) { +int unit_pid_attachable(Unit *u, PidRef *pid, sd_bus_error *error) { int r; assert(u); diff --git a/src/core/unit.h b/src/core/unit.h index 8a3b812a4bf..45b7d72b7af 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -1004,7 +1004,7 @@ int unit_warn_leftover_processes(Unit *u, bool start); bool unit_needs_console(Unit *u); -int unit_pid_attachable(Unit *unit, const PidRef *pid, sd_bus_error *error); +int unit_pid_attachable(Unit *unit, PidRef *pid, sd_bus_error *error); static inline bool unit_has_job_type(Unit *u, JobType type) { return u && u->job && u->job->type == type; diff --git a/src/notify/notify.c b/src/notify/notify.c index 6afb9560c65..1c5b5e7be3d 100644 --- a/src/notify/notify.c +++ b/src/notify/notify.c @@ -80,28 +80,49 @@ static int help(void) { return 0; } -static pid_t manager_pid(void) { - const char *e; - pid_t pid; +static int get_manager_pid(PidRef *ret) { int r; + assert(ret); + /* If we run as a service managed by systemd --user the $MANAGERPID environment variable points to * the service manager's PID. */ - e = getenv("MANAGERPID"); - if (!e) - return 0; - - r = parse_pid(e, &pid); - if (r < 0) { - log_warning_errno(r, "$MANAGERPID is set to an invalid PID, ignoring: %s", e); + const char *e = getenv("MANAGERPID"); + if (!e) { + *ret = PIDREF_NULL; return 0; } - return pid; + _cleanup_(pidref_done) PidRef manager = PIDREF_NULL; + r = pidref_set_pidstr(&manager, e); + if (r < 0) + return log_warning_errno(r, "$MANAGERPID is set to an invalid PID, ignoring: %s", e); + + e = getenv("MANAGERPIDFDID"); + if (e) { + uint64_t manager_pidfd_id; + + r = safe_atou64(e, &manager_pidfd_id); + if (r < 0) + return log_warning_errno(r, "$MANAGERPIDFDID is not set to a valid inode number, ignoring: %s", e); + + r = pidref_acquire_pidfd_id(&manager); + if (r < 0) + return log_warning_errno(r, "Unable to acquire pidfd ID for manager: %m"); + + if (manager_pidfd_id != manager.fd_id) { + log_debug("$MANAGERPIDFDID doesn't match process currently referenced by $MANAGERPID, suppressing."); + *ret = PIDREF_NULL; + return 0; + } + } + + *ret = TAKE_PIDREF(manager); + return 1; } static int pidref_parent_if_applicable(PidRef *ret) { - _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL, manager = PIDREF_NULL; int r; assert(ret); @@ -112,12 +133,18 @@ static int pidref_parent_if_applicable(PidRef *ret) { /* Don't send from PID 1 or the service manager's PID (which might be distinct from 1, if we are a * --user service). That'd just be confusing for the service manager. */ - if (pidref.pid <= 1 || - pidref.pid == manager_pid()) - return pidref_set_self(ret); + if (pidref.pid == 1) + goto from_self; + + r = get_manager_pid(&manager); + if (r > 0 && pidref_equal(&pidref, &manager)) + goto from_self; *ret = TAKE_PIDREF(pidref); return 0; + +from_self: + return pidref_set_self(ret); } static int parse_argv(int argc, char *argv[]) { diff --git a/src/test/test-pidref.c b/src/test/test-pidref.c index 10033b5826a..cb7feb03471 100644 --- a/src/test/test-pidref.c +++ b/src/test/test-pidref.c @@ -7,8 +7,6 @@ #include "stdio-util.h" #include "tests.h" -#define PIDREF_NULL_NONCONST (PidRef) { .fd = -EBADF } - TEST(pidref_is_set) { assert_se(!pidref_is_set(NULL)); assert_se(!pidref_is_set(&PIDREF_NULL)); @@ -17,14 +15,14 @@ TEST(pidref_is_set) { TEST(pidref_equal) { assert_se(pidref_equal(NULL, NULL)); - assert_se(pidref_equal(NULL, &PIDREF_NULL_NONCONST)); - assert_se(pidref_equal(&PIDREF_NULL_NONCONST, NULL)); - assert_se(pidref_equal(&PIDREF_NULL_NONCONST, &PIDREF_NULL_NONCONST)); + assert_se(pidref_equal(NULL, &PIDREF_NULL)); + assert_se(pidref_equal(&PIDREF_NULL, NULL)); + assert_se(pidref_equal(&PIDREF_NULL, &PIDREF_NULL)); assert_se(!pidref_equal(NULL, &PIDREF_MAKE_FROM_PID(1))); assert_se(!pidref_equal(&PIDREF_MAKE_FROM_PID(1), NULL)); - assert_se(!pidref_equal(&PIDREF_NULL_NONCONST, &PIDREF_MAKE_FROM_PID(1))); - assert_se(!pidref_equal(&PIDREF_MAKE_FROM_PID(1), &PIDREF_NULL_NONCONST)); + assert_se(!pidref_equal(&PIDREF_NULL, &PIDREF_MAKE_FROM_PID(1))); + assert_se(!pidref_equal(&PIDREF_MAKE_FROM_PID(1), &PIDREF_NULL)); assert_se(pidref_equal(&PIDREF_MAKE_FROM_PID(1), &PIDREF_MAKE_FROM_PID(1))); assert_se(!pidref_equal(&PIDREF_MAKE_FROM_PID(1), &PIDREF_MAKE_FROM_PID(2))); } @@ -231,7 +229,7 @@ TEST(pidref_is_remote) { assert_se(!pidref_is_remote(&PIDREF_MAKE_FROM_PID(getpid_cached()))); assert_se(!pidref_is_remote(&PIDREF_AUTOMATIC)); - static const PidRef p = { + PidRef p = { .pid = 1, .fd = -EREMOTE, .fd_id = 4711, diff --git a/src/test/test-process-util.c b/src/test/test-process-util.c index 166c0fa2c9e..0db49fe3a8f 100644 --- a/src/test/test-process-util.c +++ b/src/test/test-process-util.c @@ -28,6 +28,7 @@ #include "missing_syscall.h" #include "namespace-util.h" #include "parse-util.h" +#include "pidfd-util.h" #include "process-util.h" #include "procfs-util.h" #include "rlimit-util.h" @@ -1065,6 +1066,21 @@ TEST(pidref_from_same_root_fs) { ASSERT_OK_ZERO(pidref_from_same_root_fs(&child2, &self)); } +TEST(pidfd_get_inode_id_self_cached) { + int r; + + log_info("pid=" PID_FMT, getpid_cached()); + + uint64_t id; + r = pidfd_get_inode_id_self_cached(&id); + if (ERRNO_IS_NEG_NOT_SUPPORTED(r)) + log_info("pidfdid not supported"); + else { + assert(r >= 0); + log_info("pidfdid=%" PRIu64, id); + } +} + static int intro(void) { log_show_color(true); return EXIT_SUCCESS;