diff options
| author | Rich Felker <dalias@aerifal.cx> | 2022-10-06 20:53:01 -0400 |
|---|---|---|
| committer | Rich Felker <dalias@aerifal.cx> | 2022-10-19 14:01:32 -0400 |
| commit | aebd6a36449e91c06763a40121d558b6cea90d50 (patch) | |
| tree | a597d9d05d15d0767e7063d287fe3f9bd80f33fe /src/process | |
| parent | d64148a8743ad9ed0594091d2ff141b1e9334d4b (diff) | |
| download | musl-aebd6a36449e91c06763a40121d558b6cea90d50.tar.gz | |
fix potential deadlock between multithreaded fork and aio
as reported by Alexey Izbyshev, there is a lock order inversion
deadlock between the malloc lock and aio maplock at MT-fork time:
_Fork attempts to take the aio maplock while fork already has the
malloc lock, but a concurrent aio operation holding the maplock may
attempt to allocate memory.
move the __aio_atfork calls in the parent from _Fork to fork, and
reorder the lock before most other locks, since nothing else depends
on aio(*). this leaves us with the possibility that the child will not
be able to obtain the read lock, if _Fork is used directly and happens
concurrent with an aio operation. however, in that case, the child
context is an async signal context that cannot call any further aio
functions, so all we need is to ensure that close does not attempt to
perform any aio cancellation. this can be achieved just by nulling out
the map pointer.
(*) even if other functions call close, they will only need a read
lock, not a write lock, and read locks being recursive ensures they
can obtain it. moreover, the number of read references held is bounded
by something like twice the number of live threads, meaning that the
read lock count cannot saturate.
Diffstat (limited to 'src/process')
| -rw-r--r-- | src/process/_Fork.c | 3 | ||||
| -rw-r--r-- | src/process/fork.c | 3 |
2 files changed, 4 insertions, 2 deletions
diff --git a/src/process/_Fork.c b/src/process/_Fork.c index da063868..fb0fdc2c 100644 --- a/src/process/_Fork.c +++ b/src/process/_Fork.c @@ -14,7 +14,6 @@ pid_t _Fork(void) pid_t ret; sigset_t set; __block_all_sigs(&set); - __aio_atfork(-1); LOCK(__abort_lock); #ifdef SYS_fork ret = __syscall(SYS_fork); @@ -32,7 +31,7 @@ pid_t _Fork(void) if (libc.need_locks) libc.need_locks = -1; } UNLOCK(__abort_lock); - __aio_atfork(!ret); + if (!ret) __aio_atfork(1); __restore_sigs(&set); return __syscall_ret(ret); } diff --git a/src/process/fork.c b/src/process/fork.c index ff71845c..80e804b1 100644 --- a/src/process/fork.c +++ b/src/process/fork.c @@ -36,6 +36,7 @@ static volatile int *const *const atfork_locks[] = { static void dummy(int x) { } weak_alias(dummy, __fork_handler); weak_alias(dummy, __malloc_atfork); +weak_alias(dummy, __aio_atfork); weak_alias(dummy, __ldso_atfork); static void dummy_0(void) { } @@ -50,6 +51,7 @@ pid_t fork(void) int need_locks = libc.need_locks > 0; if (need_locks) { __ldso_atfork(-1); + __aio_atfork(-1); __inhibit_ptc(); for (int i=0; i<sizeof atfork_locks/sizeof *atfork_locks; i++) if (*atfork_locks[i]) LOCK(*atfork_locks[i]); @@ -75,6 +77,7 @@ pid_t fork(void) if (ret) UNLOCK(*atfork_locks[i]); else **atfork_locks[i] = 0; __release_ptc(); + if (ret) __aio_atfork(0); __ldso_atfork(!ret); } __restore_sigs(&set); |