Skip to content

Commit 2199422

Browse files
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 d6da406 commit 2199422

File tree

1 file changed

+14
-1
lines changed

1 file changed

+14
-1
lines changed

supervisor/shared/memory.c

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@
3333

3434
#define CIRCUITPY_SUPERVISOR_ALLOC_COUNT (12)
3535

36+
// Use a value different from zero to mark an unused allocation in order to support zero-length
37+
// allocations. These are probably useless, but rather than trying to guarantee that careless
38+
// callers aren't going to make any and be unprepared for getting NULL, let's just support them.
39+
// That costs us 28 bytes of code though (on a SAMD51).
40+
#define FREE ((uint32_t)-1)
41+
3642
static supervisor_allocation allocations[CIRCUITPY_SUPERVISOR_ALLOC_COUNT];
3743
// We use uint32_t* to ensure word (4 byte) alignment.
3844
uint32_t* low_address;
@@ -41,6 +47,9 @@ uint32_t* high_address;
4147
void memory_init(void) {
4248
low_address = port_heap_get_bottom();
4349
high_address = port_heap_get_top();
50+
for (int8_t i = 0; i < CIRCUITPY_SUPERVISOR_ALLOC_COUNT; i++) {
51+
allocations[i].length = FREE;
52+
}
4453
}
4554

4655
void free_memory(supervisor_allocation* allocation) {
@@ -61,19 +70,23 @@ void free_memory(supervisor_allocation* allocation) {
6170
}
6271
if (allocation->ptr == high_address) {
6372
high_address += allocation->length / 4;
73+
allocation->length = FREE;
6474
for (index++; index < CIRCUITPY_SUPERVISOR_ALLOC_COUNT; index++) {
6575
if (allocations[index].ptr != NULL) {
6676
break;
6777
}
6878
high_address += allocations[index].length / 4;
79+
allocations[index].length = FREE;
6980
}
7081
} else if (allocation->ptr + allocation->length / 4 == low_address) {
7182
low_address = allocation->ptr;
83+
allocation->length = FREE;
7284
for (index--; index >= 0; index--) {
7385
if (allocations[index].ptr != NULL) {
7486
break;
7587
}
7688
low_address -= allocations[index].length / 4;
89+
allocations[index].length = FREE;
7790
}
7891
} else {
7992
// Freed memory isn't in the middle so skip updating bounds. The memory will be added to the
@@ -109,7 +122,7 @@ supervisor_allocation* allocate_memory(uint32_t length, bool high) {
109122
direction = -1;
110123
}
111124
for (; index < CIRCUITPY_SUPERVISOR_ALLOC_COUNT; index += direction) {
112-
if (allocations[index].ptr == NULL) {
125+
if (allocations[index].length == FREE) {
113126
break;
114127
}
115128
}

0 commit comments

Comments
 (0)