From 7931b41e4b7ff7bc0c25d3141fef1fdc113b8203 Mon Sep 17 00:00:00 2001 From: Markus Wichmann Date: Sat, 20 Jan 2024 08:44:04 +0100 Subject: [PATCH] Introduce new fork lock separate from __abort_lock. The abort lock was being taken in several functions that have nothing to do with SIGABRT or signal dispositions. The lock had become dual-purpose, guarding against SIGABRT disposition changes and against a concurrent fork. But these are two different purposes, so use a new lock for the second one. posix_spawn() has no reason to care about the SIGABRT disposition any more than any other one, and it only changes signal dispositions in the child process, which cannot call abort() concurrently because it is single-threaded. --- src/internal/fork_impl.h | 1 + src/linux/clone.c | 2 +- src/process/_Fork.c | 4 ++-- src/process/fork_lock.c | 3 +++ src/process/posix_spawn.c | 10 +++++----- 5 files changed, 12 insertions(+), 8 deletions(-) create mode 100644 src/process/fork_lock.c diff --git a/src/internal/fork_impl.h b/src/internal/fork_impl.h index f995fce2..3889d38a 100644 --- a/src/internal/fork_impl.h +++ b/src/internal/fork_impl.h @@ -13,6 +13,7 @@ extern hidden volatile int *const __timezone_lockptr; extern hidden volatile int *const __bump_lockptr; extern hidden volatile int *const __vmlock_lockptr; +extern hidden volatile int __fork_lock[1]; hidden void __malloc_atfork(int); hidden void __ldso_atfork(int); diff --git a/src/linux/clone.c b/src/linux/clone.c index 257c1cec..1e4201ae 100644 --- a/src/linux/clone.c +++ b/src/linux/clone.c @@ -51,7 +51,7 @@ int clone(int (*func)(void *), void *stack, int flags, void *arg, ...) __clone(func, stack, flags, arg, ptid, tls, ctid)); __block_all_sigs(&csa.sigmask); - LOCK(__abort_lock); + LOCK(__fork_lock); /* Setup the a wrapper start function for the child process to do * mimic _Fork in producing a consistent execution state. */ diff --git a/src/process/_Fork.c b/src/process/_Fork.c index 9c07792d..370191c1 100644 --- a/src/process/_Fork.c +++ b/src/process/_Fork.c @@ -22,7 +22,7 @@ void __post_Fork(int ret) libc.threads_minus_1 = 0; if (libc.need_locks) libc.need_locks = -1; } - UNLOCK(__abort_lock); + UNLOCK(__fork_lock); if (!ret) __aio_atfork(1); } @@ -31,7 +31,7 @@ pid_t _Fork(void) pid_t ret; sigset_t set; __block_all_sigs(&set); - LOCK(__abort_lock); + LOCK(__fork_lock); #ifdef SYS_fork ret = __syscall(SYS_fork); #else diff --git a/src/process/fork_lock.c b/src/process/fork_lock.c new file mode 100644 index 00000000..204af1f4 --- /dev/null +++ b/src/process/fork_lock.c @@ -0,0 +1,3 @@ +#include "fork_impl.h" + +volatile int __fork_lock[1]; diff --git a/src/process/posix_spawn.c b/src/process/posix_spawn.c index 728551b3..74883527 100644 --- a/src/process/posix_spawn.c +++ b/src/process/posix_spawn.c @@ -9,6 +9,7 @@ #include "lock.h" #include "pthread_impl.h" #include "fdop.h" +#include "fork_impl.h" struct args { int p[2]; @@ -180,12 +181,11 @@ int posix_spawn(pid_t *restrict res, const char *restrict path, args.envp = envp; pthread_sigmask(SIG_BLOCK, SIGALL_SET, &args.oldmask); - /* The lock guards both against seeing a SIGABRT disposition change - * by abort and against leaking the pipe fd to fork-without-exec. */ - LOCK(__abort_lock); + /* The lock guards against leaking the pipe fd to fork-without-exec. */ + LOCK(__fork_lock); if (pipe2(args.p, O_CLOEXEC)) { - UNLOCK(__abort_lock); + UNLOCK(__fork_lock); ec = errno; goto fail; } @@ -193,7 +193,7 @@ int posix_spawn(pid_t *restrict res, const char *restrict path, pid = __clone(child, stack+sizeof stack, CLONE_VM|CLONE_VFORK|SIGCHLD, &args); close(args.p[1]); - UNLOCK(__abort_lock); + UNLOCK(__fork_lock); if (pid > 0) { if (read(args.p[0], &ec, sizeof ec) != sizeof ec) ec = 0; -- 2.39.2