[kernel] Fix freelist-clobber bug in heap allocator

The heap_allocator::get_free(order) function returns a reference to the
head pointer of the given freelist, so that it can be manipulated.
However, split_off was also taking a reference to a pointer for an out
param - passing the freelist pointer in here caused split_off to modify
the freelist.

I cleaned up a bunch of the places the freelist pointers were being
touched to make the usage more explicit.

src/kernel/heap_allocator.cpp

@@ -13,8 +13,9 @@ uint32_t & get_map_key(heap_allocator::block_info &info) { return info.offset; }
 
 struct heap_allocator::free_header
 {
-    void clear(unsigned new_order) {
+    void clear(unsigned new_order, free_header *new_next = nullptr) {
-        prev = next = nullptr;
+        prev = nullptr;
+        next = new_next;
         order = new_order;
     }
 
@@ -104,9 +105,11 @@ heap_allocator::free(void *p)
 heap_allocator::free_header *
 heap_allocator::pop_free(unsigned order)
 {
-    free_header *block = get_free(order);
+    free_header *& head = get_free(order);
+    free_header *block = head;
     if (block) {
-        get_free(order) = block->next;
+        kassert(block->prev == nullptr, "freelist head had non-null prev");
+        head = block->next;
         block->remove();
     }
     return block;
@@ -119,14 +122,17 @@ heap_allocator::merge_block(free_header *block)
 
     unsigned order = block->order;
     while (order < max_order) {
-        block_info *info = m_map.find(map_key(block->buddy()));
+        free_header *buddy = block->buddy();
+
+        block_info *info = m_map.find(map_key(buddy));
         if (!info || !info->free || info->order != order)
             break;
 
-        free_header *buddy = block->buddy();
+        free_header *&head = get_free(order);
-        if (get_free(order) == buddy)
+        if (head == buddy)
-            get_free(order) = buddy->next;
+            pop_free(order);
-        buddy->remove();
+        else
+            buddy->remove();
 
         block = block->eldest() ? block : buddy;
 
@@ -166,30 +172,31 @@ heap_allocator::register_free_block(free_header *block, unsigned order)
     info.free = true;
     info.order = order;
 
-    block->clear(order);
+    free_header *& head = get_free(order);
-    block->next = get_free(order);
+    if (head)
-    get_free(order) = block;
+        head->prev = block;
 
+    block->clear(order, head);
+    head = block;
 }
 
 bool
-heap_allocator::split_off(unsigned order, free_header *&block)
+heap_allocator::split_off(unsigned order, free_header *&split)
 {
     // The lock needs to be held while calling split_off
 
     const unsigned next = order + 1;
-    if (next > max_order) {
+    if (next > max_order)
-        block = nullptr;
         return false;
-    }
 
-    block = pop_free(next);
+    free_header *block = pop_free(next);
     if (!block && !split_off(next, block))
         return false;
 
     block->order = order;
-    free_header *buddy = block->buddy();
     register_free_block(block->buddy(), order);
     m_map[map_key(block)].order = order;
+
+    split = block;
     return true;
 }

src/kernel/heap_allocator.h

@@ -88,9 +88,9 @@ protected:
     /// Helper to get a block of the given order by splitting existing
     /// larger blocks. Returns false if there were no larger blocks.
     /// \arg order  Order (2^N) of the block we want
-    /// \arg block  [out] Receives a pointer to the requested block
+    /// \arg split  [out] Receives a pointer to the requested block
     /// \returns    True if a split was done
-    bool split_off(unsigned order, free_header *&block);
+    bool split_off(unsigned order, free_header *&split);
 
     uintptr_t m_start, m_end;
     size_t m_maxsize;