tree-wide: extend $LISTEN_FDS protocol with $LISTEN_PIDFDID

Although extremely unlikely, there is a race present in solely checking the
$LISTEN_PID environment variable, due to PID recycling. Fix that by introducing
$LISTEN_PIDFDID, which contains the 64-bit ID of a pidfd for the child process
that is not subject to recycling.

diff --git a/TODO b/TODO
index 16bf6c80af404a2a628b359d2e55f017e5a6db53..5f9739518872616927c15c10764ff1c6a64918ce 100644 (file)
--- a/TODO
+++ b/TODO
@@ -529,8 +529,8 @@ Features:
   be established on top of dm-crypt or dm-verity devices, or an allowlist of
   file systems (which should probably include vfat, for compat with the ESP)
 
-* $LISTEN_PID, $SYSTEMD_EXECPID env vars that the service manager sets should
-  be augmented with $LISTEN_PIDFDID, and $SYSTEMD_EXECPIDFD (and similar for
+* $SYSTEMD_EXECPID that the service manager sets should
+  be augmented with $SYSTEMD_EXECPIDFD (and similar for
   other env vars we might send).
 
 * port copy.c over to use LabelOps for all labelling.

diff --git a/docs/FILE_DESCRIPTOR_STORE.md b/docs/FILE_DESCRIPTOR_STORE.md
index 757a2a270804c9f89681d42d73aedcf732b7d559..00b2f3f4aaedcd10739a41f0ceea9ed52f3fcfc0 100644 (file)
--- a/docs/FILE_DESCRIPTOR_STORE.md
+++ b/docs/FILE_DESCRIPTOR_STORE.md
@@ -65,9 +65,9 @@ string of choice, which may be used to identify the fd later.
 
 Whenever the service is restarted the fds in its fdstore will be passed to the
 new instance following the same protocol as for socket activation fds. i.e. the
-`$LISTEN_FDS`, `$LISTEN_PIDS`, `$LISTEN_FDNAMES` environment variables will be
-set (the latter will be populated from the `FDNAME=…` field mentioned
-above). See
+`$LISTEN_FDS`, `$LISTEN_PID`, `$LISTEN_PIDFDID`, and `$LISTEN_FDNAMES`
+environment variables will be set (the latter will be populated from the
+`FDNAME=…` field mentioned above). See
 [`sd_listen_fds()`](https://www.freedesktop.org/software/systemd/man/sd_listen_fds.html)
 for details on receiving such fds in a service. (Note that the name set in
 `FDNAME=…` does not need to be unique, which is useful when operating with

diff --git a/man/sd_listen_fds.xml b/man/sd_listen_fds.xml
index f8a6d43ffaf63ad484e56f0a8aa3239da26c89b1..19b0b0a98d3ee53e14342dc289215153dd59f933 100644 (file)
--- a/man/sd_listen_fds.xml
+++ b/man/sd_listen_fds.xml
@@ -93,10 +93,10 @@
 
     <para>If the <parameter>unset_environment</parameter> parameter is
     non-zero, <function>sd_listen_fds()</function> will unset the
-    <varname>$LISTEN_FDS</varname>, <varname>$LISTEN_PID</varname> and
-    <varname>$LISTEN_FDNAMES</varname> environment variables before
-    returning (regardless of whether the function call itself
-    succeeded or not). Further calls to
+    <varname>$LISTEN_FDS</varname>, <varname>$LISTEN_PID</varname>,
+    <varname>$LISTEN_PIDFDID</varname>, and <varname>$LISTEN_FDNAMES</varname>
+    environment variables before returning (regardless of whether the function
+    call itself succeeded or not). Further calls to
     <function>sd_listen_fds()</function> will then return zero, but the
     variables are no longer inherited by child processes.</para>
 
@@ -175,8 +175,9 @@
     <para>On failure, these calls returns a negative errno-style error
     code. If
     <varname>$LISTEN_FDS</varname>/<varname>$LISTEN_PID</varname> was
-    not set or was not correctly set for this daemon and hence no file
-    descriptors were received, 0 is returned. Otherwise, the number of
+    not set, or one of those variables or <varname>$LISTEN_PIDFDID</varname>
+    was not correctly set for this daemon, no file
+    descriptors were received and 0 is returned. Otherwise, the number of
     file descriptors passed is returned. The application may find them
     starting with file descriptor SD_LISTEN_FDS_START, i.e. file
     descriptor 3.</para>
@@ -190,7 +191,10 @@
 
     <para>Internally, <function>sd_listen_fds()</function> checks
     whether the <varname>$LISTEN_PID</varname> environment variable
-    equals the daemon PID. If not, it returns immediately. Otherwise,
+    equals the daemon PID. If not, it returns immediately. Since version 259,
+    if the <varname>$LISTEN_PIDFDID</varname> environment variable is set,
+    it also checks whether it matches the inode of this process' pidfd.
+    If both of these checks pass,
     it parses the number passed in the <varname>$LISTEN_FDS</varname>
     environment variable, then sets the FD_CLOEXEC flag for the parsed
     number of file descriptors starting from SD_LISTEN_FDS_START.
@@ -209,6 +213,7 @@
     <variablelist class='environment-variables'>
       <varlistentry>
         <term><varname>$LISTEN_PID</varname></term>
+        <term><varname>$LISTEN_PIDFDID</varname></term>
         <term><varname>$LISTEN_FDS</varname></term>
         <term><varname>$LISTEN_FDNAMES</varname></term>
 

diff --git a/man/systemd-socket-activate.xml b/man/systemd-socket-activate.xml
index ac9deaf4b7538ea8f5eb81867cc09105f9e74aea..61139a029161eb879a00f3b6c49ea2a7c9bc4739 100644 (file)
--- a/man/systemd-socket-activate.xml
+++ b/man/systemd-socket-activate.xml
@@ -157,6 +157,7 @@
       <varlistentry>
         <term><varname>$LISTEN_FDS</varname></term>
         <term><varname>$LISTEN_PID</varname></term>
+        <term><varname>$LISTEN_PIDFDID</varname></term>
         <term><varname>$LISTEN_FDNAMES</varname></term>
 
         <listitem><para>See

diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml
index f3bb1371be0b24207db935ece1e56960f3100d54..d6ceb1490b11e26a471611e47b32ce9d8e90f2f5 100644 (file)
--- a/man/systemd.exec.xml
+++ b/man/systemd.exec.xml
@@ -4207,6 +4207,7 @@ StandardInputData=V2XigLJyZSBubyBzdHJhbmdlcnMgdG8gbG92ZQpZb3Uga25vdyB0aGUgcnVsZX
         <varlistentry>
           <term><varname>$LISTEN_FDS</varname></term>
           <term><varname>$LISTEN_PID</varname></term>
+          <term><varname>$LISTEN_PIDFDID</varname></term>
           <term><varname>$LISTEN_FDNAMES</varname></term>
 
           <listitem><para>Information about file descriptors passed to a

diff --git a/man/systemd.xml b/man/systemd.xml
index 857c942eed10087de9bfc92f45d104edab5e04d4..06d9102e475f165b6873dc8f7c17fb229ceccb3c 100644 (file)
--- a/man/systemd.xml
+++ b/man/systemd.xml
@@ -720,6 +720,7 @@
 
       <varlistentry>
         <term><varname>$LISTEN_PID</varname></term>
+        <term><varname>$LISTEN_PIDFDID</varname></term>
         <term><varname>$LISTEN_FDS</varname></term>
         <term><varname>$LISTEN_FDNAMES</varname></term>
 

diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c
index 8042dbcd77e2722adcb5afa5f50f4227296a1dc6..de59e1148ecbdc4bd9985bcf54584e34803e21a2 100644 (file)
--- a/src/core/exec-invoke.c
+++ b/src/core/exec-invoke.c
@@ -59,6 +59,7 @@
 #include "osc-context.h"
 #include "pam-util.h"
 #include "path-util.h"
+#include "pidfd-util.h"
 #include "pidref.h"
 #include "proc-cmdline.h"
 #include "process-util.h"
@@ -2014,7 +2015,7 @@ static int build_environment(
         assert(cgroup_context);
         assert(ret);
 
-#define N_ENV_VARS 19
+#define N_ENV_VARS 20
         our_env = new0(char*, N_ENV_VARS + _EXEC_DIRECTORY_TYPE_MAX + 1);
         if (!our_env)
                 return -ENOMEM;
@@ -2026,6 +2027,13 @@ static int build_environment(
                         return -ENOMEM;
                 our_env[n_env++] = x;
 
+                uint64_t pidfdid;
+                if (pidfd_get_inode_id_self_cached(&pidfdid) >= 0) {
+                        if (asprintf(&x, "LISTEN_PIDFDID=%"PRIu64, pidfdid) < 0)
+                                return -ENOMEM;
+                        our_env[n_env++] = x;
+                }
+
                 if (asprintf(&x, "LISTEN_FDS=%zu", n_fds) < 0)
                         return -ENOMEM;
                 our_env[n_env++] = x;

diff --git a/src/core/manager.c b/src/core/manager.c
index 520434993f8d06ad288d59f0c940e780463a50d2..90a3f25aebda403177e3eca7c460e685be34489a 100644 (file)
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -623,6 +623,7 @@ static char** sanitize_environment(char **l) {
                         "LISTEN_FDNAMES",
                         "LISTEN_FDS",
                         "LISTEN_PID",
+                        "LISTEN_PIDFDID",
                         "LOGS_DIRECTORY",
                         "LOG_NAMESPACE",
                         "MAINPID",

diff --git a/src/libsystemd/sd-daemon/sd-daemon.c b/src/libsystemd/sd-daemon/sd-daemon.c
index 0cf3cbdd4cd20867206a7f287bfb5c3512960579..b5fe6dfc120f4eac3844eec00d43482f550bd98d 100644 (file)
--- a/src/libsystemd/sd-daemon/sd-daemon.c
+++ b/src/libsystemd/sd-daemon/sd-daemon.c
@@ -35,6 +35,7 @@ static void unsetenv_listen(bool unset_environment) {
                 return;
 
         assert_se(unsetenv("LISTEN_PID") == 0);
+        assert_se(unsetenv("LISTEN_PIDFDID") == 0);
         assert_se(unsetenv("LISTEN_FDS") == 0);
         assert_se(unsetenv("LISTEN_FDNAMES") == 0);
 }
@@ -60,6 +61,23 @@ _public_ int sd_listen_fds(int unset_environment) {
                 goto finish;
         }
 
+        e = getenv("LISTEN_PIDFDID");
+        if (e) {
+                uint64_t own_pidfdid, pidfdid;
+
+                r = safe_atou64(e, &pidfdid);
+                if (r < 0)
+                        goto finish;
+
+                if (pidfd_get_inode_id_self_cached(&own_pidfdid) >= 0) {
+                        /* Is this *really* for us? */
+                        if (pidfdid != own_pidfdid) {
+                                r = 0;
+                                goto finish;
+                        }
+                }
+        }
+
         e = getenv("LISTEN_FDS");
         if (!e) {
                 r = 0;

diff --git a/src/libsystemd/sd-varlink/sd-varlink.c b/src/libsystemd/sd-varlink/sd-varlink.c
index 47d110ffdfd1ab60bc882eaef9aae389bc31a679..7a5d2008c97f464a1cb3f3835a344ccd62e195f4 100644 (file)
--- a/src/libsystemd/sd-varlink/sd-varlink.c
+++ b/src/libsystemd/sd-varlink/sd-varlink.c
@@ -24,6 +24,7 @@
 #include "log.h"
 #include "mkdir.h"
 #include "path-util.h"
+#include "pidfd-util.h"
 #include "process-util.h"
 #include "socket-util.h"
 #include "string-table.h"
@@ -266,6 +267,18 @@ _public_ int sd_varlink_connect_exec(sd_varlink **ret, const char *_command, cha
 
                 xsprintf(spid, PID_FMT, pid);
 
+                uint64_t pidfdid;
+                if (pidfd_get_inode_id_self_cached(&pidfdid) >= 0) {
+                        char spidfdid[DECIMAL_STR_MAX(uint64_t)+1];
+                        xsprintf(spidfdid, "%" PRIu64, pidfdid);
+
+                        if (setenv("LISTEN_PIDFDID", spidfdid, /* override= */ true) < 0) {
+                                log_debug_errno(errno,
+                                                "Failed to set environment variable 'LISTEN_PIDFDID': %m");
+                                _exit(EXIT_FAILURE);
+                        }
+                }
+
                 STRV_FOREACH_PAIR(a, b, setenv_list) {
                         if (setenv(*a, *b, /* override= */ true) < 0) {
                                 log_debug_errno(errno, "Failed to set environment variable '%s': %m", *a);

diff --git a/src/mountfsd/mountfsd-manager.c b/src/mountfsd/mountfsd-manager.c
index c81671da32dbda534f8ffd4d74c77abc9c4b130d..89e5a5aca2c79e1257e224e2b665a6286bfe028b 100644 (file)
--- a/src/mountfsd/mountfsd-manager.c
+++ b/src/mountfsd/mountfsd-manager.c
@@ -14,6 +14,7 @@
 #include "format-util.h"
 #include "log.h"
 #include "mountfsd-manager.h"
+#include "pidfd-util.h"
 #include "process-util.h"
 #include "set.h"
 #include "signal-util.h"
@@ -165,6 +166,17 @@ static int start_one_worker(Manager *m) {
                         _exit(EXIT_FAILURE);
                 }
 
+                uint64_t pidfdid;
+                if (pidfd_get_inode_id_self_cached(&pidfdid) >= 0) {
+                        char pidfdids[DECIMAL_STR_MAX(uint64_t)];
+
+                        xsprintf(pidfdids, "%" PRIu64, pidfdid);
+                        if (setenv("LISTEN_PIDFDID", pidfdids, 1) < 0) {
+                                log_error_errno(errno, "Failed to set $LISTEN_PIDFDID: %m");
+                                _exit(EXIT_FAILURE);
+                        }
+                }
+
                 if (setenv("LISTEN_FDS", "1", 1) < 0) {
                         log_error_errno(errno, "Failed to set $LISTEN_FDS: %m");
                         _exit(EXIT_FAILURE);

diff --git a/src/nsresourced/nsresourced-manager.c b/src/nsresourced/nsresourced-manager.c
index d58d88c7afd2fc868de55f7eb39586d4d9e0ee11..e57797503b2844add52d01e2cec64040fffb96c7 100644 (file)
--- a/src/nsresourced/nsresourced-manager.c
+++ b/src/nsresourced/nsresourced-manager.c
@@ -20,6 +20,7 @@
 #include "mkdir.h"
 #include "nsresourced-manager.h"
 #include "parse-util.h"
+#include "pidfd-util.h"
 #include "process-util.h"
 #include "recurse-dir.h"
 #include "set.h"
@@ -201,6 +202,17 @@ static int start_one_worker(Manager *m) {
                         _exit(EXIT_FAILURE);
                 }
 
+                uint64_t pidfdid;
+                if (pidfd_get_inode_id_self_cached(&pidfdid) >= 0) {
+                        char pidfdids[DECIMAL_STR_MAX(uint64_t)];
+
+                        xsprintf(pidfdids, "%" PRIu64, pidfdid);
+                        if (setenv("LISTEN_PIDFDID", pidfdids, 1) < 0) {
+                                log_error_errno(errno, "Failed to set $LISTEN_PIDFDID: %m");
+                                _exit(EXIT_FAILURE);
+                        }
+                }
+
                 if (setenv("LISTEN_FDS", "1", 1) < 0) {
                         log_error_errno(errno, "Failed to set $LISTEN_FDS: %m");
                         _exit(EXIT_FAILURE);

diff --git a/src/socket-activate/socket-activate.c b/src/socket-activate/socket-activate.c
index 8e5dfb41d4bf61a63854757fa968199a302ccffe..29940ac354c1543833361d878025ec7b392a6326 100644 (file)
--- a/src/socket-activate/socket-activate.c
+++ b/src/socket-activate/socket-activate.c
@@ -17,6 +17,7 @@
 #include "format-util.h"
 #include "log.h"
 #include "main-func.h"
+#include "pidfd-util.h"
 #include "pretty-print.h"
 #include "process-util.h"
 #include "socket-netlink.h"
@@ -186,6 +187,13 @@ static int exec_process(char * const *argv, int start_fd, size_t n_fds) {
                 if (r < 0)
                         return r;
 
+                uint64_t pidfdid;
+                if (pidfd_get_inode_id_self_cached(&pidfdid) >= 0) {
+                        r = strv_extendf(&envp, "LISTEN_PIDFDID=%" PRIu64, pidfdid);
+                        if (r < 0)
+                                return r;
+                }
+
                 if (arg_fdnames) {
                         _cleanup_free_ char *names = NULL;
                         size_t len;

diff --git a/src/systemd/sd-daemon.h b/src/systemd/sd-daemon.h
index b6ec73465646b3329c35fc7352c2986e4d41fcc9..de5fad7b80b166fce8808b933ebcdbcecd2dea82 100644 (file)
--- a/src/systemd/sd-daemon.h
+++ b/src/systemd/sd-daemon.h
@@ -55,16 +55,16 @@ _SD_BEGIN_DECLARATIONS;
 
 /*
   Returns how many file descriptors have been passed, or a negative
-  errno code on failure. Optionally, removes the $LISTEN_FDS and
-  $LISTEN_PID file descriptors from the environment (recommended, but
-  problematic in threaded environments). If r is the return value of
-  this function you'll find the file descriptors passed as fds
-  SD_LISTEN_FDS_START to SD_LISTEN_FDS_START+r-1. Returns a negative
-  errno style error code on failure. This function call ensures that
-  the FD_CLOEXEC flag is set for the passed file descriptors, to make
-  sure they are not passed on to child processes. If FD_CLOEXEC shall
-  not be set, the caller needs to unset it after this call for all file
-  descriptors that are used.
+  errno code on failure. Optionally, removes the $LISTEN_FDS,
+  $LISTEN_PID, $LISTEN_PIDFDID, and $LISTEN_FDNAMES variables from the
+  environment (recommended, but problematic in threaded environments).
+  If r is the return value of this function you'll find the file
+  descriptors passed as fds SD_LISTEN_FDS_START to
+  SD_LISTEN_FDS_START+r-1. Returns a negative errno style error code on
+  failure. This function call ensures that the FD_CLOEXEC flag is set
+  for the passed file descriptors, to make sure they are not passed on
+  to child processes. If FD_CLOEXEC shall not be set, the caller needs
+  to unset it after this call for all file descriptors that are used.
 
   See sd_listen_fds(3) for more information.
 */

diff --git a/src/userdb/userdbd-manager.c b/src/userdb/userdbd-manager.c
index 1c62bb1697b0c1c7b08c64fe8a7fe4e9c9dce0d9..91fa09d316620e66a5cac549e9091529e3f3faed 100644 (file)
--- a/src/userdb/userdbd-manager.c
+++ b/src/userdb/userdbd-manager.c
@@ -15,6 +15,7 @@
 #include "fs-util.h"
 #include "log.h"
 #include "mkdir.h"
+#include "pidfd-util.h"
 #include "process-util.h"
 #include "set.h"
 #include "signal-util.h"
@@ -181,6 +182,15 @@ static int start_one_worker(Manager *m) {
                         _exit(EXIT_FAILURE);
                 }
 
+                uint64_t pidfdid;
+                if (pidfd_get_inode_id_self_cached(&pidfdid) >= 0) {
+                        r = setenvf("LISTEN_PIDFDID", true, "%" PRIu64, pidfdid);
+                        if (r < 0) {
+                                log_error_errno(r, "Failed to set $LISTEN_PIDFDID: %m");
+                                _exit(EXIT_FAILURE);
+                        }
+                }
+
                 if (setenv("LISTEN_FDS", "1", 1) < 0) {
                         log_error_errno(errno, "Failed to set $LISTEN_FDS: %m");
                         _exit(EXIT_FAILURE);

diff --git a/src/varlinkctl/varlinkctl.c b/src/varlinkctl/varlinkctl.c
index 7768ca52058742d65517502312eb37588039d635..865235bb30c0b727cc44c0cd7339a4fe2daa6d25 100644 (file)
--- a/src/varlinkctl/varlinkctl.c
+++ b/src/varlinkctl/varlinkctl.c
@@ -20,6 +20,7 @@
 #include "pager.h"
 #include "parse-argument.h"
 #include "parse-util.h"
+#include "pidfd-util.h"
 #include "pretty-print.h"
 #include "process-util.h"
 #include "string-util.h"
@@ -876,9 +877,19 @@ static int verb_call(int argc, char *argv[], void *userdata) {
                                         log_error_errno(r, "Failed to set $LISTEN_PID environment variable: %m");
                                         _exit(EXIT_FAILURE);
                                 }
+
+                                uint64_t pidfdid;
+                                if (pidfd_get_inode_id_self_cached(&pidfdid) >= 0) {
+                                        r = setenvf("LISTEN_PIDFDID", /* overwrite= */ true, "%" PRIu64, pidfdid);
+                                        if (r < 0) {
+                                                log_error_errno(r, "Failed to set $LISTEN_PIDFDID environment variable: %m");
+                                                _exit(EXIT_FAILURE);
+                                        }
+                                }
                         } else {
                                 (void) unsetenv("LISTEN_FDS");
                                 (void) unsetenv("LISTEN_PID");
+                                (void) unsetenv("LISTEN_PIDFDID");
                         }
                         (void) unsetenv("LISTEN_FDNAMES");