From e48e098db1b4b4e4711a47c55b1c49f6c119936f Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Fri, 19 Dec 2025 23:26:49 +0100 Subject: [PATCH 1/5] process-util: dedup restore_sigsetp() with block_signals_reset() --- src/basic/process-util.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/basic/process-util.c b/src/basic/process-util.c index 31dbdff87a..50fa34c2e5 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -1484,11 +1484,6 @@ pid_t clone_with_nested_stack(int (*fn)(void *), int flags, void *userdata) { return pid; } -static void restore_sigsetp(sigset_t **ssp) { - if (*ssp) - (void) sigprocmask(SIG_SETMASK, *ssp, NULL); -} - static int fork_flags_to_signal(ForkFlags flags) { return (flags & FORK_DEATHSIG_SIGTERM) ? SIGTERM : (flags & FORK_DEATHSIG_SIGINT) ? SIGINT : @@ -1505,7 +1500,7 @@ int pidref_safe_fork_full( pid_t original_pid, pid; sigset_t saved_ss, ss; - _unused_ _cleanup_(restore_sigsetp) sigset_t *saved_ssp = NULL; + _unused_ _cleanup_(block_signals_reset) 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; From 3532691d72feb0b8861bf0320ec5aea442d67270 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Fri, 28 Nov 2025 19:50:35 +0100 Subject: [PATCH 2/5] process-util: trivial cleanup for read_errno() --- src/basic/process-util.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/basic/process-util.c b/src/basic/process-util.c index 50fa34c2e5..c52084f625 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -2341,16 +2341,15 @@ int read_errno(int errno_fd) { log_debug_errno(n, "Failed to read errno: %m"); return -EIO; } - if (n == sizeof(r)) { - if (r == 0) - return 0; - if (r < 0) /* child process reported an error, return it */ - return log_debug_errno(r, "Child process failed with errno: %m"); - return log_debug_errno(SYNTHETIC_ERRNO(EIO), "Received an errno, but it's a positive value."); - } - if (n != 0) - return log_debug_errno(SYNTHETIC_ERRNO(EIO), "Received unexpected amount of bytes while reading errno."); + if (n == 0) /* the process exited without reporting an error, assuming success */ + return 0; + if (n != sizeof(r)) + return log_debug_errno(SYNTHETIC_ERRNO(EIO), "Received unexpected amount of bytes (%zi) while reading errno.", n); - /* the process exited without reporting an error, assuming success */ - return 0; + if (r == 0) + return 0; + if (r < 0) /* child process reported an error, return it */ + return log_debug_errno(r, "Child process failed with errno: %m"); + + return log_debug_errno(SYNTHETIC_ERRNO(EIO), "Received positive errno from child, refusing: %d", r); } From 8c007f68ffc372e494fc27ca950d2e639743030f Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Wed, 19 Nov 2025 16:24:15 +0100 Subject: [PATCH 3/5] process-util: Migrate namespace_fork() to PidRef Co-authored-by: Mike Yuan --- src/analyze/analyze-unit-shell.c | 9 +++---- src/basic/process-util.c | 35 ++++++++++++++++----------- src/basic/process-util.h | 20 +++++++++++++-- src/basic/terminal-util.c | 4 +-- src/core/namespace.c | 11 +++++---- src/coredump/coredump-send.c | 21 ++++++++-------- src/coredump/coredump-submit.c | 4 +-- src/libsystemd/sd-bus/bus-container.c | 10 ++++---- src/libsystemd/sd-id128/id128-util.c | 8 +++--- src/machine/machine.c | 15 +++++------- src/machine/machined-core.c | 14 +++-------- src/shared/mount-util.c | 13 +++------- 12 files changed, 86 insertions(+), 78 deletions(-) diff --git a/src/analyze/analyze-unit-shell.c b/src/analyze/analyze-unit-shell.c index 6e5a1f84d5..8990ffdf1d 100644 --- a/src/analyze/analyze-unit-shell.c +++ b/src/analyze/analyze-unit-shell.c @@ -13,6 +13,7 @@ #include "fd-util.h" #include "log.h" #include "namespace-util.h" +#include "pidref.h" #include "process-util.h" #include "runtime-scope.h" #include "strv.h" @@ -78,12 +79,10 @@ int verb_unit_shell(int argc, char *argv[], void *userdata) { return log_oom(); } - pid_t child; + _cleanup_(pidref_done) PidRef child = PIDREF_NULL; r = namespace_fork( "(unit-shell-ns)", "(unit-shell)", - /* except_fds= */ NULL, - /* n_except_fds= */ 0, FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGKILL, pidns_fd, mntns_fd, @@ -117,8 +116,8 @@ int verb_unit_shell(int argc, char *argv[], void *userdata) { _exit(EXIT_FAILURE); } - return wait_for_terminate_and_check( + return pidref_wait_for_terminate_and_check( "(unit-shell)", - child, + &child, WAIT_LOG_ABNORMAL|WAIT_LOG_NON_ZERO_EXIT_STATUS); } diff --git a/src/basic/process-util.c b/src/basic/process-util.c index c52084f625..700fba5cba 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -1884,7 +1884,7 @@ int safe_fork_full( return r; } -int namespace_fork( +int namespace_fork_full( const char *outer_name, const char *inner_name, int except_fds[], @@ -1895,7 +1895,7 @@ int namespace_fork( int netns_fd, int userns_fd, int root_fd, - pid_t *ret_pid) { + PidRef *ret) { int r; @@ -1904,14 +1904,16 @@ int namespace_fork( * /proc/self/fd works correctly. */ assert(!FLAGS_SET(flags, FORK_ALLOW_DLOPEN)); /* never allow loading shared library from another ns */ - r = safe_fork_full(outer_name, - NULL, - except_fds, n_except_fds, - (flags|FORK_DEATHSIG_SIGINT|FORK_DEATHSIG_SIGTERM|FORK_DEATHSIG_SIGKILL) & ~(FORK_REOPEN_LOG|FORK_NEW_MOUNTNS|FORK_MOUNTNS_SLAVE), ret_pid); + r = pidref_safe_fork_full( + outer_name, + NULL, + except_fds, n_except_fds, + (flags|FORK_DEATHSIG_SIGINT|FORK_DEATHSIG_SIGTERM|FORK_DEATHSIG_SIGKILL) & ~(FORK_REOPEN_LOG|FORK_NEW_MOUNTNS|FORK_MOUNTNS_SLAVE), + ret); if (r < 0) return r; if (r == 0) { - pid_t pid; + _cleanup_(pidref_done) PidRef pidref_inner = PIDREF_NULL; /* Child */ @@ -1922,20 +1924,25 @@ int namespace_fork( } /* We mask a few flags here that either make no sense for the grandchild, or that we don't have to do again */ - r = safe_fork_full(inner_name, - NULL, - except_fds, n_except_fds, - flags & ~(FORK_WAIT|FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_REARRANGE_STDIO), &pid); + r = pidref_safe_fork_full( + inner_name, + NULL, + except_fds, n_except_fds, + flags & ~(FORK_WAIT|FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_REARRANGE_STDIO), + &pidref_inner); if (r < 0) _exit(EXIT_FAILURE); if (r == 0) { /* Child */ - if (ret_pid) - *ret_pid = pid; + if (ret) + *ret = TAKE_PIDREF(pidref_inner); return 0; } - r = wait_for_terminate_and_check(inner_name, pid, FLAGS_SET(flags, FORK_LOG) ? WAIT_LOG : 0); + r = pidref_wait_for_terminate_and_check( + inner_name, + &pidref_inner, + FLAGS_SET(flags, FORK_LOG) ? WAIT_LOG : 0); if (r < 0) _exit(EXIT_FAILURE); diff --git a/src/basic/process-util.h b/src/basic/process-util.h index 0290978b99..bb65f0eedc 100644 --- a/src/basic/process-util.h +++ b/src/basic/process-util.h @@ -216,7 +216,7 @@ 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( +int namespace_fork_full( const char *outer_name, const char *inner_name, int except_fds[], @@ -227,7 +227,23 @@ int namespace_fork( int netns_fd, int userns_fd, int root_fd, - pid_t *ret_pid); + PidRef *ret); + +static inline int namespace_fork( + const char *outer_name, + const char *inner_name, + ForkFlags flags, + int pidns_fd, + int mntns_fd, + int netns_fd, + int userns_fd, + int root_fd, + PidRef *ret) { + + return namespace_fork_full(outer_name, inner_name, NULL, 0, flags, + pidns_fd, mntns_fd, netns_fd, userns_fd, root_fd, + ret); +} int set_oom_score_adjust(int value); int get_oom_score_adjust(int *ret); diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index c46286ae8e..9abef38aad 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -1665,15 +1665,13 @@ int openpt_allocate_in_namespace( r = namespace_fork( "(sd-openptns)", "(sd-openpt)", - /* except_fds= */ NULL, - /* n_except_fds= */ 0, FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGKILL|FORK_WAIT, pidnsfd, mntnsfd, /* netns_fd= */ -EBADF, usernsfd, rootfd, - /* ret_pid= */ NULL); + /* ret= */ NULL); if (r < 0) return r; if (r == 0) { diff --git a/src/core/namespace.c b/src/core/namespace.c index 7a8facce97..2c87975baf 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -3911,7 +3911,7 @@ int refresh_extensions_in_namespace( if (r == 0) { /* Child (host namespace) */ _cleanup_close_pair_ int pair[2] = EBADF_PAIR; - _cleanup_(sigkill_waitp) pid_t grandchild_pid = 0; + _cleanup_(pidref_done_sigkill_wait) PidRef grandchild = PIDREF_NULL; (void) mkdir_p_label(overlay_prefix, 0555); @@ -3928,9 +3928,7 @@ int refresh_extensions_in_namespace( _exit(EXIT_FAILURE); } - r = safe_fork("(sd-ns-refresh-exts-grandchild)", - FORK_LOG|FORK_DEATHSIG_SIGKILL, - &grandchild_pid); + r = pidref_safe_fork("(sd-ns-refresh-exts-grandchild)", FORK_LOG|FORK_DEATHSIG_SIGKILL, &grandchild); if (r < 0) _exit(EXIT_FAILURE); if (r == 0) { @@ -3964,11 +3962,14 @@ int refresh_extensions_in_namespace( _exit(EXIT_FAILURE); } - r = wait_for_terminate_and_check("(sd-ns-refresh-exts-grandchild)", TAKE_PID(grandchild_pid), 0); + r = pidref_wait_for_terminate_and_check("(sd-ns-refresh-exts-grandchild)", &grandchild, 0); if (r < 0) { log_debug_errno(r, "Failed to wait for target namespace process to finish: %m"); _exit(EXIT_FAILURE); } + + pidref_done(&grandchild); + if (r != EXIT_SUCCESS) { log_debug("Target namespace fork did not succeed"); _exit(EXIT_FAILURE); diff --git a/src/coredump/coredump-send.c b/src/coredump/coredump-send.c index eadec5f0f5..12c781d05a 100644 --- a/src/coredump/coredump-send.c +++ b/src/coredump/coredump-send.c @@ -224,14 +224,6 @@ static int receive_ucred(int transport_fd, struct ucred *ret_ucred) { } int coredump_send_to_container(CoredumpContext *context) { - _cleanup_close_ int pidnsfd = -EBADF, mntnsfd = -EBADF, netnsfd = -EBADF, usernsfd = -EBADF, rootfd = -EBADF; - _cleanup_close_pair_ int pair[2] = EBADF_PAIR; - pid_t child; - struct ucred ucred = { - .pid = context->pidref.pid, - .uid = context->uid, - .gid = context->gid, - }; int r; assert(context); @@ -255,6 +247,15 @@ int coredump_send_to_container(CoredumpContext *context) { if (r <= 0) return r; + _cleanup_close_ int pidnsfd = -EBADF, mntnsfd = -EBADF, netnsfd = -EBADF, usernsfd = -EBADF, rootfd = -EBADF; + _cleanup_(pidref_done) PidRef child = PIDREF_NULL; + _cleanup_close_pair_ int pair[2] = EBADF_PAIR; + struct ucred ucred = { + .pid = context->pidref.pid, + .uid = context->uid, + .gid = context->gid, + }; + r = RET_NERRNO(socketpair(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0, pair)); if (r < 0) return log_debug_errno(r, "Failed to create socket pair: %m"); @@ -267,7 +268,7 @@ int coredump_send_to_container(CoredumpContext *context) { if (r < 0) return log_debug_errno(r, "Failed to open namespaces of PID " PID_FMT ": %m", leader_pid.pid); - r = namespace_fork("(sd-coredumpns)", "(sd-coredump)", NULL, 0, + r = namespace_fork("(sd-coredumpns)", "(sd-coredump)", FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGTERM, pidnsfd, mntnsfd, netnsfd, usernsfd, rootfd, &child); if (r < 0) @@ -325,7 +326,7 @@ int coredump_send_to_container(CoredumpContext *context) { if (r < 0) return log_debug_errno(r, "Failed to send metadata to container: %m"); - r = wait_for_terminate_and_check("(sd-coredumpns)", child, 0); + r = pidref_wait_for_terminate_and_check("(sd-coredumpns)", &child, 0); if (r < 0) return log_debug_errno(r, "Failed to wait for child to terminate: %m"); if (r != EXIT_SUCCESS) diff --git a/src/coredump/coredump-submit.c b/src/coredump/coredump-submit.c index 0553881ada..9134697d5b 100644 --- a/src/coredump/coredump-submit.c +++ b/src/coredump/coredump-submit.c @@ -494,15 +494,13 @@ static int acquire_pid_mount_tree_fd(const CoredumpConfig *config, CoredumpConte r = namespace_fork("(sd-mount-tree-ns)", "(sd-mount-tree)", - /* except_fds= */ NULL, - /* n_except_fds= */ 0, FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGKILL|FORK_LOG|FORK_WAIT, /* pidns_fd= */ -EBADF, mntns_fd, /* netns_fd= */ -EBADF, /* userns_fd= */ -EBADF, root_fd, - NULL); + /* ret= */ NULL); if (r < 0) return r; if (r == 0) { diff --git a/src/libsystemd/sd-bus/bus-container.c b/src/libsystemd/sd-bus/bus-container.c index 8d760660dc..3bbcbb81c6 100644 --- a/src/libsystemd/sd-bus/bus-container.c +++ b/src/libsystemd/sd-bus/bus-container.c @@ -1,6 +1,5 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ -#include #include #include "bus-container.h" @@ -10,14 +9,15 @@ #include "format-util.h" #include "log.h" #include "namespace-util.h" +#include "pidref.h" #include "process-util.h" #include "string-util.h" int bus_container_connect_socket(sd_bus *b) { - _cleanup_close_pair_ int pair[2] = EBADF_PAIR; _cleanup_close_ int pidnsfd = -EBADF, mntnsfd = -EBADF, usernsfd = -EBADF, rootfd = -EBADF; + _cleanup_(pidref_done) PidRef child = PIDREF_NULL; + _cleanup_close_pair_ int pair[2] = EBADF_PAIR; int r, error_buf = 0; - pid_t child; ssize_t n; assert(b); @@ -53,7 +53,7 @@ int bus_container_connect_socket(sd_bus *b) { if (socketpair(AF_UNIX, SOCK_SEQPACKET|SOCK_CLOEXEC, 0, pair) < 0) return log_debug_errno(errno, "Failed to create a socket pair: %m"); - r = namespace_fork("(sd-buscntrns)", "(sd-buscntr)", NULL, 0, FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGKILL, + r = namespace_fork("(sd-buscntrns)", "(sd-buscntr)", FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGKILL, pidnsfd, mntnsfd, -1, usernsfd, rootfd, &child); if (r < 0) return log_debug_errno(r, "Failed to create namespace for (sd-buscntr): %m"); @@ -73,7 +73,7 @@ int bus_container_connect_socket(sd_bus *b) { pair[1] = safe_close(pair[1]); - r = wait_for_terminate_and_check("(sd-buscntrns)", child, 0); + r = pidref_wait_for_terminate_and_check("(sd-buscntrns)", &child, 0); if (r < 0) return r; bool nonzero_exit_status = r != EXIT_SUCCESS; diff --git a/src/libsystemd/sd-id128/id128-util.c b/src/libsystemd/sd-id128/id128-util.c index 0aaa98292d..86a2938af4 100644 --- a/src/libsystemd/sd-id128/id128-util.c +++ b/src/libsystemd/sd-id128/id128-util.c @@ -11,6 +11,7 @@ #include "id128-util.h" #include "io-util.h" #include "namespace-util.h" +#include "pidref.h" #include "process-util.h" #include "sha256.h" #include "siphash24.h" @@ -274,8 +275,9 @@ sd_id128_t id128_digest(const void *data, size_t size) { int id128_get_boot_for_machine(const char *machine, sd_id128_t *ret) { _cleanup_close_ int pidnsfd = -EBADF, mntnsfd = -EBADF, rootfd = -EBADF; + _cleanup_(pidref_done) PidRef child = PIDREF_NULL; _cleanup_close_pair_ int pair[2] = EBADF_PAIR; - pid_t pid, child; + pid_t pid; sd_id128_t id; ssize_t k; int r; @@ -296,7 +298,7 @@ int id128_get_boot_for_machine(const char *machine, sd_id128_t *ret) { if (socketpair(AF_UNIX, SOCK_DGRAM, 0, pair) < 0) return -errno; - r = namespace_fork("(sd-bootidns)", "(sd-bootid)", NULL, 0, FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGKILL, + r = namespace_fork("(sd-bootidns)", "(sd-bootid)", FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGKILL, pidnsfd, mntnsfd, -1, -1, rootfd, &child); if (r < 0) return r; @@ -316,7 +318,7 @@ int id128_get_boot_for_machine(const char *machine, sd_id128_t *ret) { pair[1] = safe_close(pair[1]); - r = wait_for_terminate_and_check("(sd-bootidns)", child, 0); + r = pidref_wait_for_terminate_and_check("(sd-bootidns)", &child, 0); if (r < 0) return r; if (r != EXIT_SUCCESS) diff --git a/src/machine/machine.c b/src/machine/machine.c index 281f377b3e..1176649388 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -1141,9 +1141,9 @@ int machine_copy_from_to_operation( Operation **ret) { _cleanup_close_ int host_fd = -EBADF, target_mntns_fd = -EBADF, source_mntns_fd = -EBADF; + _cleanup_(pidref_done_sigkill_wait) PidRef child = PIDREF_NULL; _cleanup_close_pair_ int errno_pipe_fd[2] = EBADF_PAIR; _cleanup_free_ char *host_basename = NULL, *container_basename = NULL; - _cleanup_(sigkill_waitp) pid_t child = 0; uid_t uid_shift; int r; @@ -1183,8 +1183,6 @@ int machine_copy_from_to_operation( r = namespace_fork("(sd-copyns)", "(sd-copy)", - /* except_fds= */ NULL, - /* n_except_fds= */ 0, FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGKILL, /* pidns_fd= */ -EBADF, target_mntns_fd, @@ -1244,13 +1242,14 @@ int machine_copy_from_to_operation( errno_pipe_fd[1] = safe_close(errno_pipe_fd[1]); + // TODO: port to PidRef and donate child rather than destroying it Operation *operation; - r = operation_new(manager, machine, child, errno_pipe_fd[0], &operation); + r = operation_new(manager, machine, child.pid, errno_pipe_fd[0], &operation); if (r < 0) return r; TAKE_FD(errno_pipe_fd[0]); - TAKE_PID(child); + pidref_done(&child); *ret = operation; return 0; @@ -1532,8 +1531,8 @@ int machine_open_root_directory(Machine *machine) { case MACHINE_CONTAINER: { _cleanup_close_ int mntns_fd = -EBADF, root_fd = -EBADF; + _cleanup_(pidref_done) PidRef child = PIDREF_NULL; _cleanup_close_pair_ int errno_pipe_fd[2] = EBADF_PAIR, fd_pass_socket[2] = EBADF_PAIR; - pid_t child; r = pidref_namespace_open(&machine->leader, /* ret_pidns_fd= */ NULL, @@ -1553,8 +1552,6 @@ int machine_open_root_directory(Machine *machine) { r = namespace_fork( "(sd-openrootns)", "(sd-openroot)", - /* except_fds= */ NULL, - /* n_except_fds= */ 0, FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGKILL, /* pidns_fd= */ -EBADF, mntns_fd, @@ -1589,7 +1586,7 @@ int machine_open_root_directory(Machine *machine) { errno_pipe_fd[1] = safe_close(errno_pipe_fd[1]); fd_pass_socket[1] = safe_close(fd_pass_socket[1]); - r = wait_for_terminate_and_check("(sd-openrootns)", child, /* flags= */ 0); + r = pidref_wait_for_terminate_and_check("(sd-openrootns)", &child, /* flags= */ 0); if (r < 0) return log_debug_errno(r, "Failed to wait for child: %m"); diff --git a/src/machine/machined-core.c b/src/machine/machined-core.c index a262efb798..c98c9e1eeb 100644 --- a/src/machine/machined-core.c +++ b/src/machine/machined-core.c @@ -226,9 +226,9 @@ int machine_get_addresses(Machine *machine, struct local_address **ret_addresses } case MACHINE_CONTAINER: { + _cleanup_(pidref_done) PidRef child = PIDREF_NULL; _cleanup_close_pair_ int pair[2] = EBADF_PAIR; _cleanup_close_ int netns_fd = -EBADF; - pid_t child; int r; r = pidref_in_same_namespace(/* pid1= */ NULL, &machine->leader, NAMESPACE_NET); @@ -251,8 +251,6 @@ int machine_get_addresses(Machine *machine, struct local_address **ret_addresses r = namespace_fork("(sd-addrns)", "(sd-addr)", - /* except_fds= */ NULL, - /* n_except_fds= */ 0, FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGKILL, /* pidns_fd= */ -EBADF, /* mntns_fd= */ -EBADF, @@ -281,8 +279,6 @@ int machine_get_addresses(Machine *machine, struct local_address **ret_addresses } } - pair[1] = safe_close(pair[1]); - _exit(EXIT_SUCCESS); } @@ -313,7 +309,7 @@ int machine_get_addresses(Machine *machine, struct local_address **ret_addresses return log_debug_errno(r, "Failed to add local address: %m"); } - r = wait_for_terminate_and_check("(sd-addrns)", child, /* flags= */ 0); + r = pidref_wait_for_terminate_and_check("(sd-addrns)", &child, /* flags= */ 0); if (r < 0) return log_debug_errno(r, "Failed to wait for child: %m"); if (r != EXIT_SUCCESS) @@ -348,9 +344,9 @@ int machine_get_os_release(Machine *machine, char ***ret_os_release) { case MACHINE_CONTAINER: { _cleanup_close_ int mntns_fd = -EBADF, root_fd = -EBADF, pidns_fd = -EBADF; + _cleanup_(pidref_done) PidRef child = PIDREF_NULL; _cleanup_close_pair_ int pair[2] = EBADF_PAIR; _cleanup_fclose_ FILE *f = NULL; - pid_t child; r = pidref_namespace_open(&machine->leader, &pidns_fd, @@ -366,8 +362,6 @@ int machine_get_os_release(Machine *machine, char ***ret_os_release) { r = namespace_fork("(sd-osrelns)", "(sd-osrel)", - /* except_fds= */ NULL, - /* n_except_fds= */ 0, FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGKILL, pidns_fd, mntns_fd, @@ -409,7 +403,7 @@ int machine_get_os_release(Machine *machine, char ***ret_os_release) { if (r < 0) return log_debug_errno(r, "Failed to load OS release information: %m"); - r = wait_for_terminate_and_check("(sd-osrelns)", child, /* flags= */ 0); + r = pidref_wait_for_terminate_and_check("(sd-osrelns)", &child, /* flags= */ 0); if (r < 0) return log_debug_errno(r, "Failed to wait for child: %m"); if (r == EXIT_NOT_FOUND) diff --git a/src/shared/mount-util.c b/src/shared/mount-util.c index ca383f68ba..12d9cdaeb8 100644 --- a/src/shared/mount-util.c +++ b/src/shared/mount-util.c @@ -1,6 +1,5 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ -#include #include #include #include @@ -968,7 +967,7 @@ static int mount_in_namespace_legacy( bool mount_slave_created = false, mount_slave_mounted = false, mount_tmp_created = false, mount_tmp_mounted = false, mount_outside_created = false, mount_outside_mounted = false; - pid_t child; + _cleanup_(pidref_done) PidRef child = PIDREF_NULL; int r; assert(chased_src_path); @@ -1092,8 +1091,6 @@ static int mount_in_namespace_legacy( r = namespace_fork( "(sd-bindmnt)", "(sd-bindmnt-inner)", - /* except_fds= */ NULL, - /* n_except_fds= */ 0, FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGTERM, pidns_fd, mntns_fd, @@ -1139,7 +1136,7 @@ static int mount_in_namespace_legacy( errno_pipe_fd[1] = safe_close(errno_pipe_fd[1]); - r = wait_for_terminate_and_check("(sd-bindmnt)", child, 0); + r = pidref_wait_for_terminate_and_check("(sd-bindmnt)", &child, 0); if (r < 0) { log_debug_errno(r, "Failed to wait for child: %m"); goto finish; @@ -1242,7 +1239,7 @@ static int mount_in_namespace( _cleanup_(dissected_image_unrefp) DissectedImage *img = NULL; _cleanup_close_ int new_mount_fd = -EBADF; _cleanup_close_pair_ int errno_pipe_fd[2] = EBADF_PAIR; - pid_t child; + _cleanup_(pidref_done) PidRef child = PIDREF_NULL; if (flags & MOUNT_IN_NAMESPACE_IS_IMAGE) { r = verity_dissect_and_mount( @@ -1286,8 +1283,6 @@ static int mount_in_namespace( r = namespace_fork("(sd-bindmnt)", "(sd-bindmnt-inner)", - /* except_fds= */ NULL, - /* n_except_fds= */ 0, FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGTERM, pidns_fd, mntns_fd, @@ -1335,7 +1330,7 @@ static int mount_in_namespace( errno_pipe_fd[1] = safe_close(errno_pipe_fd[1]); - r = wait_for_terminate_and_check("(sd-bindmnt)", child, 0); + r = pidref_wait_for_terminate_and_check("(sd-bindmnt)", &child, 0); if (r < 0) return log_debug_errno(r, "Failed to wait for child: %m"); if (r != EXIT_SUCCESS) { From ed0f6758c82f4380baf31612290a395bb7df87a6 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 14 Dec 2025 16:02:17 +0100 Subject: [PATCH 4/5] process-util: revamp flags handling in namespace_fork() * Specifying all 3 of FORK_DEATHSIG_SIG{KILL,TERM,INT} for the middle man makes zero sense. Use SIGKILL only. * Make sure operations on except_fds work sensibly - close/pack/ de-ocloexecify fds only in the second level, so that the namespace fds remain usable across first safe_fork(). * Fire FORK_NEW_*NS after attaching to the desired namespaces, not already in the outer process. * Insist on PDEATHSIG being enabled to ensure propagation of killing. * Suppress more redundant flags. --- src/basic/process-util.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/basic/process-util.c b/src/basic/process-util.c index 700fba5cba..0a87c5ec77 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -1901,14 +1901,21 @@ int namespace_fork_full( /* This is much like safe_fork(), but forks twice, and joins the specified namespaces in the middle * process. This ensures that we are fully a member of the destination namespace, with pidns an all, so that - * /proc/self/fd works correctly. */ + * /proc/self/fd works correctly. + * + * TODO: once we can rely on PIDFD_INFO_EXIT, do not keep the middle process around and instead + * return the pidfd of the inner process for direct tracking. */ + + /* Insist on PDEATHSIG being enabled, as the pid returned is the one of the middle man, and otherwise + * killing of it won't be propagated to the inner child. */ + assert((flags & (FORK_DEATHSIG_SIGKILL|FORK_DEATHSIG_SIGTERM|FORK_DEATHSIG_SIGINT)) != 0); + assert((flags & (FORK_DETACH|FORK_FREEZE)) == 0); assert(!FLAGS_SET(flags, FORK_ALLOW_DLOPEN)); /* never allow loading shared library from another ns */ r = pidref_safe_fork_full( outer_name, - NULL, - except_fds, n_except_fds, - (flags|FORK_DEATHSIG_SIGINT|FORK_DEATHSIG_SIGTERM|FORK_DEATHSIG_SIGKILL) & ~(FORK_REOPEN_LOG|FORK_NEW_MOUNTNS|FORK_MOUNTNS_SLAVE), + /* stdio_fds = */ NULL, /* except_fds = */ NULL, /* n_except_fds = */ 0, + (flags|FORK_DEATHSIG_SIGKILL) & ~(FORK_DEATHSIG_SIGTERM|FORK_DEATHSIG_SIGINT|FORK_REOPEN_LOG|FORK_NEW_MOUNTNS|FORK_MOUNTNS_SLAVE|FORK_NEW_USERNS|FORK_NEW_NETNS|FORK_NEW_PIDNS|FORK_CLOSE_ALL_FDS|FORK_PACK_FDS|FORK_CLOEXEC_OFF|FORK_RLIMIT_NOFILE_SAFE), ret); if (r < 0) return r; @@ -1928,7 +1935,7 @@ int namespace_fork_full( inner_name, NULL, except_fds, n_except_fds, - flags & ~(FORK_WAIT|FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_REARRANGE_STDIO), + flags & ~(FORK_WAIT|FORK_RESET_SIGNALS|FORK_REARRANGE_STDIO|FORK_FLUSH_STDIO|FORK_STDOUT_TO_STDERR), &pidref_inner); if (r < 0) _exit(EXIT_FAILURE); @@ -1939,6 +1946,11 @@ int namespace_fork_full( return 0; } + log_close(); + log_set_open_when_needed(true); + + (void) close_all_fds(&pidref_inner.fd, 1); + r = pidref_wait_for_terminate_and_check( inner_name, &pidref_inner, From 73c40aa1f661e34251fe77bc38a6a87d762572a9 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 15 Dec 2025 16:07:17 +0100 Subject: [PATCH 5/5] process-util: synchronously wait for namespace_enter() before returning Additionally, report actual errors using errno pipe too. --- src/basic/process-util.c | 46 +++++++++++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/src/basic/process-util.c b/src/basic/process-util.c index 0a87c5ec77..f25f4b81e5 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -1897,7 +1897,9 @@ int namespace_fork_full( int root_fd, PidRef *ret) { - int r; + _cleanup_(pidref_done_sigkill_wait) PidRef pidref_outer = PIDREF_NULL; + _cleanup_close_pair_ int errno_pipe_fd[2] = EBADF_PAIR; + int r, prio = FLAGS_SET(flags, FORK_LOG) ? LOG_ERR : LOG_DEBUG; /* This is much like safe_fork(), but forks twice, and joins the specified namespaces in the middle * process. This ensures that we are fully a member of the destination namespace, with pidns an all, so that @@ -1912,11 +1914,23 @@ int namespace_fork_full( assert((flags & (FORK_DETACH|FORK_FREEZE)) == 0); assert(!FLAGS_SET(flags, FORK_ALLOW_DLOPEN)); /* never allow loading shared library from another ns */ + /* We want read() to block as a synchronization point */ + assert_cc(sizeof(int) <= PIPE_BUF); + if (pipe2(errno_pipe_fd, O_CLOEXEC) < 0) + return log_full_errno(prio, errno, "Failed to create pipe: %m"); + r = pidref_safe_fork_full( outer_name, /* stdio_fds = */ NULL, /* except_fds = */ NULL, /* n_except_fds = */ 0, (flags|FORK_DEATHSIG_SIGKILL) & ~(FORK_DEATHSIG_SIGTERM|FORK_DEATHSIG_SIGINT|FORK_REOPEN_LOG|FORK_NEW_MOUNTNS|FORK_MOUNTNS_SLAVE|FORK_NEW_USERNS|FORK_NEW_NETNS|FORK_NEW_PIDNS|FORK_CLOSE_ALL_FDS|FORK_PACK_FDS|FORK_CLOEXEC_OFF|FORK_RLIMIT_NOFILE_SAFE), - ret); + &pidref_outer); + if (r == -EPROTO && FLAGS_SET(flags, FORK_WAIT)) { + errno_pipe_fd[1] = safe_close(errno_pipe_fd[1]); + + int k = read_errno(errno_pipe_fd[0]); + if (k < 0 && k != -EIO) + return k; + } if (r < 0) return r; if (r == 0) { @@ -1924,10 +1938,12 @@ int namespace_fork_full( /* Child */ + errno_pipe_fd[0] = safe_close(errno_pipe_fd[0]); + r = namespace_enter(pidns_fd, mntns_fd, netns_fd, userns_fd, root_fd); if (r < 0) { - log_full_errno(FLAGS_SET(flags, FORK_LOG) ? LOG_ERR : LOG_DEBUG, r, "Failed to join namespace: %m"); - _exit(EXIT_FAILURE); + log_full_errno(prio, r, "Failed to join namespace: %m"); + report_errno_and_exit(errno_pipe_fd[1], r); } /* We mask a few flags here that either make no sense for the grandchild, or that we don't have to do again */ @@ -1938,9 +1954,18 @@ int namespace_fork_full( flags & ~(FORK_WAIT|FORK_RESET_SIGNALS|FORK_REARRANGE_STDIO|FORK_FLUSH_STDIO|FORK_STDOUT_TO_STDERR), &pidref_inner); if (r < 0) - _exit(EXIT_FAILURE); + report_errno_and_exit(errno_pipe_fd[1], r); if (r == 0) { /* Child */ + + if (!FLAGS_SET(flags, FORK_CLOSE_ALL_FDS)) { + errno_pipe_fd[1] = safe_close(errno_pipe_fd[1]); + pidref_done(&pidref_outer); + } else { + errno_pipe_fd[1] = -EBADF; + pidref_outer = PIDREF_NULL; + } + if (ret) *ret = TAKE_PIDREF(pidref_inner); return 0; @@ -1961,6 +1986,17 @@ int namespace_fork_full( _exit(r); } + errno_pipe_fd[1] = safe_close(errno_pipe_fd[1]); + + r = read_errno(errno_pipe_fd[0]); + if (r < 0) + return r; /* the child logs about failures on its own, no need to duplicate here */ + + if (ret) + *ret = TAKE_PIDREF(pidref_outer); + else + pidref_done(&pidref_outer); /* disarm sigkill_wait */ + return 1; }