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

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 <jepler@gmail.com>
Signed-off-by: Damien George <damien@micropython.org>
This commit is contained in:
Jeff Epler 2021-08-27 09:04:32 -05:00 committed by Damien George
parent 84071590b3
commit 9f434dd8de
1 changed files with 14 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
py/gc.c
View File

@ -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;