about summary refs log tree commit diff
path: root/malloc/arena.c
diff options
context:
space:
mode:
authorSiddhesh Poyarekar <siddhesh@redhat.com>2015-05-19 06:40:37 +0530
committerSiddhesh Poyarekar <siddhesh@redhat.com>2015-05-19 06:40:38 +0530
commitfff94fa2245612191123a8015eac94eb04f001e2 (patch)
tree8a1881efb6cc8b2964773a3597b1c6406e85c87f /malloc/arena.c
parent99db95db37b4fd95986fadb263e4180b7381d10d (diff)
downloadglibc-fff94fa2245612191123a8015eac94eb04f001e2.tar.gz
glibc-fff94fa2245612191123a8015eac94eb04f001e2.tar.xz
glibc-fff94fa2245612191123a8015eac94eb04f001e2.zip
Avoid deadlock in malloc on backtrace (BZ #16159)
When the malloc subsystem detects some kind of memory corruption,
depending on the configuration it prints the error, a backtrace, a
memory map and then aborts the process.  In this process, the
backtrace() call may result in a call to malloc, resulting in
various kinds of problematic behavior.

In one case, the malloc it calls may detect a corruption and call
backtrace again, and a stack overflow may result due to the infinite
recursion.  In another case, the malloc it calls may deadlock on an
arena lock with the malloc (or free, realloc, etc.) that detected the
corruption.  In yet another case, if the program is linked with
pthreads, backtrace may do a pthread_once initialization, which
deadlocks on itself.

In all these cases, the program exit is not as intended.  This is
avoidable by marking the arena that malloc detected a corruption on,
as unusable.  The following patch does that.  Features of this patch
are as follows:

- A flag is added to the mstate struct of the arena to indicate if the
  arena is corrupt.

- The flag is checked whenever malloc functions try to get a lock on
  an arena.  If the arena is unusable, a NULL is returned, causing the
  malloc to use mmap or try the next arena.

- malloc_printerr sets the corrupt flag on the arena when it detects a
  corruption

- free does not concern itself with the flag at all.  It is not
  important since the backtrace workflow does not need free.  A free
  in a parallel thread may cause another corruption, but that's not
  new

- The flag check and set are not atomic and may race.  This is fine
  since we don't care about contention during the flag check.  We want
  to make sure that the malloc call in the backtrace does not trip on
  itself and all that action happens in the same thread and not across
  threads.

I verified that the test case does not show any regressions due to
this patch.  I also ran the malloc benchmarks and found an
insignificant difference in timings (< 2%).

	* malloc/Makefile (tests): New test case tst-malloc-backtrace.
	* malloc/arena.c (arena_lock): Check if arena is corrupt.
	(reused_arena): Find a non-corrupt arena.
	(heap_trim): Pass arena to unlink.
	* malloc/hooks.c (malloc_check_get_size): Pass arena to
	malloc_printerr.
	(top_check): Likewise.
	(free_check): Likewise.
	(realloc_check): Likewise.
	* malloc/malloc.c (malloc_printerr): Add arena argument.
	(unlink): Likewise.
	(munmap_chunk): Adjust.
	(ARENA_CORRUPTION_BIT): New macro.
	(arena_is_corrupt): Likewise.
	(set_arena_corrupt): Likewise.
	(sysmalloc): Use mmap if there are no usable arenas.
	(_int_malloc): Likewise.
	(__libc_malloc): Don't fail if arena_get returns NULL.
	(_mid_memalign): Likewise.
	(__libc_calloc): Likewise.
	(__libc_realloc): Adjust for additional argument to
	malloc_printerr.
	(_int_free): Likewise.
	(malloc_consolidate): Likewise.
	(_int_realloc): Likewise.
	(_int_memalign): Don't touch corrupt arenas.
	* malloc/tst-malloc-backtrace.c: New test case.
Diffstat (limited to 'malloc/arena.c')
-rw-r--r--malloc/arena.c22
1 files changed, 18 insertions, 4 deletions
diff --git a/malloc/arena.c b/malloc/arena.c
index d85f3712f7..2466697d1a 100644
--- a/malloc/arena.c
+++ b/malloc/arena.c
@@ -99,7 +99,7 @@ int __malloc_initialized = -1;
   } while (0)
 
 #define arena_lock(ptr, size) do {					      \
-      if (ptr)								      \
+      if (ptr && !arena_is_corrupt (ptr))				      \
         (void) mutex_lock (&ptr->mutex);				      \
       else								      \
         ptr = arena_get2 (ptr, (size), NULL);				      \
@@ -686,7 +686,7 @@ heap_trim (heap_info *heap, size_t pad)
       if (!prev_inuse (p)) /* consolidate backward */
         {
           p = prev_chunk (p);
-          unlink (p, bck, fwd);
+          unlink (ar_ptr, p, bck, fwd);
         }
       assert (((unsigned long) ((char *) p + new_size) & (pagesz - 1)) == 0);
       assert (((char *) p + new_size) == ((char *) heap + heap->size));
@@ -809,7 +809,7 @@ reused_arena (mstate avoid_arena)
   result = next_to_use;
   do
     {
-      if (!mutex_trylock (&result->mutex))
+      if (!arena_is_corrupt (result) && !mutex_trylock (&result->mutex))
         goto out;
 
       result = result->next;
@@ -821,7 +821,21 @@ reused_arena (mstate avoid_arena)
   if (result == avoid_arena)
     result = result->next;
 
-  /* No arena available.  Wait for the next in line.  */
+  /* Make sure that the arena we get is not corrupted.  */
+  mstate begin = result;
+  while (arena_is_corrupt (result) || result == avoid_arena)
+    {
+      result = result->next;
+      if (result == begin)
+	break;
+    }
+
+  /* We could not find any arena that was either not corrupted or not the one
+     we wanted to avoid.  */
+  if (result == begin || result == avoid_arena)
+    return NULL;
+
+  /* No arena available without contention.  Wait for the next in line.  */
   LIBC_PROBE (memory_arena_reuse_wait, 3, &result->mutex, result, avoid_arena);
   (void) mutex_lock (&result->mutex);