Fix a use-after-free bug for detached threads (#420)

* Fix a use-after-free bug for detached threads

the issue was pointed out by @alexcrichton in
https://github.com/WebAssembly/wasi-libc/issues/405

* Rename map_base_lazy_free_queue as it only keeps a single item

Also, align the comment style with musl.

diff --git a/libc-top-half/musl/src/thread/pthread_create.c b/libc-top-half/musl/src/thread/pthread_create.c
index 12094d63489c1f109c5c5a08da9f4613b9887f10..5de9f5a0cbfc23240bf7110ba83a2bdac352fff1 100644 (file)
--- a/libc-top-half/musl/src/thread/pthread_create.c
+++ b/libc-top-half/musl/src/thread/pthread_create.c
@@ -60,6 +60,17 @@ void __tl_sync(pthread_t td)
        if (tl_lock_waiters) __wake(&__thread_list_lock, 1, 0);
 }
 
+#ifndef __wasilibc_unmodified_upstream
+static void *map_base_deferred_free;
+
+static void process_map_base_deferred_free()
+{
+       /* called with __tl_lock held */
+       free(map_base_deferred_free);
+       map_base_deferred_free = NULL;
+}
+#endif
+
 #ifdef __wasilibc_unmodified_upstream
 _Noreturn void __pthread_exit(void *result)
 #else
@@ -182,7 +193,17 @@ static void __pthread_exit(void *result)
        }
 #else
        if (state==DT_DETACHED && self->map_base) {
-               free(self->map_base);
+               /* As we use malloc/free which is considerably more complex
+                * than mmap/munmap to call and can even require a valid
+                * thread context, it's difficult to implement __unmapself.
+                *
+                * Here we take an alternative approach which simply defers
+                * the deallocation. An obvious downside of this approach is
+                * that it keeps the stack longer. (possibly forever.)
+                * To avoid wasting too much memory, we only defer a single
+                * item at most. */
+               process_map_base_deferred_free();
+               map_base_deferred_free = self->map_base;
                // Can't use `exit()` here, because it is too high level
                return;
        }
@@ -424,6 +445,14 @@ int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict att
                        if (map == MAP_FAILED) goto fail;
                }
 #else
+               /* Process the deferred free request if any before
+                * allocationg a new one. Hopefully it enables a reuse of the memory.
+                *
+                * Note: We can't perform a simple "handoff" becasue allocation
+                * sizes might be different. (eg. the stack size might differ) */
+               __tl_lock();
+               process_map_base_deferred_free();
+               __tl_unlock();
                map = malloc(size);
                if (!map) goto fail;
 #endif