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.

diff --git a/src/aio/aio.c b/src/aio/aio.c
index fa24f6b..82f8ecc 100644 (file)
--- a/src/aio/aio.c
+++ b/src/aio/aio.c
@@ -401,11 +401,26 @@ void __aio_atfork(int who)
        if (who<0) {
                pthread_rwlock_rdlock(&maplock);
                return;
+       } else if (!who) {
+               pthread_rwlock_unlock(&maplock);
+               return;
        }
-       if (who>0 && map) for (int a=0; a<(-1U/2+1)>>24; a++)
+       aio_fd_cnt = 0;
+       if (pthread_rwlock_tryrdlock(&maplock)) {
+               /* Obtaining lock may fail if _Fork was called nor via
+                * fork. In this case, no further aio is possible from
+                * child and we can just null out map so __aio_close
+                * does not attempt to do anything. */
+               map = 0;
+               return;
+       }
+       if (map) for (int a=0; a<(-1U/2+1)>>24; a++)
                if (map[a]) for (int b=0; b<256; b++)
                        if (map[a][b]) for (int c=0; c<256; c++)
                                if (map[a][b][c]) for (int d=0; d<256; d++)
                                        map[a][b][c][d] = 0;
-       pthread_rwlock_unlock(&maplock);
+       /* Re-initialize the rwlock rather than unlocking since there
+        * may have been more than one reference on it in the parent.
+        * We are not a lock holder anyway; the thread in the parent was. */
+       pthread_rwlock_init(&maplock, 0);
 }

diff --git a/src/process/_Fork.c b/src/process/_Fork.c
index da06386..fb0fdc2 100644 (file)
--- 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 ff71845..80e804b 100644 (file)
--- 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);