fix init race that could lead to deadlock in malloc init code
authorRich Felker <dalias@aerifal.cx>
Wed, 4 Mar 2015 14:29:39 +0000 (09:29 -0500)
committerRich Felker <dalias@aerifal.cx>
Mon, 30 Mar 2015 05:55:19 +0000 (01:55 -0400)
the malloc init code provided its own version of pthread_once type
logic, including the exact same bug that was fixed in pthread_once in
commit 0d0c2f40344640a2a6942dda156509593f51db5d.

since this code is called adjacent to expand_heap, which takes a lock,
there is no reason to have pthread_once-type initialization. simply
moving the init code into the interval where expand_heap already holds
its lock on the brk achieves the same result with much less
synchronization logic, and allows the buggy code to be eliminated
rather than just fixed.

(cherry picked from commit 7a81fe3710be0128d29071e76c5acbea3d84277b)

src/malloc/malloc.c

index 7932a97586b5fb2cf03e695c4ffff54c844c4a32..ed2285e7d4d8f99a2c41afb2e85cf2a0cbd24328 100644 (file)
@@ -154,11 +154,22 @@ void __dump_heap(int x)
 
 static struct chunk *expand_heap(size_t n)
 {
+       static int init;
        struct chunk *w;
        uintptr_t new;
 
        lock(mal.brk_lock);
 
+       if (!init) {
+               mal.brk = __brk(0);
+#ifdef SHARED
+               mal.brk = mal.brk + PAGE_SIZE-1 & -PAGE_SIZE;
+#endif
+               mal.brk = mal.brk + 2*SIZE_ALIGN-1 & -SIZE_ALIGN;
+               mal.heap = (void *)mal.brk;
+               init = 1;
+       }
+
        if (n > SIZE_MAX - mal.brk - 2*PAGE_SIZE) goto fail;
        new = mal.brk + n + SIZE_ALIGN + PAGE_SIZE - 1 & -PAGE_SIZE;
        n = new - mal.brk;
@@ -186,6 +197,9 @@ static struct chunk *expand_heap(size_t n)
                return area;
        }
 
+       w = MEM_TO_CHUNK(mal.heap);
+       w->psize = 0 | C_INUSE;
+
        w = MEM_TO_CHUNK(new);
        w->psize = n | C_INUSE;
        w->csize = 0 | C_INUSE;
@@ -203,44 +217,6 @@ fail:
        return 0;
 }
 
-static int init_malloc(size_t n)
-{
-       static int init, waiters;
-       int state;
-       struct chunk *c;
-
-       if (init == 2) return 0;
-
-       while ((state=a_swap(&init, 1)) == 1)
-               __wait(&init, &waiters, 1, 1);
-       if (state) {
-               a_store(&init, 2);
-               return 0;
-       }
-
-       mal.brk = __brk(0);
-#ifdef SHARED
-       mal.brk = mal.brk + PAGE_SIZE-1 & -PAGE_SIZE;
-#endif
-       mal.brk = mal.brk + 2*SIZE_ALIGN-1 & -SIZE_ALIGN;
-
-       c = expand_heap(n);
-
-       if (!c) {
-               a_store(&init, 0);
-               if (waiters) __wake(&init, 1, 1);
-               return -1;
-       }
-
-       mal.heap = (void *)c;
-       c->psize = 0 | C_INUSE;
-       free(CHUNK_TO_MEM(c));
-
-       a_store(&init, 2);
-       if (waiters) __wake(&init, -1, 1);
-       return 1;
-}
-
 static int adjust_size(size_t *n)
 {
        /* Result of pointer difference must fit in ptrdiff_t. */
@@ -375,7 +351,6 @@ void *malloc(size_t n)
        for (;;) {
                uint64_t mask = mal.binmap & -(1ULL<<i);
                if (!mask) {
-                       if (init_malloc(n) > 0) continue;
                        c = expand_heap(n);
                        if (!c) return 0;
                        if (alloc_rev(c)) {