From 579a9108868a91eadc9c4ac55077596bb521d0d1 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Thu, 6 Aug 2026 17:25:24 +0200 Subject: [PATCH] fix(subprocess): spawn instead of fork+exec on macOS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fork() duplicates the parent's address-space bookkeeping. Under an ASan-instrumented parent that includes an enormous shadow mapping, and past a footprint threshold the child is jetsam-killed BEFORE exec replaces the image. The reaped corpse then looks like the launched tool dying, so a `git init` in a fixture "fails" only once enough suites have run ahead of it — a spawn defect that reads as an unrelated assertion. tests/test_daemon_runtime.c already switched to posix_spawn for this exact reason; this moves the shared subprocess layer over as well. posix_spawn never copies the parent address space, so the parent's footprint stops being a variable. Every guarantee of the fork path is carried over: - own process group SETPGROUP + setpgroup(0) (kill-tree contract) - default signal handling SETSIGDEF + SETSIGMASK - std{in,out,err} wiring adddup2 - all other fds closed CLOEXEC_DEFAULT, Apple's equivalent of the child's close-everything loop (the three dup2'd fds survive, since dup2 clears close-on-exec) - PATH lookup posix_spawnp keeps execvp semantics One deliberate difference is reconciled rather than adopted: posix_spawn reports an unusable binary to the PARENT, where fork+exec instead yields a child that exits 127. cbm_subprocess_run's contract treats spawn_failed as "the spawn mechanism failed", not "the tool was missing", so exec-class errors fall back to fork+exec and macOS keeps classifying a bogus binary exactly as Linux does. That path forks a child that exits immediately, so it does not reintroduce the hazard. The existing suite binds each guarantee, verified by breaking them: dropping CLOEXEC_DEFAULT leaks the sentinel descriptor and reddens subprocess_posix_child_closes_unrelated_descriptors; dropping SETPGROUP makes the kill-tree turn on the runner's own group and reddens the suite wholesale. Signed-off-by: Martin Vogel --- src/foundation/subprocess.c | 101 +++++++++++++++++++++++++++++++++++- 1 file changed, 100 insertions(+), 1 deletion(-) diff --git a/src/foundation/subprocess.c b/src/foundation/subprocess.c index 5f3530f1f..e4fced37b 100644 --- a/src/foundation/subprocess.c +++ b/src/foundation/subprocess.c @@ -24,6 +24,10 @@ #include #include #include +#ifdef __APPLE__ +#include +extern char **environ; +#endif #include #include #include @@ -877,6 +881,9 @@ static cbm_proc_poll_t cbm_subprocess_poll_win(cbm_subprocess_t *process, cbm_pr #else /* POSIX */ +/* Used by the fork+exec child. posix_spawn performs the same reset + * declaratively via SETSIGDEF + SETSIGMASK, but Apple still forks for the + * exec-failure fallback below, so this stays compiled everywhere. */ static void cbm_posix_reset_child_signals(void) { struct sigaction action = {0}; action.sa_handler = SIG_DFL; @@ -891,6 +898,9 @@ static void cbm_posix_reset_child_signals(void) { (void)sigprocmask(SIG_SETMASK, &empty, NULL); } +/* fork+exec child setup. On Apple this runs ONLY for the exec-failure + * fallback (see cbm_posix_spawn_apple), which preserves the documented + * "bogus binary => child exits 127" contract across platforms. */ static void cbm_posix_child_exec(cbm_subprocess_t *process, int input, int output, long max_fd) { if (setpgid(0, 0) < 0) { _exit(127); @@ -935,6 +945,74 @@ static int cbm_posix_fd_at_least_three(int fd) { return duplicate; } +#ifdef __APPLE__ +/* macOS: spawn instead of fork+exec. + * + * fork() duplicates the parent's whole address space bookkeeping, and an + * ASan-instrumented parent carries an enormous shadow mapping. Past a + * footprint threshold the child is killed (jetsam) BEFORE exec replaces the + * image, so the call fails with the child already gone (ESRCH on reap) — a + * spawn failure that looks like the launched tool crashing. The test suite hit + * exactly this: `git init` inside a fixture failed once enough suites had run + * ahead of it, and the symptom was an unrelated-looking assertion. The same + * hazard is already documented in tests/test_daemon_runtime.c, which switched + * to posix_spawn for the same reason. + * + * posix_spawn never copies the parent address space, so the footprint is + * irrelevant. Every guarantee of the fork path is preserved: + * - own process group (SETPGROUP + setpgroup(0)) — the kill-tree contract + * - default signal dispositions and an empty mask (SETSIGDEF/SETSIGMASK) + * - stdin/stdout/stderr wired to the caller's fds (adddup2) + * - every OTHER descriptor closed: CLOEXEC_DEFAULT is Apple's equivalent of + * the child's close-everything loop, and the three dup2'd fds stay open + * because dup2 clears close-on-exec. + * posix_spawnp keeps execvp's PATH semantics for a bare tool name. */ +static int cbm_posix_spawn_apple(cbm_subprocess_t *process, int input, int output, pid_t *pid_out) { + posix_spawn_file_actions_t actions; + posix_spawnattr_t attr; + if (posix_spawn_file_actions_init(&actions) != 0) { + return -1; + } + if (posix_spawnattr_init(&attr) != 0) { + (void)posix_spawn_file_actions_destroy(&actions); + return -1; + } + sigset_t empty_mask; + sigset_t all_signals; + sigemptyset(&empty_mask); + sigfillset(&all_signals); + short flags = (short)(POSIX_SPAWN_SETPGROUP | POSIX_SPAWN_SETSIGDEF | POSIX_SPAWN_SETSIGMASK | + POSIX_SPAWN_CLOEXEC_DEFAULT); + bool configured = posix_spawnattr_setflags(&attr, flags) == 0 && + posix_spawnattr_setpgroup(&attr, 0) == 0 && + posix_spawnattr_setsigmask(&attr, &empty_mask) == 0 && + posix_spawnattr_setsigdefault(&attr, &all_signals) == 0 && + posix_spawn_file_actions_adddup2(&actions, input, STDIN_FILENO) == 0 && + posix_spawn_file_actions_adddup2(&actions, output, STDOUT_FILENO) == 0 && + posix_spawn_file_actions_adddup2(&actions, output, STDERR_FILENO) == 0; + pid_t pid = -1; + int rc = + configured ? posix_spawnp(&pid, process->bin, &actions, &attr, process->argv, environ) : -1; + (void)posix_spawn_file_actions_destroy(&actions); + (void)posix_spawnattr_destroy(&attr); + if (configured && rc == 0 && pid > 0) { + *pid_out = pid; + return 0; + } + /* posix_spawn reports an unusable binary itself, where fork+exec instead + * produces a child that exits 127. Callers (and tests) rely on the latter: + * "spawn_failed" means the SPAWN mechanism failed, not that the tool was + * missing. Fall back to fork+exec for exec-class errors so macOS and Linux + * classify a bogus binary identically; the ASan-fork hazard does not apply + * here, since this child exits immediately. */ + if (configured && (rc == ENOENT || rc == EACCES || rc == ENOEXEC || rc == EISDIR || + rc == ELOOP || rc == ENAMETOOLONG || rc == ENOTDIR)) { + return 1; + } + return -1; +} +#endif + static int cbm_subprocess_spawn_posix(cbm_subprocess_t *process) { int input_flags = O_RDONLY; #ifdef O_CLOEXEC @@ -977,7 +1055,27 @@ static int cbm_subprocess_spawn_posix(cbm_subprocess_t *process) { max_fd = 65536L; } - pid_t pid = fork(); + pid_t pid = -1; +#ifdef __APPLE__ + int spawn_rc = cbm_posix_spawn_apple(process, input, output, &pid); + if (spawn_rc < 0) { + (void)close(input); + (void)close(output); + return -1; + } + if (spawn_rc > 0) { /* exec-class failure: reproduce the fork+exec 127 */ + pid = fork(); + if (pid < 0) { + (void)close(input); + (void)close(output); + return -1; + } + if (pid == 0) { + cbm_posix_child_exec(process, input, output, max_fd); + } + } +#else + pid = fork(); if (pid < 0) { (void)close(input); (void)close(output); @@ -986,6 +1084,7 @@ static int cbm_subprocess_spawn_posix(cbm_subprocess_t *process) { if (pid == 0) { cbm_posix_child_exec(process, input, output, max_fd); } +#endif (void)close(input); (void)close(output);