pidref: record pidfd inode number in PidRef struct

Besides internal comparisons, the inode number of pidfds
might be interesting directly to users, too. In the future
this field should also be exposed, so that it can serve as
a unique identifier of a process (but only for display,
as there's no method to map this back to a pid or pidfd).

diff --git a/src/basic/pidref.c b/src/basic/pidref.c
index 11abadff8260cc6b58be79829226e443efae7412..d9380a84ab4fe263a7f2613b2c5c037ddc5c798e 100644 (file)
--- 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 9920ebb9b3bca65e5edc2c4ab73e3114c3b86464..0581f1af1e5fbb9be8286ac988197326bc250730 100644 (file)
--- 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 2c4d894e77fe610c5f32715bc9b60e22e7307c73..53ed10d15374086c6ba1feaa86f8397c3b4d6195 100644 (file)
--- 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)));
 }