From 68db777ae26569ad9630aaa91547f4ee549276a6 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 1 Jun 2025 09:04:27 +0200 Subject: [PATCH 1/6] sd-login: reject invalid pidfd with -EBADF consistently We got it mostly right except for one function, fix it. --- src/libsystemd/sd-login/sd-login.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsystemd/sd-login/sd-login.c b/src/libsystemd/sd-login/sd-login.c index d730c5673c..133e97b432 100644 --- a/src/libsystemd/sd-login/sd-login.c +++ b/src/libsystemd/sd-login/sd-login.c @@ -273,7 +273,7 @@ _public_ int sd_pidfd_get_owner_uid(int pidfd, uid_t *ret_uid) { pid_t pid; int r; - assert_return(pidfd >= 0, -EINVAL); + assert_return(pidfd >= 0, -EBADF); r = pidfd_get_pid(pidfd, &pid); if (r < 0) From dbb28f7b2461e3029b5c2edfe8e0cceb1b119409 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 1 Jun 2025 09:06:36 +0200 Subject: [PATCH 2/6] man/sd_pid_get_owner_uid: don't limit -EBADF to socket fds --- man/sd_pid_get_owner_uid.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/sd_pid_get_owner_uid.xml b/man/sd_pid_get_owner_uid.xml index 70e13dc82f..4b5da8bccd 100644 --- a/man/sd_pid_get_owner_uid.xml +++ b/man/sd_pid_get_owner_uid.xml @@ -322,7 +322,7 @@ -EBADF - The specified socket file descriptor was invalid. + The specified file descriptor is invalid. From e09b7ced6557135becc8f5fb6dc23db753772124 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 1 Jun 2025 08:55:50 +0200 Subject: [PATCH 3/6] pidfd-util: open an internal pidfd if none is passed in pidfd_check_pidfs() I'd like to introduce a libsystemd helper for acquiring pidfd inode id, which however means the fd passed to pidfd_check_pidfs() can no longer be trusted. Let's add back the logic of allocating a genuine pidfd allocated internally, which was remove in 5dc9d5b4eacbe32f58ad6ca18d70931ab89ea409. --- src/basic/pidfd-util.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/basic/pidfd-util.c b/src/basic/pidfd-util.c index f53a94f2d3..629ffe9596 100644 --- a/src/basic/pidfd-util.c +++ b/src/basic/pidfd-util.c @@ -26,6 +26,15 @@ static int pidfd_check_pidfs(int pid_fd) { if (have_pidfs >= 0) return have_pidfs; + _cleanup_close_ int our_fd = -EBADF; + if (pid_fd < 0) { + our_fd = pidfd_open(getpid_cached(), /* flags = */ 0); + if (our_fd < 0) + return -errno; + + pid_fd = our_fd; + } + return (have_pidfs = fd_is_fs_type(pid_fd, PID_FS_MAGIC)); } From a842c26be3c57eba52bc8725ed310957ba3daeb3 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 1 Jun 2025 08:57:49 +0200 Subject: [PATCH 4/6] pidfd-util: extract pidfd_get_inode_id_impl() and make it thread safe Preparation for later commits. --- src/basic/pidfd-util.c | 28 ++++++++++++++++++---------- src/basic/pidfd-util.h | 4 +++- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/basic/pidfd-util.c b/src/basic/pidfd-util.c index 629ffe9596..78f8dc41c6 100644 --- a/src/basic/pidfd-util.c +++ b/src/basic/pidfd-util.c @@ -17,9 +17,9 @@ #include "stdio-util.h" #include "string-util.h" -static int have_pidfs = -1; +static thread_local int have_pidfs = -1; -static int pidfd_check_pidfs(int pid_fd) { +int pidfd_check_pidfs(int pid_fd) { /* NB: the passed fd *must* be acquired via pidfd_open(), i.e. must be a true pidfd! */ @@ -229,18 +229,12 @@ int pidfd_get_cgroupid(int fd, uint64_t *ret) { return 0; } -int pidfd_get_inode_id(int fd, uint64_t *ret) { - static bool file_handle_supported = true; +int pidfd_get_inode_id_impl(int fd, uint64_t *ret) { + static thread_local bool file_handle_supported = true; int r; assert(fd >= 0); - r = pidfd_check_pidfs(fd); - if (r < 0) - return r; - if (r == 0) - return -EOPNOTSUPP; - if (file_handle_supported) { union { struct file_handle file_handle; @@ -284,6 +278,20 @@ int pidfd_get_inode_id(int fd, uint64_t *ret) { #endif } +int pidfd_get_inode_id(int fd, uint64_t *ret) { + int r; + + assert(fd >= 0); + + r = pidfd_check_pidfs(fd); + if (r < 0) + return r; + if (r == 0) + return -EOPNOTSUPP; + + return pidfd_get_inode_id_impl(fd, ret); +} + 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 */ diff --git a/src/basic/pidfd-util.h b/src/basic/pidfd-util.h index 774f97018a..e998b8a3bc 100644 --- a/src/basic/pidfd-util.h +++ b/src/basic/pidfd-util.h @@ -14,6 +14,8 @@ int pidfd_get_ppid(int fd, pid_t *ret); int pidfd_get_uid(int fd, uid_t *ret); int pidfd_get_cgroupid(int fd, uint64_t *ret); +int pidfd_get_inode_id_impl(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); + +int pidfd_check_pidfs(int pid_fd); From 46b0844743f653b880df6b0dc94447f9327787dc Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 1 Jun 2025 09:12:13 +0200 Subject: [PATCH 5/6] sd-daemon: add sd_pidfd_get_inode_id() We nowadays expose pidfdid at various places, e.g. envvars and dbus properties. Also the sd_notify() MAINPID= message has been complemented with MAINPIDFDID=. But acquiring pidfdid is actually non-trivial especially considering the 32-bit case, hence let's introduce a public helper in sd-daemon specifically for that purpose. --- NEWS | 5 ++ man/rules/meson.build | 1 + man/sd-daemon.xml | 9 ++- man/sd_notify.xml | 3 +- man/sd_pidfd_get_inode_id.xml | 107 +++++++++++++++++++++++++++ src/libsystemd/libsystemd.sym | 1 + src/libsystemd/sd-daemon/sd-daemon.c | 23 ++++++ src/systemd/sd-daemon.h | 2 + 8 files changed, 145 insertions(+), 6 deletions(-) create mode 100644 man/sd_pidfd_get_inode_id.xml diff --git a/NEWS b/NEWS index 9e5f4e5e27..6f4e43da4b 100644 --- a/NEWS +++ b/NEWS @@ -1208,6 +1208,11 @@ CHANGES WITH 258 in spe: * sd-bus: a new API call sd_bus_message_dump_json() returns a JSON representation of a D-Bus message. + * sd-daemon: a new call sd_pidfd_get_inode_id() has been added + for acquiring the unique inode ID of a pidfd, coupling the + $MAINPIDFDID/$MANAGERPIDFDID and session/machine leader pidfd IDs + exposed as described above. + — , CHANGES WITH 257: diff --git a/man/rules/meson.build b/man/rules/meson.build index 7f64f28596..b5dfdb925c 100644 --- a/man/rules/meson.build +++ b/man/rules/meson.build @@ -882,6 +882,7 @@ manpages = [ 'sd_pidfd_get_user_slice', 'sd_pidfd_get_user_unit'], 'HAVE_PAM'], + ['sd_pidfd_get_inode_id', '3', [], ''], ['sd_seat_get_active', '3', ['sd_seat_can_graphical', 'sd_seat_can_tty', 'sd_seat_get_sessions'], diff --git a/man/sd-daemon.xml b/man/sd-daemon.xml index ed7905afe5..45f06caa02 100644 --- a/man/sd-daemon.xml +++ b/man/sd-daemon.xml @@ -55,10 +55,10 @@ sd_notify3, sd_booted3, sd_is_fifo3, - sd_watchdog_enabled3 - for more information about the functions implemented. In addition - to these functions, a couple of logging prefixes are defined as - macros: + sd_watchdog_enabled3, + and sd_pidfd_get_inode_id3 + for more information about the functions implemented. In addition to these functions, a couple of + logging prefixes are defined as macros: #define SD_EMERG "<0>" /* system is unusable */ #define SD_ALERT "<1>" /* action must be taken immediately */ @@ -104,6 +104,7 @@ sd_booted3 sd_is_fifo3 sd_watchdog_enabled3 + sd_pidfd_get_inode_id3 daemon7 systemd.service5 systemd.socket5 diff --git a/man/sd_notify.xml b/man/sd_notify.xml index 746789e955..f9bb56b2d4 100644 --- a/man/sd_notify.xml +++ b/man/sd_notify.xml @@ -306,8 +306,7 @@ The pidfd inode number of the new main process (specified through MAINPID=). This information can be acquired through - name_to_handle_at2 - or fstat2 + sd_pidfd_get_inode_id3 on the pidfd and is used to identify the process in a race-free fashion. Alternatively, a pidfd can be sent directly to the service manager (see MAINPIDFD=1 below). diff --git a/man/sd_pidfd_get_inode_id.xml b/man/sd_pidfd_get_inode_id.xml new file mode 100644 index 0000000000..d8dbad1375 --- /dev/null +++ b/man/sd_pidfd_get_inode_id.xml @@ -0,0 +1,107 @@ + + + + + + + + sd_pidfd_get_inode_id + systemd + + + + sd_pidfd_get_inode_id + 3 + + + + sd_pidfd_get_inode_id + Acquire the 64-bit inode ID of a PID file descriptor (PIDFD) + + + + + #include <systemd/sd-daemon.h> + + + int sd_pidfd_get_inode_id + int pidfd + uint64_t *ret + + + + + + Description + + sd_pidfd_get_inode_id() may be invoked to acquire the 64-bit inode ID of + a PID file descriptor (PIDFD), which can be used to reliably identify a process for the current boot. + + As a typical example, the service manager sets $MAINPIDFDID and $MANAGERPIDFDID + environment variables to the inode IDs of the service main process and the service manager itself, respectively, + if such functionality is supported by the kernel. + + On 64-bit architectures, the inode ID can be directly obtained via a call to + fstat2 + on a given pidfd. However, on 32-bit architectures struct stat's .st_ino + field is also 32-bit, which similar to PIDs is subject to reuse. Therefore, a second mechanism leveraging + name_to_handle_at2 + has been added to kernel in v6.14. This helper is added to simplify downstream handling of pidfd/pidfs internals. + + + + + Return Value + + On success, the function returns 0 or a positive integer. On failure, a negative errno-style + error code is returned. + + + Errors + + Returned errors may indicate the following problems: + + + + + -EOPNOTSUPP + + The stable PIDFD inode ID is not supported by the running kernel, or the system + is 32-bit and name_to_handle_at() is unavailable. + + + + -EBADF + + The specified file descriptor is invalid, or is not a PIDFD. + + + + + + + + Notes + + + + + + History + sd_pidfd_get_inode_id() was added in version 258. + + + + See Also + + + systemd1 + sd-daemon3 + sd_notify3 + systemd.exec5 + + + + diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index d16c7265f2..2cdb9152a7 100644 --- a/src/libsystemd/libsystemd.sym +++ b/src/libsystemd/libsystemd.sym @@ -1067,6 +1067,7 @@ global: sd_device_get_sysattr_value_with_size; sd_json_variant_type_from_string; sd_json_variant_type_to_string; + sd_pidfd_get_inode_id; sd_varlink_get_current_method; sd_varlink_get_description; sd_varlink_get_input_fd; diff --git a/src/libsystemd/sd-daemon/sd-daemon.c b/src/libsystemd/sd-daemon/sd-daemon.c index 14733d0f17..d7ed4e88e8 100644 --- a/src/libsystemd/sd-daemon/sd-daemon.c +++ b/src/libsystemd/sd-daemon/sd-daemon.c @@ -18,8 +18,10 @@ #include "io-util.h" #include "iovec-util.h" #include "log.h" +#include "missing_magic.h" #include "parse-util.h" #include "path-util.h" +#include "pidfd-util.h" #include "process-util.h" #include "socket-util.h" #include "stat-util.h" @@ -752,6 +754,27 @@ finish: return r; } +_public_ int sd_pidfd_get_inode_id(int pidfd, uint64_t *ret) { + int r; + + assert_return(pidfd >= 0, -EBADF); + + /* Are pidfds backed by pidfs where the unique inode id is relevant? Note that the pidfd + * passed to us is extrinsic and hence cannot be trusted to initialize our "have_pidfs" cache, + * instead pidfd_check_pidfs() will allocate one internally. */ + r = pidfd_check_pidfs(/* pid_fd = */ -EBADF); + if (r <= 0) + return -EOPNOTSUPP; + + r = fd_is_fs_type(pidfd, PID_FS_MAGIC); + if (r < 0) + return r; + if (r == 0) + return -EBADF; /* pidfs is definitely around, so it's the fd that's of invalid type */ + + return pidfd_get_inode_id_impl(pidfd, ret); +} + _public_ int sd_booted(void) { int r; diff --git a/src/systemd/sd-daemon.h b/src/systemd/sd-daemon.h index 932e8ea53e..cd9b401e92 100644 --- a/src/systemd/sd-daemon.h +++ b/src/systemd/sd-daemon.h @@ -311,6 +311,8 @@ int sd_notify_barrier(int unset_environment, uint64_t timeout); */ int sd_pid_notify_barrier(pid_t pid, int unset_environment, uint64_t timeout); +int sd_pidfd_get_inode_id(int pidfd, uint64_t *ret); + /* Returns > 0 if the system was booted with systemd. Returns < 0 on error. Returns 0 if the system was not booted with systemd. Note From b8194aa8b7d41e2f0bc1347f684452ab21d2aa0c Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Tue, 3 Jun 2025 17:15:00 +0200 Subject: [PATCH 6/6] test-daemon: add test case for sd_pidfd_get_inode_id() --- src/test/test-daemon.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/test/test-daemon.c b/src/test/test-daemon.c index 1d3ac41cdd..979097a466 100644 --- a/src/test/test-daemon.c +++ b/src/test/test-daemon.c @@ -1,15 +1,20 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ +#include + #include "sd-daemon.h" +#include "fd-util.h" #include "parse-util.h" +#include "pidfd-util.h" +#include "process-util.h" #include "strv.h" #include "tests.h" #include "time-util.h" int main(int argc, char *argv[]) { _cleanup_strv_free_ char **l = NULL; - int n, i; + int r, n, i; usec_t duration = USEC_PER_SEC / 10; test_setup_logging(LOG_DEBUG); @@ -54,5 +59,16 @@ int main(int argc, char *argv[]) { "STOPPING=1"); usleep_safe(duration); + _cleanup_close_ int fd = eventfd(0, EFD_CLOEXEC); + ASSERT_OK_ERRNO(fd); + + r = sd_pidfd_get_inode_id(fd, NULL); + ASSERT_TRUE(IN_SET(r, -EOPNOTSUPP, -EBADF)); + if (r == -EBADF) { + safe_close(fd); + ASSERT_OK_ERRNO(fd = pidfd_open(getpid_cached(), 0)); + ASSERT_OK(sd_pidfd_get_inode_id(fd, NULL)); + } + return EXIT_SUCCESS; }