1
0
mirror of https://github.com/systemd/systemd.git synced 2025-03-21 02:50:18 +03:00

pidref: now that we have the cached pidfdid of our own process, use it

Note that this drops a lot of "const" qualifiers on PidRef arguments.
That's because pidref_is_self() suddenly might end changing the PidRef
because it acquires the pidfd ID.

We had this previously already with pidfd_equal(), but this amplifies
the problem.

I guess we C's "const" doesn't really work for stuff that contains
caches, that is just conceptually constant, but not actually.
This commit is contained in:
Lennart Poettering 2025-01-17 14:02:08 +01:00
parent afede53ae9
commit 4ace93da8c
9 changed files with 43 additions and 26 deletions

View File

@ -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 (;;) {

View File

@ -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))

View File

@ -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 */

View File

@ -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);

View File

@ -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);

View File

@ -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);

View File

@ -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);

View File

@ -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;

View File

@ -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,