tevent: Fix a race condition in tevent context rundown
authorVolker Lendecke <vl@samba.org>
Wed, 24 May 2017 14:22:34 +0000 (16:22 +0200)
committerJeremy Allison <jra@samba.org>
Thu, 8 Jun 2017 22:45:26 +0000 (00:45 +0200)
We protect setting tctx->event_ctx=NULL with tctx->event_ctx_mutex.
But in _tevent_threaded_schedule_immediate we have the classic
TOCTOU race: After we checked "ev==NULL", looking at
tevent_common_context_destructor the event context can go after
_tevent_threaded_schedule_immediate checked. We need to serialize
things a bit by keeping tctx->event_ctx_mutex locked while we
reference "ev", in particular in the

DLIST_ADD_END(ev->scheduled_immediates,im);

I think the locking hierarchy is still maintained, tevent_atfork_prepare()
first locks all the tctx locks, and then the scheduled_mutex.  Also,
I don't think this will impact parallelism too badly: event_ctx_mutex
is only used to protect setting tctx->ev.

Found by staring at code while fixing the FreeBSD memleak due to
not destroying scheduled_mutex.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Fri Jun  9 00:45:26 CEST 2017 on sn-devel-144

lib/tevent/tevent_threads.c

index 8197323af020e43444415da0f3d42191e8590452..8ecda027c331a615d73030609585c3412d661b17 100644 (file)
@@ -443,15 +443,14 @@ void _tevent_threaded_schedule_immediate(struct tevent_threaded_context *tctx,
 
        ev = tctx->event_ctx;
 
-       ret = pthread_mutex_unlock(&tctx->event_ctx_mutex);
-       if (ret != 0) {
-               abort();
-       }
-
        if (ev == NULL) {
                /*
                 * Our event context is already gone.
                 */
+               ret = pthread_mutex_unlock(&tctx->event_ctx_mutex);
+               if (ret != 0) {
+                       abort();
+               }
                return;
        }
 
@@ -479,6 +478,11 @@ void _tevent_threaded_schedule_immediate(struct tevent_threaded_context *tctx,
                abort();
        }
 
+       ret = pthread_mutex_unlock(&tctx->event_ctx_mutex);
+       if (ret != 0) {
+               abort();
+       }
+
        /*
         * We might want to wake up the main thread under the lock. We
         * had a slightly similar situation in pthreadpool, changed