Skip to content

Commit d6d02c6

Browse files
cwalthercwalther
committed
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.
1 parent 551f78e commit d6d02c6

1 file changed

Lines changed: 10 additions & 2 deletions

File tree

‎supervisor/shared/memory.c‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@
3333

3434
#define CIRCUITPY_SUPERVISOR_ALLOC_COUNT (12)
3535

36+
// Using a zero length to mark an unused allocation makes the code a bit shorter (but makes it
37+
// impossible to support zero-length allocations).
38+
#define FREE 0
39+
3640
static supervisor_allocation allocations[CIRCUITPY_SUPERVISOR_ALLOC_COUNT];
3741
// We use uint32_t* to ensure word (4 byte) alignment.
3842
uint32_t* low_address;
@@ -61,19 +65,23 @@ void free_memory(supervisor_allocation* allocation) {
6165
}
6266
if (allocation->ptr == high_address) {
6367
high_address += allocation->length / 4;
68+
allocation->length = FREE;
6469
for (index++; index < CIRCUITPY_SUPERVISOR_ALLOC_COUNT; index++) {
6570
if (allocations[index].ptr != NULL) {
6671
break;
6772
}
6873
high_address += allocations[index].length / 4;
74+
allocations[index].length = FREE;
6975
}
7076
} else if (allocation->ptr + allocation->length / 4 == low_address) {
7177
low_address = allocation->ptr;
78+
allocation->length = FREE;
7279
for (index--; index >= 0; index--) {
7380
if (allocations[index].ptr != NULL) {
7481
break;
7582
}
7683
low_address -= allocations[index].length / 4;
84+
allocations[index].length = FREE;
7785
}
7886
} else {
7987
// 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) {
99107
}
100108

101109
supervisor_allocation* allocate_memory(uint32_t length, bool high) {
102-
if ((high_address - low_address) * 4 < (int32_t) length || length % 4 != 0) {
110+
if (length == 0 || (high_address - low_address) * 4 < (int32_t) length || length % 4 != 0) {
103111
return NULL;
104112
}
105113
uint8_t index = 0;
@@ -109,7 +117,7 @@ supervisor_allocation* allocate_memory(uint32_t length, bool high) {
109117
direction = -1;
110118
}
111119
for (; index < CIRCUITPY_SUPERVISOR_ALLOC_COUNT; index += direction) {
112-
if (allocations[index].ptr == NULL) {
120+
if (allocations[index].length == FREE) {
113121
break;
114122
}
115123
}

0 commit comments

Comments
 (0)