Race condition in heap coalesce.

    Previously the heap coalesce function looked at the "free" flag
    on the next block to determine if two blocks could be merged
    together.  The coalesce function is ran with all cores synchronized
    so no one should be modifying the heap, but there is still a
    subtle race condition.

    A user task could be in the process of freeing a block, marking it
    "free" but not actually inserting it onto the queues yet.  The
    coalesce will combine this block together but then when the user
    task resumes it continues to try to insert it onto the queue.
    This causes both the larger merged block and the smaller block to
    be on a free queue and memory corruption when both blocks are used.

    This is resolved by having a new flag indicating that the block
    is known to be part of the "coalesce" group and using this in
    addition to the "free" flag.  Only blocks which are actually present
    on the free queues are considered to be part of the "coalesce"
    group.

Change-Id: If33d15d731d32e07e01104244ebc65daf2295878
Reviewed-on: http://gfw160.austin.ibm.com:8080/gerrit/4520
Reviewed-by: Douglas R. Gilbert <dgilbert@us.ibm.com>
Reviewed-by: Daniel M. Crowell <dcrowell@us.ibm.com>
Tested-by: Jenkins Server
Reviewed-by: A. Patrick Williams III <iawillia@us.ibm.com>

1 files changed, 8 insertions, 4 deletions

diff --git a/src/kernel/heapmgr.C b/src/kernel/heapmgr.C
index 5518a96f8..d2df127e9 100644
--- a/src/kernel/heapmgr.C
+++ b/src/kernel/heapmgr.C
@@ -330,7 +330,10 @@ void HeapManager::_coalesce()
         chunk = NULL;
         while(NULL != (chunk = first_chunk[bucket].pop()))
         {
+            kassert(chunk->free);
+
             chunk->next = head;
+            chunk->coalesce = true;
             head = chunk;
         }
     }
@@ -351,7 +354,7 @@ void HeapManager::_coalesce()
             {
                 // This chunk might already be combined with a chunk earlier
                 // in the loop.
-                if(!chunk->free)
+                if((!chunk->coalesce) || (!chunk->free))
                 {
                     break;
                 }
@@ -370,7 +373,7 @@ void HeapManager::_coalesce()
                 }
 
                 // Cannot merge if buddy is not free.
-                if (!buddy->free)
+                if ((!buddy->free) || (!buddy->coalesce))
                 {
                     break;
                 }
@@ -387,7 +390,7 @@ void HeapManager::_coalesce()
                 }
 
                 // Do merge.
-                buddy->free = false;
+                buddy->free = buddy->coalesce = false;
                 chunk->bucket = newBucket;
                 incrementChunk = false;
                 mergedChunks = true;
@@ -407,7 +410,7 @@ void HeapManager::_coalesce()
         chunk = head;
         while (NULL != chunk)
         {
-            if (chunk->free)
+            if ((chunk->free) && (chunk->coalesce))
             {
                 chunk_t* temp = chunk->next;
                 chunk->next = newHead;
@@ -434,6 +437,7 @@ void HeapManager::_coalesce()
     {
         chunk_t * temp = chunk->next;
 
+        chunk->coalesce = false;
         push_bucket(chunk,chunk->bucket);
 
         ++cv_free_chunks;