diff --git a/src/basic/pidref.c b/src/basic/pidref.c index 11abadff82..d9380a84ab 100644 --- a/src/basic/pidref.c +++ b/src/basic/pidref.c @@ -32,9 +32,38 @@ static int pidfd_inode_ids_supported(void) { return (cached = fd_is_fs_type(fd, PID_FS_MAGIC)); } -bool pidref_equal(const PidRef *a, const PidRef *b) { +int pidref_acquire_pidfd_id(PidRef *pidref) { int r; + assert(pidref); + + if (!pidref_is_set(pidref)) + return -ESRCH; + + if (pidref->fd < 0) + return -ENOMEDIUM; + + if (pidref->fd_id > 0) + return 0; + + r = pidfd_inode_ids_supported(); + if (r < 0) + return r; + if (r == 0) + return -EOPNOTSUPP; + + struct stat st; + + if (fstat(pidref->fd, &st) < 0) + return log_debug_errno(errno, "Failed to get inode number of pidfd for pid " PID_FMT ": %m", + pidref->pid); + + pidref->fd_id = (uint64_t) st.st_ino; + return 0; +} + +bool pidref_equal(PidRef *a, PidRef *b) { + if (pidref_is_set(a)) { if (!pidref_is_set(b)) return false; @@ -42,20 +71,13 @@ bool pidref_equal(const PidRef *a, const PidRef *b) { if (a->pid != b->pid) return false; - if (a->fd < 0 || b->fd < 0) + /* Try to compare pidfds using their inode numbers. This way we can ensure that we don't + * spuriously consider two PidRefs equal if the pid has been reused once. Note that we + * ignore all errors here, not only EOPNOTSUPP, as fstat() might fail due to many reasons. */ + if (pidref_acquire_pidfd_id(a) < 0 || pidref_acquire_pidfd_id(b) < 0) return true; - /* pidfds live in their own pidfs and each process comes with a unique inode number since - * kernel 6.9. */ - - if (pidfd_inode_ids_supported() <= 0) - return true; - - r = fd_inode_same(a->fd, b->fd); - if (r < 0) - log_debug_errno(r, "Failed to check whether pidfds for pid " PID_FMT " are equal, assuming yes: %m", - a->pid); - return r != 0; + return a->fd_id == b->fd_id; } return !pidref_is_set(b); diff --git a/src/basic/pidref.h b/src/basic/pidref.h index 9920ebb9b3..0581f1af1e 100644 --- a/src/basic/pidref.h +++ b/src/basic/pidref.h @@ -5,8 +5,10 @@ /* An embeddable structure carrying a reference to a process. Supposed to be used when tracking processes continuously. */ 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 */ + pid_t pid; /* always valid */ + int fd; /* only valid if pidfd are available in the kernel, and we manage to get an fd */ + uint64_t fd_id; /* the inode number of pidfd. only useful in kernel 6.9+ where pidfds live in + their own pidfs and each process comes with a unique inode number */ } PidRef; #define PIDREF_NULL (const PidRef) { .fd = -EBADF } @@ -19,7 +21,8 @@ static inline bool pidref_is_set(const PidRef *pidref) { return pidref && pidref->pid > 0; } -bool pidref_equal(const PidRef *a, const PidRef *b); +int pidref_acquire_pidfd_id(PidRef *pidref); +bool pidref_equal(PidRef *a, PidRef *b); /* This turns a pid_t into a PidRef structure, and acquires a pidfd for it, if possible. (As opposed to * PIDREF_MAKE_FROM_PID() above, which does not acquire a pidfd.) */ diff --git a/src/test/test-pidref.c b/src/test/test-pidref.c index 2c4d894e77..53ed10d153 100644 --- a/src/test/test-pidref.c +++ b/src/test/test-pidref.c @@ -7,6 +7,8 @@ #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)); @@ -15,14 +17,14 @@ TEST(pidref_is_set) { TEST(pidref_equal) { assert_se(pidref_equal(NULL, NULL)); - 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_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_MAKE_FROM_PID(1))); assert_se(!pidref_equal(&PIDREF_MAKE_FROM_PID(1), NULL)); - 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_NULL_NONCONST, &PIDREF_MAKE_FROM_PID(1))); + assert_se(!pidref_equal(&PIDREF_MAKE_FROM_PID(1), &PIDREF_NULL_NONCONST)); 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))); }