parlib: Fix potential races with DTLS
authorBarret Rhoden <brho@cs.berkeley.edu>
Thu, 2 Mar 2017 17:56:21 +0000 (12:56 -0500)
committerBarret Rhoden <brho@cs.berkeley.edu>
Thu, 2 Mar 2017 18:01:29 +0000 (13:01 -0500)
I didn't see these happen, but stumbled on them as possibilities during
other bug hunts.

The DTLS one sounds a lot like a problem with glibc's version.  For the
pthread code, I'm being a little paranoid.  pthread_cleanup_pop() will call
free(), which also might want to use the DTLS.  free() should be able to
handle things if the key is deleted, but who knows.

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
user/parlib/dtls.c
user/pthread/pthread.c

index 52c9f18..676abb7 100644 (file)
@@ -199,10 +199,15 @@ static inline void __destroy_dtls(dtls_data_t *dtls_data)
                        v->dtls = NULL;
                        key->dtor(dtls);
                }
-               __maybe_free_dtls_key(key);
-
                n = TAILQ_NEXT(v, link);
                TAILQ_REMOVE(&dtls_data->list, v, link);
+               /* Free both the key (which is v->key) and v *after* removing v from the
+                * list.  It's possible that free() will call back into the DTLS (e.g.
+                * pthread_getspecific()), and v must be off the list by then.
+                *
+                * For a similar, hilarious bug in glibc, check out:
+                * https://sourceware.org/bugzilla/show_bug.cgi?id=3317 */
+               __maybe_free_dtls_key(key);
                __free_dtls_value(v);
                v = n;
        }
index 5d01b54..4d699d6 100644 (file)
@@ -694,10 +694,11 @@ static void __pth_exit_cb(struct uthread *uthread, void *junk)
 static inline void pthread_exit_no_cleanup(void *ret)
 {
        struct pthread_tcb *pthread = pthread_self();
+
        pthread->retval = ret;
-       destroy_dtls();
        while (SLIST_FIRST(&pthread->cr_stack))
                pthread_cleanup_pop(FALSE);
+       destroy_dtls();
        uthread_yield(FALSE, __pth_exit_cb, 0);
 }