djsundog
/
circuitpython
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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.
This commit is contained in:
Christian Walther 2020-09-28 22:34:02 +02:00
parent 551f78e10b
commit d6d02c67d2
1 changed files with 10 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
supervisor/shared/memory.c
View File

@ -33,6 +33,10 @@
#define CIRCUITPY_SUPERVISOR_ALLOC_COUNT (12) #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]; static supervisor_allocation allocations[CIRCUITPY_SUPERVISOR_ALLOC_COUNT];
// We use uint32_t* to ensure word (4 byte) alignment. // We use uint32_t* to ensure word (4 byte) alignment.
uint32_t* low_address; uint32_t* low_address;
@ -61,19 +65,23 @@ void free_memory(supervisor_allocation* allocation) {
} }
if (allocation->ptr == high_address) { if (allocation->ptr == high_address) {
high_address += allocation->length / 4; high_address += allocation->length / 4;
allocation->length = FREE;
for (index++; index < CIRCUITPY_SUPERVISOR_ALLOC_COUNT; index++) { for (index++; index < CIRCUITPY_SUPERVISOR_ALLOC_COUNT; index++) {
if (allocations[index].ptr != NULL) { if (allocations[index].ptr != NULL) {
break; break;
} }
high_address += allocations[index].length / 4; high_address += allocations[index].length / 4;
allocations[index].length = FREE;
} }
} else if (allocation->ptr + allocation->length / 4 == low_address) { } else if (allocation->ptr + allocation->length / 4 == low_address) {
low_address = allocation->ptr; low_address = allocation->ptr;
allocation->length = FREE;
for (index--; index >= 0; index--) { for (index--; index >= 0; index--) {
if (allocations[index].ptr != NULL) { if (allocations[index].ptr != NULL) {
break; break;
} }
low_address -= allocations[index].length / 4; low_address -= allocations[index].length / 4;
allocations[index].length = FREE;
} }
} else { } else {
// Freed memory isn't in the middle so skip updating bounds. The memory will be added to the // 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) { 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; return NULL;
} }
uint8_t index = 0; uint8_t index = 0;
@ -109,7 +117,7 @@ supervisor_allocation* allocate_memory(uint32_t length, bool high) {
direction = -1; direction = -1;
} }
for (; index < CIRCUITPY_SUPERVISOR_ALLOC_COUNT; index += direction) { for (; index < CIRCUITPY_SUPERVISOR_ALLOC_COUNT; index += direction) {
if (allocations[index].ptr == NULL) { if (allocations[index].length == FREE) {
break; break;
} }
} }