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>
Wed, 4 Mar 2015 14:29:39 +0000 (09:29 -0500)
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.

src/malloc/malloc.c

index 70c7b3f..4f61807 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 volatile 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)) {