From 0cdb8df6202e1b18d219c7fd0d74dd262231de78 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 11 Mar 2024 16:28:30 +0800 Subject: [PATCH 1/3] stat-util: introduce fd_inode_same --- src/basic/stat-util.h | 4 +++- src/shared/ptyfwd.c | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/basic/stat-util.h b/src/basic/stat-util.h index 47445d852a..c5dfd2db16 100644 --- a/src/basic/stat-util.h +++ b/src/basic/stat-util.h @@ -47,10 +47,12 @@ static inline int null_or_empty_path(const char *fn) { int path_is_read_only_fs(const char *path); int inode_same_at(int fda, const char *filea, int fdb, const char *fileb, int flags); - static inline int inode_same(const char *filea, const char *fileb, int flags) { return inode_same_at(AT_FDCWD, filea, AT_FDCWD, fileb, flags); } +static inline int fd_inode_same(int fda, int fdb) { + return inode_same_at(fda, NULL, fdb, NULL, AT_EMPTY_PATH); +} /* The .f_type field of struct statfs is really weird defined on * different archs. Let's give its type a name. */ diff --git a/src/shared/ptyfwd.c b/src/shared/ptyfwd.c index 17c1549132..d572a02ca4 100644 --- a/src/shared/ptyfwd.c +++ b/src/shared/ptyfwd.c @@ -884,7 +884,7 @@ int pty_forward_new( assert(f->input_fd >= 0); - same = inode_same_at(f->input_fd, NULL, f->output_fd, NULL, AT_EMPTY_PATH); + same = fd_inode_same(f->input_fd, f->output_fd); if (same < 0) return same; From 945a8210c770801c8492eda03b6e9af3ec5d03a3 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 11 Mar 2024 18:27:50 +0800 Subject: [PATCH 2/3] stat-util: struct stat could be initialized with (st_mode & S_IFMT == 0) For anonymous inodes, the result would be 0, but the struct stat is initialized obviously. So let's switch to st_dev for the check, which is guaranteed to be non-zero. Also this is completely unnecessary for statx(), since we check stx_mask first and that on its own denotes that the struct is initialized. --- src/basic/stat-util.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/basic/stat-util.c b/src/basic/stat-util.c index ec6273c7d0..70d5b74224 100644 --- a/src/basic/stat-util.c +++ b/src/basic/stat-util.c @@ -365,7 +365,7 @@ bool stat_inode_same(const struct stat *a, const struct stat *b) { * a thorough check, comparing inode nr, backing device and if the inode is still of the same type. */ return a && b && - (a->st_mode & S_IFMT) != 0 && /* We use the check for .st_mode if the structure was ever initialized */ + a->st_dev != 0 && /* is the structure ever initialized? */ ((a->st_mode ^ b->st_mode) & S_IFMT) == 0 && /* same inode type */ a->st_dev == b->st_dev && a->st_ino == b->st_ino; @@ -395,7 +395,6 @@ bool statx_inode_same(const struct statx *a, const struct statx *b) { return a && b && FLAGS_SET(a->stx_mask, STATX_TYPE|STATX_INO) && FLAGS_SET(b->stx_mask, STATX_TYPE|STATX_INO) && - (a->stx_mode & S_IFMT) != 0 && ((a->stx_mode ^ b->stx_mode) & S_IFMT) == 0 && a->stx_dev_major == b->stx_dev_major && a->stx_dev_minor == b->stx_dev_minor && From 2f41f10bcd5eb1d52e8aeeafd92d93e7efa69091 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 11 Mar 2024 16:41:51 +0800 Subject: [PATCH 3/3] pidref: use fd_inode_same to compare pidfds --- src/basic/pidref.c | 27 +++++++++++++++++++++++++++ src/basic/pidref.h | 12 +----------- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/basic/pidref.c b/src/basic/pidref.c index abe372be2e..5f1042fae8 100644 --- a/src/basic/pidref.c +++ b/src/basic/pidref.c @@ -8,6 +8,33 @@ #include "pidref.h" #include "process-util.h" #include "signal-util.h" +#include "stat-util.h" + +bool pidref_equal(const PidRef *a, const PidRef *b) { + int r; + + if (pidref_is_set(a)) { + if (!pidref_is_set(b)) + return false; + + if (a->pid != b->pid) + return false; + + if (a->fd < 0 || b->fd < 0) + return true; + + /* pidfds live in their own pidfs and each process comes with a unique inode number since + * kernel 6.8. We can safely do this on older kernels too though, as previously anonymous + * inode was used and inode number was the same for all pidfds. */ + 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 !pidref_is_set(b); +} int pidref_set_pid(PidRef *pidref, pid_t pid) { int fd; diff --git a/src/basic/pidref.h b/src/basic/pidref.h index c440c8b0e0..9920ebb9b3 100644 --- a/src/basic/pidref.h +++ b/src/basic/pidref.h @@ -19,17 +19,7 @@ static inline bool pidref_is_set(const PidRef *pidref) { return pidref && pidref->pid > 0; } -static inline bool pidref_equal(const PidRef *a, const PidRef *b) { - - if (pidref_is_set(a)) { - if (!pidref_is_set(b)) - return false; - - return a->pid == b->pid; - } - - return !pidref_is_set(b); -} +bool pidref_equal(const PidRef *a, const 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.) */