From d6d02c67d2bcacaa02499930aabd76365797b108 Mon Sep 17 00:00:00 2001
From: Christian Walther <cwalther@gmx.ch>
Date: Mon, 28 Sep 2020 22:34:02 +0200
Subject: [PATCH] Fix inconsistent supervisor heap.

When allocations were freed in a different order from the reverse of how they were allocated (leaving holes), the heap would get into an inconsistent state, eventually resulting in crashes.

free_memory() relies on having allocations in order, but allocate_memory() did not guarantee that: It reused the first allocation with a NULL ptr without ensuring that it was between low_address and high_address. When it belongs to a hole in the allocated memory, such an allocation is not really free for reuse, because free_memory() still needs its length.

Instead, explicitly mark allocations available for reuse with a special (invalid) value in the length field. Only allocations that lie between low_address and high_address are marked that way.
---
 supervisor/shared/memory.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/supervisor/shared/memory.c b/supervisor/shared/memory.c
index 8ae8a16997..56c617b9cf 100755
--- a/supervisor/shared/memory.c
+++ b/supervisor/shared/memory.c
@@ -33,6 +33,10 @@
 
 #define CIRCUITPY_SUPERVISOR_ALLOC_COUNT (12)
 
+// Using a zero length to mark an unused allocation makes the code a bit shorter (but makes it
+// impossible to support zero-length allocations).
+#define FREE 0
+
 static supervisor_allocation allocations[CIRCUITPY_SUPERVISOR_ALLOC_COUNT];
 // We use uint32_t* to ensure word (4 byte) alignment.
 uint32_t* low_address;
@@ -61,19 +65,23 @@ void free_memory(supervisor_allocation* allocation) {
     }
     if (allocation->ptr == high_address) {
         high_address += allocation->length / 4;
+        allocation->length = FREE;
         for (index++; index < CIRCUITPY_SUPERVISOR_ALLOC_COUNT; index++) {
             if (allocations[index].ptr != NULL) {
                 break;
             }
             high_address += allocations[index].length / 4;
+            allocations[index].length = FREE;
         }
     } else if (allocation->ptr + allocation->length / 4 == low_address) {
         low_address = allocation->ptr;
+        allocation->length = FREE;
         for (index--; index >= 0; index--) {
             if (allocations[index].ptr != NULL) {
                 break;
             }
             low_address -= allocations[index].length / 4;
+            allocations[index].length = FREE;
         }
     } else {
         // Freed memory isn't in the middle so skip updating bounds. The memory will be added to the
@@ -99,7 +107,7 @@ supervisor_allocation* allocate_remaining_memory(void) {
 }
 
 supervisor_allocation* allocate_memory(uint32_t length, bool high) {
-    if ((high_address - low_address) * 4 < (int32_t) length || length % 4 != 0) {
+    if (length == 0 || (high_address - low_address) * 4 < (int32_t) length || length % 4 != 0) {
         return NULL;
     }
     uint8_t index = 0;
@@ -109,7 +117,7 @@ supervisor_allocation* allocate_memory(uint32_t length, bool high) {
         direction = -1;
     }
     for (; index < CIRCUITPY_SUPERVISOR_ALLOC_COUNT; index += direction) {
-        if (allocations[index].ptr == NULL) {
+        if (allocations[index].length == FREE) {
             break;
         }
     }