Skip to content

Commit

Permalink
py/gc: Ensure a gap of one byte after the ATB.
Browse files Browse the repository at this point in the history
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 micropython#7116.

See also adafruit#5021

Signed-off-by: Jeff Epler <jepler@gmail.com>
Signed-off-by: Damien George <damien@micropython.org>
  • Loading branch information
jepler authored and Gerald Zernatto committed Feb 10, 2023
1 parent 3b88733 commit 20c9ada
Showing 1 changed file with 14 additions and 8 deletions.
22 changes: 14 additions & 8 deletions py/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down

0 comments on commit 20c9ada

Please sign in to comment.