From 9f434dd8de3c327c194d531c5496f603d6288d98 Mon Sep 17 00:00:00 2001 From: Jeff Epler Date: Fri, 27 Aug 2021 09:04:32 -0500 Subject: [PATCH] py/gc: Ensure a gap of one byte after the ATB. Prior to this fix the follow crash occurred. With a GC layout of: GC layout: alloc table at 0x3fd80428, length 32001 bytes, 128004 blocks finaliser table at 0x3fd88129, length 16001 bytes, 128008 blocks pool at 0x3fd8bfc0, length 2048064 bytes, 128004 blocks Block 128003 is an AT_HEAD and eventually is passed to gc_mark_subtree. This causes gc_mark_subtree to call ATB_GET_KIND(128004). When block 1 is created with a finaliser, the first byte of the finaliser table becomes 0x2, but ATB_GET_KIND(128004) reads these bits as AT_TAIL, and then gc_mark_subtree references past the end of the heap, which happened to be past the end of PSRAM on the esp32-s2. The fix in this commit is to ensure there is a one-byte gap after the ATB filled permanently with AT_FREE. Fixes issue #7116. See also https://github.com/adafruit/circuitpython/issues/5021 Signed-off-by: Jeff Epler Signed-off-by: Damien George --- py/gc.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/py/gc.c b/py/gc.c index 7ed60fe2cc..99eacfbe6f 100644 --- a/py/gc.c +++ b/py/gc.c @@ -95,6 +95,12 @@ #define BLOCK_FROM_PTR(area, ptr) (((byte *)(ptr) - area->gc_pool_start) / BYTES_PER_BLOCK) #define PTR_FROM_BLOCK(area, block) (((block) * BYTES_PER_BLOCK + (uintptr_t)area->gc_pool_start)) +// After the ATB, there must be a byte filled with AT_FREE so that gc_mark_tree +// cannot erroneously conclude that a block extends past the end of the GC heap +// due to bit patterns in the FTB (or first block, if finalizers are disabled) +// being interpreted as AT_TAIL. +#define ALLOC_TABLE_GAP_BYTE (1) + #if MICROPY_ENABLE_FINALISER // FTB = finaliser table byte // if set, then the corresponding block may have a finaliser @@ -123,16 +129,16 @@ STATIC void gc_setup_area(mp_state_mem_area_t *area, void *start, void *end) { // => T = A * (1 + BLOCKS_PER_ATB / BLOCKS_PER_FTB + BLOCKS_PER_ATB * BYTES_PER_BLOCK) size_t total_byte_len = (byte *)end - (byte *)start; #if MICROPY_ENABLE_FINALISER - area->gc_alloc_table_byte_len = total_byte_len * MP_BITS_PER_BYTE / (MP_BITS_PER_BYTE + MP_BITS_PER_BYTE * BLOCKS_PER_ATB / BLOCKS_PER_FTB + MP_BITS_PER_BYTE * BLOCKS_PER_ATB * BYTES_PER_BLOCK); + area->gc_alloc_table_byte_len = (total_byte_len - ALLOC_TABLE_GAP_BYTE) * MP_BITS_PER_BYTE / (MP_BITS_PER_BYTE + MP_BITS_PER_BYTE * BLOCKS_PER_ATB / BLOCKS_PER_FTB + MP_BITS_PER_BYTE * BLOCKS_PER_ATB * BYTES_PER_BLOCK); #else - area->gc_alloc_table_byte_len = total_byte_len / (1 + MP_BITS_PER_BYTE / 2 * BYTES_PER_BLOCK); + area->gc_alloc_table_byte_len = (total_byte_len - ALLOC_TABLE_GAP_BYTE) / (1 + MP_BITS_PER_BYTE / 2 * BYTES_PER_BLOCK); #endif area->gc_alloc_table_start = (byte *)start; #if MICROPY_ENABLE_FINALISER size_t gc_finaliser_table_byte_len = (area->gc_alloc_table_byte_len * BLOCKS_PER_ATB + BLOCKS_PER_FTB - 1) / BLOCKS_PER_FTB; - area->gc_finaliser_table_start = area->gc_alloc_table_start + area->gc_alloc_table_byte_len; + area->gc_finaliser_table_start = area->gc_alloc_table_start + area->gc_alloc_table_byte_len + ALLOC_TABLE_GAP_BYTE; #endif size_t gc_pool_block_len = area->gc_alloc_table_byte_len * BLOCKS_PER_ATB; @@ -143,12 +149,12 @@ STATIC void gc_setup_area(mp_state_mem_area_t *area, void *start, void *end) { assert(area->gc_pool_start >= area->gc_finaliser_table_start + gc_finaliser_table_byte_len); #endif - // clear ATBs - memset(area->gc_alloc_table_start, 0, area->gc_alloc_table_byte_len); - #if MICROPY_ENABLE_FINALISER - // clear FTBs - memset(area->gc_finaliser_table_start, 0, gc_finaliser_table_byte_len); + // clear ATBs and FTBs + memset(area->gc_alloc_table_start, 0, gc_finaliser_table_byte_len + area->gc_alloc_table_byte_len + ALLOC_TABLE_GAP_BYTE); + #else + // clear ATBs + memset(area->gc_alloc_table_start, 0, area->gc_alloc_table_byte_len + ALLOC_TABLE_GAP_BYTE); #endif area->gc_last_free_atb_index = 0;