SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

exec-invoke: Use FORK_DETACH when forking off pid namespace child (#36446)

Browse Source 
This ensures the child process is immediately re-parented to the
manager process which avoids a "Supervising process xxx which is
not our child. We'll most likely not notice when it exits." warning
which can currently happen if the parent systemd-executor parent
process sends the pid namespace child process pidref to the manager
process and the manager process dispatches the child process pidref
before the systemd-executor parent process exits, since at that point
the pid namespace child process's parent will still be the
systemd-executor parent process and not the manager process.
This commit is contained in:
Luca Boccassi
2025-02-21 00:27:17 +00:00
committed by GitHub
parent 9b2b7625d2 698ac172aa
commit 48ae15816b
5 changed files with 134 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/basic/process-util.c
+99 -24
View File
@@ -36,6 +36,7 @@
#include "fs-util.h"
#include "hostname-util.h"
#include "io-util.h"
#include "iovec-util.h"
#include "locale-util.h"
#include "log.h"
#include "macro.h"
@@ -53,6 +54,7 @@
#include "raw-clone.h"
#include "rlimit-util.h"
#include "signal-util.h"
#include "socket-util.h"
#include "stat-util.h"
#include "stdio-util.h"
#include "string-table.h"
@@ -1525,25 +1527,27 @@ 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;
_unused_ _cleanup_(restore_sigsetp) sigset_t *saved_ssp = NULL;
bool block_signals = false, block_all = false, intermediary = false;
_cleanup_close_pair_ int pidref_transport_fds[2] = EBADF_PAIR;
int prio, r;
assert(!FLAGS_SET(flags, FORK_DETACH) ||
(!ret_pid && (flags & (FORK_WAIT|FORK_DEATHSIG_SIGTERM|FORK_DEATHSIG_SIGINT|FORK_DEATHSIG_SIGKILL)) == 0));
(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;
@@ -1586,14 +1590,43 @@ int safe_fork_full(
if (!r) {
/* Not a reaper process, hence do a double fork() so we are reparented to one */
if (ret_pid && socketpair(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0, pidref_transport_fds) < 0)
return log_full_errno(prio, errno, "Failed to allocate pidref socket: %m");
pid = fork();
if (pid < 0)
return log_full_errno(prio, errno, "Failed to fork off '%s': %m", strna(name));
if (pid > 0) {
log_debug("Successfully forked off intermediary '%s' as PID " PID_FMT ".", strna(name), pid);
pidref_transport_fds[1] = safe_close(pidref_transport_fds[1]);
if (pidref_transport_fds[0] >= 0) {
/* Wait for the intermediary child to exit so the caller can be certain the actual child
* process has been reparented by the time this function returns. */
r = wait_for_terminate_and_check(name, pid, FLAGS_SET(flags, FORK_LOG) ? WAIT_LOG : 0);
if (r < 0)
return log_full_errno(prio, r, "Failed to wait for intermediary process: %m");
if (r != EXIT_SUCCESS) /* exit status > 0 should be treated as failure, too */
return -EPROTO;
int pidfd;
ssize_t n = receive_one_fd_iov(
pidref_transport_fds[0],
&IOVEC_MAKE(&pid, sizeof(pid)),
/* iovlen= */ 1,
/* flags= */ 0,
&pidfd);
if (n < 0)
return log_full_errno(prio, n, "Failed to receive child pidref: %m");
*ret_pid = (PidRef) { .pid = pid, .fd = pidfd };
}
return 1; /* return in the parent */
}
pidref_transport_fds[0] = safe_close(pidref_transport_fds[0]);
intermediary = true;
}
}
@@ -1611,8 +1644,30 @@ int safe_fork_full(
if (pid > 0) {
/* If we are in the intermediary process, exit now */
if (intermediary)
if (intermediary) {
if (pidref_transport_fds[1] >= 0) {
_cleanup_(pidref_done) PidRef pidref = PIDREF_NULL;
r = pidref_set_pid(&pidref, pid);
if (r < 0) {
log_full_errno(prio, r, "Failed to open reference to PID "PID_FMT": %m", pid);
_exit(EXIT_FAILURE);
}
r = send_one_fd_iov(
pidref_transport_fds[1],
pidref.fd,
&IOVEC_MAKE(&pidref.pid, sizeof(pidref.pid)),
/* iovlen= */ 1,
/* flags= */ 0);
if (r < 0) {
log_full_errno(prio, r, "Failed to send child pidref: %m");
_exit(EXIT_FAILURE);
}
}
_exit(EXIT_SUCCESS);
}
/* We are in the parent process */
log_debug("Successfully forked off '%s' as PID " PID_FMT ".", strna(name), pid);
@@ -1630,16 +1685,31 @@ 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;
}
/* We are in the child process */
pidref_transport_fds[1] = safe_close(pidref_transport_fds[1]);
/* Restore signal mask manually */
saved_ssp = NULL;
@@ -1801,36 +1871,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);
/* 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);
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;
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;
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;
}
src/basic/process-util.h
+13 -12
View File
@@ -193,20 +193,9 @@ 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(
const char *name,
const int stdio_fds[3],
int except_fds[],
size_t n_except_fds,
ForkFlags flags,
pid_t *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 pidref_safe_fork_full(
const char *name,
const int stdio_fds[3],
@@ -219,6 +208,18 @@ static inline int pidref_safe_fork(const char *name, ForkFlags flags, PidRef *re
return pidref_safe_fork_full(name, NULL, NULL, 0, flags, ret_pid);
}
int 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);
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(
const char *outer_name,
const char *inner_name,
src/core/exec-invoke.c
+2 -1
View File
@@ -2480,7 +2480,8 @@ static int setup_private_pids(const ExecContext *c, ExecParameters *p) {
if (pipe2(errno_pipe, O_CLOEXEC) < 0)
return log_exec_debug_errno(c, p, errno, "Failed to create pipe for communicating with parent process: %m");
r = pidref_safe_fork("(sd-pidns-child)", FORK_NEW_PIDNS, &pidref);
/* Set FORK_DETACH to immediately re-parent the child process to the invoking manager process. */
r = pidref_safe_fork("(sd-pidns-child)", FORK_NEW_PIDNS|FORK_DETACH, &pidref);
if (r < 0)
return log_exec_debug_errno(c, p, r, "Failed to fork child into new pid namespace: %m");
if (r > 0) {
src/test/test-async.c
+2 -2
View File
@@ -81,7 +81,7 @@ TEST(asynchronous_rm_rf) {
/* Do this once more, from a subreaper. Which is nice, because we can watch the async child even
* though detached */
r = safe_fork("(subreaper)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_DEATHSIG_SIGTERM|FORK_LOG|FORK_WAIT, NULL);
r = safe_fork("(subreaper)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_DEATHSIG_SIGTERM|FORK_REOPEN_LOG|FORK_LOG|FORK_WAIT, NULL);
ASSERT_OK(r);
if (r == 0) {
@@ -100,7 +100,7 @@ TEST(asynchronous_rm_rf) {
for (;;) {
siginfo_t si = {};
ASSERT_OK(waitid(P_ALL, 0, &si, WEXITED));
ASSERT_OK_ERRNO(waitid(P_ALL, 0, &si, WEXITED));
if (access(tt, F_OK) < 0) {
assert_se(errno == ENOENT);
src/test/test-process-util.c
+18
View File
@@ -659,6 +659,24 @@ TEST(safe_fork) {
ASSERT_OK(wait_for_terminate(pid, &status));
ASSERT_EQ(status.si_code, CLD_EXITED);
ASSERT_EQ(status.si_status, 88);
_cleanup_(pidref_done) PidRef child = PIDREF_NULL;
r = pidref_safe_fork("(test-child)", FORK_DETACH, &child);
if (r == 0) {
/* Don't freeze so this doesn't linger around forever in case something goes wrong. */
usleep_safe(100 * USEC_PER_SEC);
_exit(EXIT_SUCCESS);
}
ASSERT_OK_POSITIVE(r);
ASSERT_GT(child.pid, 0);
ASSERT_OK(pidref_get_ppid(&child, &pid));
ASSERT_OK(pidref_kill(&child, SIGKILL));
if (is_reaper_process())
ASSERT_EQ(pid, getpid_cached());
else
ASSERT_NE(pid, getpid_cached());
}
TEST(pid_to_ptr) {