]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
process-util: Implement safe_fork_full() on top of pidref_safe_fork_full()
authorDaan De Meyer <daan.j.demeyer@gmail.com>
Thu, 20 Feb 2025 08:24:58 +0000 (09:24 +0100)
committerDaan De Meyer <daan.j.demeyer@gmail.com>
Thu, 20 Feb 2025 19:13:53 +0000 (20:13 +0100)
Let's switch things around, and move the internals of safe_fork_full() into
pidref_safe_fork_full() and make safe_fork_full() a trivial wrapper on top
of pidref_safe_fork_full().

src/basic/process-util.c
src/basic/process-util.h

index fbde9c35e67f585c0a0adcf96947c73061086450..6a55cca62540ba2c29eb776944e348be1a1f46ba 100644 (file)
@@ -1525,13 +1525,13 @@ static int fork_flags_to_signal(ForkFlags flags) {
                                                  SIGKILL;
 }
 
-int safe_fork_full(
+int pidref_safe_fork_full(
                 const char *name,
                 const int stdio_fds[3],
                 int except_fds[],
                 size_t n_except_fds,
                 ForkFlags flags,
-                pid_t *ret_pid) {
+                PidRef *ret_pid) {
 
         pid_t original_pid, pid;
         sigset_t saved_ss, ss;
@@ -1542,8 +1542,9 @@ int safe_fork_full(
         assert(!FLAGS_SET(flags, FORK_DETACH) ||
                (!ret_pid && (flags & (FORK_WAIT|FORK_DEATHSIG_SIGTERM|FORK_DEATHSIG_SIGINT|FORK_DEATHSIG_SIGKILL)) == 0));
 
-        /* A wrapper around fork(), that does a couple of important initializations in addition to mere forking. Always
-         * returns the child's PID in *ret_pid. Returns == 0 in the child, and > 0 in the parent. */
+        /* A wrapper around fork(), that does a couple of important initializations in addition to mere
+         * forking. If provided, ret_pid is initialized in both the parent and the child process, both times
+         * referencing the child process. Returns == 0 in the child and > 0 in the parent. */
 
         prio = flags & FORK_LOG ? LOG_ERR : LOG_DEBUG;
 
@@ -1630,10 +1631,23 @@ int safe_fork_full(
                                 return r;
                         if (r != EXIT_SUCCESS) /* exit status > 0 should be treated as failure, too */
                                 return -EPROTO;
+
+                        /* If we are in the parent and successfully waited, then the process doesn't exist anymore. */
+                        if (ret_pid)
+                                *ret_pid = PIDREF_NULL;
+
+                        return 1;
                 }
 
-                if (ret_pid)
-                        *ret_pid = pid;
+                if (ret_pid) {
+                        if (FLAGS_SET(flags, FORK_PID_ONLY))
+                                *ret_pid = PIDREF_MAKE_FROM_PID(pid);
+                        else {
+                                r = pidref_set_pid(ret_pid, pid);
+                                if (r < 0) /* Let's not fail for this, no matter what, the process exists after all, and that's key */
+                                        *ret_pid = PIDREF_MAKE_FROM_PID(pid);
+                        }
+                }
 
                 return 1;
         }
@@ -1801,36 +1815,41 @@ int safe_fork_full(
         if (FLAGS_SET(flags, FORK_FREEZE))
                 freeze();
 
-        if (ret_pid)
-                *ret_pid = getpid_cached();
+        if (ret_pid) {
+                if (FLAGS_SET(flags, FORK_PID_ONLY))
+                        *ret_pid = PIDREF_MAKE_FROM_PID(getpid_cached());
+                else {
+                        r = pidref_set_self(ret_pid);
+                        if (r < 0) {
+                                log_full_errno(prio, r, "Failed to acquire PID reference on ourselves: %m");
+                                _exit(EXIT_FAILURE);
+                        }
+                }
+        }
 
         return 0;
 }
 
-int pidref_safe_fork_full(
+int safe_fork_full(
                 const char *name,
                 const int stdio_fds[3],
                 int except_fds[],
                 size_t n_except_fds,
                 ForkFlags flags,
-                PidRef *ret_pid) {
+                pid_t *ret_pid) {
 
-        pid_t pid;
-        int r, q;
+        _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL;
+        int r;
 
-        r = safe_fork_full(name, stdio_fds, except_fds, n_except_fds, flags, &pid);
-        if (r < 0 || !ret_pid)
-                return r;
+        /* Getting the detached child process pid without pidfd is racy, so don't allow it if not returning
+         * a pidref to the caller. */
+        assert(!FLAGS_SET(flags, FORK_DETACH) || !ret_pid);
 
-        if (r > 0 && FLAGS_SET(flags, FORK_WAIT)) {
-                /* If we are in the parent and successfully waited, then the process doesn't exist anymore */
-                *ret_pid = PIDREF_NULL;
+        r = pidref_safe_fork_full(name, stdio_fds, except_fds, n_except_fds, flags|FORK_PID_ONLY, ret_pid ? &pidref : NULL);
+        if (r < 0 || !ret_pid)
                 return r;
-        }
 
-        q = pidref_set_pid(ret_pid, pid);
-        if (q < 0) /* Let's not fail for this, no matter what, the process exists after all, and that's key */
-                *ret_pid = PIDREF_MAKE_FROM_PID(pid);
+        *ret_pid = pidref.pid;
 
         return r;
 }
index 58fff2b1740958810cbedd74a7affe4aea7224c0..c9b35d79f02756289e3c027abea2c5743b098953 100644 (file)
@@ -193,30 +193,31 @@ typedef enum ForkFlags {
         FORK_NEW_NETNS          = 1 << 20, /* Run child in its own network namespace                             ðŸ’£ DO NOT USE IN THREADED PROGRAMS! ðŸ’£ */
         FORK_NEW_PIDNS          = 1 << 21, /* Run child in its own PID namespace                                 ðŸ’£ DO NOT USE IN THREADED PROGRAMS! ðŸ’£ */
         FORK_FREEZE             = 1 << 22, /* Don't return in child, just call freeze() instead */
+        FORK_PID_ONLY           = 1 << 23, /* Don't open a pidfd referencing the child process */
 } ForkFlags;
 
-int safe_fork_full(
+int pidref_safe_fork_full(
                 const char *name,
                 const int stdio_fds[3],
                 int except_fds[],
                 size_t n_except_fds,
                 ForkFlags flags,
-                pid_t *ret_pid);
+                PidRef *ret_pid);
 
-static inline int safe_fork(const char *name, ForkFlags flags, pid_t *ret_pid) {
-        return safe_fork_full(name, NULL, NULL, 0, flags, ret_pid);
+static inline int pidref_safe_fork(const char *name, ForkFlags flags, PidRef *ret_pid) {
+        return pidref_safe_fork_full(name, NULL, NULL, 0, flags, ret_pid);
 }
 
-int pidref_safe_fork_full(
+int safe_fork_full(
                 const char *name,
                 const int stdio_fds[3],
                 int except_fds[],
                 size_t n_except_fds,
                 ForkFlags flags,
-                PidRef *ret_pid);
+                pid_t *ret_pid);
 
-static inline int pidref_safe_fork(const char *name, ForkFlags flags, PidRef *ret_pid) {
-        return pidref_safe_fork_full(name, NULL, NULL, 0, flags, ret_pid);
+static inline int safe_fork(const char *name, ForkFlags flags, pid_t *ret_pid) {
+        return safe_fork_full(name, NULL, NULL, 0, flags, ret_pid);
 }
 
 int namespace_fork(