From 93d4b0b38c819bd5ae5c1c131367c9844e87829a Mon Sep 17 00:00:00 2001 From: Guillaume Souchere Date: Fri, 20 Oct 2023 15:23:10 +0200 Subject: [PATCH] fix(heap): Patch tlsf_check_pool in ROM heap The integrity_walker now calls the integrity check hook to control free AND used blocks of memory in the TLSF pool. This integrity walker function is called from tlsf_check_pool. This commit creates a patch of integrity_walker function to update the outdated implementation in the ROM. --- components/esp_rom/CMakeLists.txt | 4 +- .../esp_rom/esp32c2/Kconfig.soc_caps.in | 4 - components/esp_rom/esp32c2/esp_rom_caps.h | 1 - .../esp_rom/esp32c6/Kconfig.soc_caps.in | 4 + components/esp_rom/esp32c6/esp_rom_caps.h | 1 + .../esp_rom/esp32c6/ld/esp32c6.rom.heap.ld | 1 - .../esp_rom/esp32h2/Kconfig.soc_caps.in | 4 + components/esp_rom/esp32h2/esp_rom_caps.h | 1 + .../esp_rom/esp32h2/ld/esp32h2.rom.heap.ld | 1 - components/esp_rom/linker.lf | 2 +- components/esp_rom/patches/esp_rom_tlsf.c | 195 ++++-------------- components/heap/Kconfig | 10 + .../heap_tests/main/test_corruption_check.c | 4 - 13 files changed, 67 insertions(+), 165 deletions(-) diff --git a/components/esp_rom/CMakeLists.txt b/components/esp_rom/CMakeLists.txt index 02ec89d331..0701edc6c8 100644 --- a/components/esp_rom/CMakeLists.txt +++ b/components/esp_rom/CMakeLists.txt @@ -30,7 +30,7 @@ else() endif() endif() - if(CONFIG_HEAP_TLSF_USE_ROM_IMPL AND CONFIG_ESP_ROM_TLSF_CHECK_PATCH) + if(CONFIG_HEAP_TLSF_USE_ROM_IMPL AND (CONFIG_ESP_ROM_TLSF_CHECK_PATCH OR CONFIG_HEAP_TLSF_CHECK_PATCH)) # This file shall be included in the build if TLSF in ROM is activated list(APPEND sources "patches/esp_rom_tlsf.c") endif() @@ -301,7 +301,7 @@ else() # Regular app build # to force the linker to integrate the whole `esp_rom_tlsf.c` object file inside the # final binary. This is necessary because tlsf_set_rom_patches is a constructor, thus, # there as no explicit reference/call to it in IDF. - if(CONFIG_ESP_ROM_TLSF_CHECK_PATCH) + if((CONFIG_ESP_ROM_TLSF_CHECK_PATCH OR CONFIG_HEAP_TLSF_CHECK_PATCH)) target_link_libraries(${COMPONENT_LIB} PRIVATE "-u tlsf_set_rom_patches") endif() diff --git a/components/esp_rom/esp32c2/Kconfig.soc_caps.in b/components/esp_rom/esp32c2/Kconfig.soc_caps.in index 752cf25be6..b2d3381e8c 100644 --- a/components/esp_rom/esp32c2/Kconfig.soc_caps.in +++ b/components/esp_rom/esp32c2/Kconfig.soc_caps.in @@ -39,10 +39,6 @@ config ESP_ROM_HAS_HEAP_TLSF bool default y -config ESP_ROM_TLSF_CHECK_PATCH - bool - default y - config ESP_ROM_HAS_LAYOUT_TABLE bool default y diff --git a/components/esp_rom/esp32c2/esp_rom_caps.h b/components/esp_rom/esp32c2/esp_rom_caps.h index 8eacb1616c..cc3fdc5180 100644 --- a/components/esp_rom/esp32c2/esp_rom_caps.h +++ b/components/esp_rom/esp32c2/esp_rom_caps.h @@ -15,7 +15,6 @@ #define ESP_ROM_HAS_HAL_WDT (1) // ROM has the implementation of Watchdog HAL driver #define ESP_ROM_HAS_HAL_SYSTIMER (1) // ROM has the implementation of Systimer HAL driver #define ESP_ROM_HAS_HEAP_TLSF (1) // ROM has the implementation of the tlsf and multi-heap library -#define ESP_ROM_TLSF_CHECK_PATCH (1) // ROM does not contain the patch of tlsf_check() #define ESP_ROM_HAS_LAYOUT_TABLE (1) // ROM has the layout table #define ESP_ROM_HAS_SPI_FLASH (1) // ROM has the implementation of SPI Flash driver #define ESP_ROM_HAS_NEWLIB_NANO_FORMAT (1) // ROM has the newlib nano version of formatting functions diff --git a/components/esp_rom/esp32c6/Kconfig.soc_caps.in b/components/esp_rom/esp32c6/Kconfig.soc_caps.in index 9e23f349b7..df5e153a0d 100644 --- a/components/esp_rom/esp32c6/Kconfig.soc_caps.in +++ b/components/esp_rom/esp32c6/Kconfig.soc_caps.in @@ -47,6 +47,10 @@ config ESP_ROM_HAS_HEAP_TLSF bool default y +config ESP_ROM_TLSF_CHECK_PATCH + bool + default y + config ESP_ROM_HAS_LAYOUT_TABLE bool default y diff --git a/components/esp_rom/esp32c6/esp_rom_caps.h b/components/esp_rom/esp32c6/esp_rom_caps.h index 1ec2d87239..1eeab3bd3e 100644 --- a/components/esp_rom/esp32c6/esp_rom_caps.h +++ b/components/esp_rom/esp32c6/esp_rom_caps.h @@ -17,6 +17,7 @@ #define ESP_ROM_HAS_HAL_WDT (1) // ROM has the implementation of Watchdog HAL driver #define ESP_ROM_HAS_HAL_SYSTIMER (1) // ROM has the implementation of Systimer HAL driver #define ESP_ROM_HAS_HEAP_TLSF (1) // ROM has the implementation of the tlsf and multi-heap library +#define ESP_ROM_TLSF_CHECK_PATCH (1) // ROM does not contain the patch of tlsf_check_pool() #define ESP_ROM_HAS_LAYOUT_TABLE (1) // ROM has the layout table #define ESP_ROM_HAS_SPI_FLASH (1) // ROM has the implementation of SPI Flash driver #define ESP_ROM_HAS_REGI2C_BUG (1) // ROM has the regi2c bug diff --git a/components/esp_rom/esp32c6/ld/esp32c6.rom.heap.ld b/components/esp_rom/esp32c6/ld/esp32c6.rom.heap.ld index 1e95388383..f09bb74f08 100644 --- a/components/esp_rom/esp32c6/ld/esp32c6.rom.heap.ld +++ b/components/esp_rom/esp32c6/ld/esp32c6.rom.heap.ld @@ -37,7 +37,6 @@ tlsf_pool_overhead = 0x40000438; tlsf_alloc_overhead = 0x4000043c; tlsf_walk_pool = 0x40000440; tlsf_check = 0x40000444; -tlsf_check_pool = 0x40000448; tlsf_poison_fill_pfunc_set = 0x4000044c; tlsf_poison_check_pfunc_set = 0x40000450; multi_heap_get_block_address_impl = 0x40000454; diff --git a/components/esp_rom/esp32h2/Kconfig.soc_caps.in b/components/esp_rom/esp32h2/Kconfig.soc_caps.in index a5b8f87971..9e95fbf265 100644 --- a/components/esp_rom/esp32h2/Kconfig.soc_caps.in +++ b/components/esp_rom/esp32h2/Kconfig.soc_caps.in @@ -39,6 +39,10 @@ config ESP_ROM_HAS_HEAP_TLSF bool default y +config ESP_ROM_TLSF_CHECK_PATCH + bool + default y + config ESP_ROM_HAS_LAYOUT_TABLE bool default y diff --git a/components/esp_rom/esp32h2/esp_rom_caps.h b/components/esp_rom/esp32h2/esp_rom_caps.h index 321ab91b8e..375f91137d 100644 --- a/components/esp_rom/esp32h2/esp_rom_caps.h +++ b/components/esp_rom/esp32h2/esp_rom_caps.h @@ -15,6 +15,7 @@ #define ESP_ROM_HAS_HAL_WDT (1) // ROM has the implementation of Watchdog HAL driver #define ESP_ROM_HAS_HAL_SYSTIMER (1) // ROM has the implementation of Systimer HAL driver #define ESP_ROM_HAS_HEAP_TLSF (1) // ROM has the implementation of the tlsf and multi-heap library +#define ESP_ROM_TLSF_CHECK_PATCH (1) // ROM does not contain the patch of tlsf_check_pool() #define ESP_ROM_HAS_LAYOUT_TABLE (1) // ROM has the layout table #define ESP_ROM_HAS_SPI_FLASH (1) // ROM has the implementation of SPI Flash driver #define ESP_ROM_WITHOUT_REGI2C (1) // ROM has no regi2c APIs diff --git a/components/esp_rom/esp32h2/ld/esp32h2.rom.heap.ld b/components/esp_rom/esp32h2/ld/esp32h2.rom.heap.ld index 3e17e87eba..0c176cbbb4 100644 --- a/components/esp_rom/esp32h2/ld/esp32h2.rom.heap.ld +++ b/components/esp_rom/esp32h2/ld/esp32h2.rom.heap.ld @@ -37,7 +37,6 @@ tlsf_pool_overhead = 0x40000430; tlsf_alloc_overhead = 0x40000434; tlsf_walk_pool = 0x40000438; tlsf_check = 0x4000043c; -tlsf_check_pool = 0x40000440; tlsf_poison_fill_pfunc_set = 0x40000444; tlsf_poison_check_pfunc_set = 0x40000448; multi_heap_get_block_address_impl = 0x4000044c; diff --git a/components/esp_rom/linker.lf b/components/esp_rom/linker.lf index 99ddf6bdd4..bc708c5697 100644 --- a/components/esp_rom/linker.lf +++ b/components/esp_rom/linker.lf @@ -6,7 +6,7 @@ entries: esp_rom_cache_esp32s2_esp32s3 (noflash) if ESP_ROM_HAS_CACHE_WRITEBACK_BUG = y: esp_rom_cache_writeback_esp32s3 (noflash) - if HEAP_TLSF_USE_ROM_IMPL = y && ESP_ROM_TLSF_CHECK_PATCH = y: + if HEAP_TLSF_USE_ROM_IMPL = y && (ESP_ROM_TLSF_CHECK_PATCH = y || HEAP_TLSF_CHECK_PATCH = y): esp_rom_tlsf (noflash) if SOC_SYSTIMER_SUPPORTED = y: esp_rom_systimer (noflash) diff --git a/components/esp_rom/patches/esp_rom_tlsf.c b/components/esp_rom/patches/esp_rom_tlsf.c index 088248acce..06da058328 100644 --- a/components/esp_rom/patches/esp_rom_tlsf.c +++ b/components/esp_rom/patches/esp_rom_tlsf.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -33,46 +33,10 @@ typedef void* tlsf_walker; #define tlsf_cast(t, exp) ((t) (exp)) -enum tlsf_config { - /* log2 of number of linear subdivisions of block sizes. Larger - ** values require more memory in the control structure. Values of - ** 4 or 5 are typical. - */ - SL_INDEX_COUNT_LOG2 = 5, - - /* All allocation sizes and addresses are aligned to 4 bytes. */ - ALIGN_SIZE_LOG2 = 2, - ALIGN_SIZE = (1 << ALIGN_SIZE_LOG2), - -/* - ** We support allocations of sizes up to (1 << FL_INDEX_MAX) bits. - ** However, because we linearly subdivide the second-level lists, and - ** our minimum size granularity is 4 bytes, it doesn't make sense to - ** create first-level lists for sizes smaller than SL_INDEX_COUNT * 4, - ** or (1 << (SL_INDEX_COUNT_LOG2 + 2)) bytes, as there we will be - ** trying to split size ranges into more slots than we have available. - ** Instead, we calculate the minimum threshold size, and place all - ** blocks below that size into the 0th first-level list. - */ - - /* Fix the value of FL_INDEX_MAX to match the value that is defined - * in the ROM implementation. */ - FL_INDEX_MAX = 18, //Each pool can have up 256KB - - SL_INDEX_COUNT = (1 << SL_INDEX_COUNT_LOG2), - FL_INDEX_SHIFT = (SL_INDEX_COUNT_LOG2 + ALIGN_SIZE_LOG2), - FL_INDEX_COUNT = (FL_INDEX_MAX - FL_INDEX_SHIFT + 1), - - SMALL_BLOCK_SIZE = (1 << FL_INDEX_SHIFT), -}; - #define block_header_free_bit (1 << 0) #define block_header_prev_free_bit (1 << 1) #define block_header_overhead (sizeof(size_t)) #define block_start_offset (offsetof(block_header_t, size) + sizeof(size_t)) -#define block_size_min (sizeof(block_header_t) - sizeof(block_header_t*)) - -typedef ptrdiff_t tlsfptr_t; typedef struct block_header_t { @@ -87,26 +51,6 @@ typedef struct block_header_t struct block_header_t* prev_free; } block_header_t; -/* The TLSF control structure. */ -typedef struct control_t -{ - /* Empty lists point at this block to indicate they are free. */ - block_header_t block_null; - - /* Bitmaps for free lists. */ - unsigned int fl_bitmap; - unsigned int sl_bitmap[FL_INDEX_COUNT]; - - /* Head of free lists. */ - block_header_t* blocks[FL_INDEX_COUNT][SL_INDEX_COUNT]; -} control_t; - -static inline __attribute__((__always_inline__)) int tlsf_fls(unsigned int word) -{ - const int bit = word ? 32 - __builtin_clz(word) : 0; - return bit - 1; -} - static inline __attribute__((__always_inline__)) size_t block_size(const block_header_t* block) { return block->size & ~(block_header_free_bit | block_header_prev_free_bit); @@ -122,41 +66,10 @@ static inline __attribute__((__always_inline__)) int block_is_prev_free(const bl return tlsf_cast(int, block->size & block_header_prev_free_bit); } -static inline __attribute__((__always_inline__)) block_header_t* offset_to_block(const void* ptr, size_t size) +static inline __attribute__((always_inline)) block_header_t* block_from_ptr(const void* ptr) { - return tlsf_cast(block_header_t*, tlsf_cast(tlsfptr_t, ptr) + size); -} - -static inline __attribute__((__always_inline__)) void* block_to_ptr(const block_header_t* block) -{ - return tlsf_cast(void*, - tlsf_cast(unsigned char*, block) + block_start_offset); -} - -static inline __attribute__((__always_inline__)) block_header_t* block_next(const block_header_t* block) -{ - block_header_t* next = offset_to_block(block_to_ptr(block), - block_size(block) - block_header_overhead); - return next; -} - -static inline __attribute__((__always_inline__)) void mapping_insert(size_t size, int* fli, int* sli) -{ - int fl, sl; - if (size < SMALL_BLOCK_SIZE) - { - /* Store small blocks in first list. */ - fl = 0; - sl = tlsf_cast(int, size) >> 2; - } - else - { - fl = tlsf_fls(size); - sl = tlsf_cast(int, size >> (fl - SL_INDEX_COUNT_LOG2)) ^ (1 << SL_INDEX_COUNT_LOG2); - fl -= (FL_INDEX_SHIFT - 1); - } - *fli = fl; - *sli = sl; + return tlsf_cast(block_header_t*, + tlsf_cast(unsigned char*, ptr) - block_start_offset); } /* ---------------------------------------------------------------- @@ -173,74 +86,54 @@ void tlsf_poison_check_pfunc_set(poison_check_pfunc_t pfunc) #define tlsf_insist_no_assert(x) { if (!(x)) { status--; } } -int tlsf_check(tlsf_t tlsf) +typedef struct integrity_t { - int i, j; + int prev_status; + int status; +} integrity_t; - control_t* control = tlsf_cast(control_t*, tlsf); - int status = 0; +static void integrity_walker(void* ptr, size_t size, int used, void* user) +{ + block_header_t* block = block_from_ptr(ptr); + integrity_t* integ = tlsf_cast(integrity_t*, user); + const int this_prev_status = block_is_prev_free(block) ? 1 : 0; + const int this_status = block_is_free(block) ? 1 : 0; + const size_t this_block_size = block_size(block); - /* Check that the free lists and bitmaps are accurate. */ - for (i = 0; i < FL_INDEX_COUNT; ++i) - { - for (j = 0; j < SL_INDEX_COUNT; ++j) - { - const int fl_map = control->fl_bitmap & (1 << i); - const int sl_list = control->sl_bitmap[i]; - const int sl_map = sl_list & (1 << j); - const block_header_t* block = control->blocks[i][j]; + int status = 0; + tlsf_insist_no_assert(integ->prev_status == this_prev_status && "prev status incorrect"); + tlsf_insist_no_assert(size == this_block_size && "block size incorrect"); - /* Check that first- and second-level lists agree. */ - if (!fl_map) - { - tlsf_insist_no_assert(!sl_map && "second-level map must be null"); - } + if (s_poison_check_region != NULL) + { + /* block_size(block) returns the size of the usable memory when the block is allocated. + * As the block under test is free, we need to subtract to the block size the next_free + * and prev_free fields of the block header as they are not a part of the usable memory + * when the block is free. In addition, we also need to subtract the size of prev_phys_block + * as this field is in fact part of the current free block and not part of the next (allocated) + * block. Check the comments in block_split function for more details. + */ + const size_t actual_free_block_size = used ? this_block_size : + this_block_size - offsetof(block_header_t, next_free)- block_header_overhead; - if (!sl_map) - { - tlsf_insist_no_assert(block == &control->block_null && "block list must be null"); - continue; - } + void* ptr_block = used ? (void*)block + block_start_offset : + (void*)block + sizeof(block_header_t); - /* Check that there is at least one free block. */ - tlsf_insist_no_assert(sl_list && "no free blocks in second-level map"); - tlsf_insist_no_assert(block != &control->block_null && "block should not be null"); + tlsf_insist_no_assert(s_poison_check_region(ptr_block, actual_free_block_size, !used, true)); + } - while (block != &control->block_null) - { - int fli, sli; - const bool is_block_free = block_is_free(block); - tlsf_insist_no_assert(is_block_free && "block should be free"); - tlsf_insist_no_assert(!block_is_prev_free(block) && "blocks should have coalesced"); - tlsf_insist_no_assert(!block_is_free(block_next(block)) && "blocks should have coalesced"); - tlsf_insist_no_assert(block_is_prev_free(block_next(block)) && "block should be free"); - tlsf_insist_no_assert(block_size(block) >= block_size_min && "block not minimum size"); + integ->prev_status = this_status; + integ->status += status; +} - mapping_insert(block_size(block), &fli, &sli); - tlsf_insist_no_assert(fli == i && sli == j && "block size indexed in wrong list"); +extern void tlsf_walk_pool(pool_t pool, tlsf_walker walker, void* user); +int tlsf_check_pool(pool_t pool) +{ + /* Check that the blocks are physically correct. */ + integrity_t integ = { 0, 0 }; + tlsf_walk_pool(pool, integrity_walker, &integ); - /* block_size(block) returns the size of the usable memory when the block is allocated. - * As the block under test is free, we need to subtract to the block size the next_free - * and prev_free fields of the block header as they are not a part of the usable memory - * when the block is free. In addition, we also need to subtract the size of prev_phys_block - * as this field is in fact part of the current free block and not part of the next (allocated) - * block. Check the comments in block_split function for more details. - */ - const size_t actual_free_block_size = block_size(block) - - offsetof(block_header_t, next_free) - - block_header_overhead; - - if (s_poison_check_region != NULL) { - tlsf_insist_no_assert(s_poison_check_region((char *)block + sizeof(block_header_t), - actual_free_block_size, is_block_free, true /* print errors */)); - } - - block = block->next_free; - } - } - } - - return status; + return integ.status; } #undef tlsf_insist_no_assert @@ -299,7 +192,7 @@ void __attribute__((constructor)) tlsf_set_rom_patches(void) memcpy(&heap_tlsf_patch_table_ptr, heap_tlsf_table_ptr, sizeof(struct heap_tlsf_stub_table_t)); /* Set the patched function here */ - heap_tlsf_patch_table_ptr.tlsf_check = tlsf_check; + heap_tlsf_patch_table_ptr.tlsf_check_pool = tlsf_check_pool; /* Set our table as the one to use in the ROM code */ heap_tlsf_table_ptr = &heap_tlsf_patch_table_ptr; diff --git a/components/heap/Kconfig b/components/heap/Kconfig index 8de0f77519..48382f1206 100644 --- a/components/heap/Kconfig +++ b/components/heap/Kconfig @@ -128,4 +128,14 @@ menu "Heap memory debugging" Note that it is only safe to enable this configuration if no functions from esp_heap_caps.h or esp_heap_trace.h are called from ISR. + + config HEAP_TLSF_CHECK_PATCH + bool "Patch the tlsf_check_pool() for ROM HEAP TLSF implementation" + depends on IDF_TARGET_ESP32C2 && ESP32C2_REV_MIN_FULL < 200 + default y + help + ROM does not contain the patch of tlsf_check_pool() allowing perform + the integrity checking on used blocks. The patch to allow such check + needs to be applied. + endmenu diff --git a/components/heap/test_apps/heap_tests/main/test_corruption_check.c b/components/heap/test_apps/heap_tests/main/test_corruption_check.c index ec88930c7a..2ee7aa21e3 100644 --- a/components/heap/test_apps/heap_tests/main/test_corruption_check.c +++ b/components/heap/test_apps/heap_tests/main/test_corruption_check.c @@ -70,8 +70,6 @@ TEST_CASE("multi_heap poisoning detection", "[heap]") } } -#if !defined(CONFIG_HEAP_TLSF_USE_ROM_IMPL) - #ifdef CONFIG_HEAP_TASK_TRACKING #define HEAD_CANARY_OFFSET 3 // head canary | task tracking | allocated size #else @@ -117,6 +115,4 @@ TEST_CASE("canary corruption in light or comprehensive poisoning mode", "[heap]" ptr[TAIL_CANARY_OFFSET] = canary; heap_caps_free(ptr); } - -#endif // !CONFIG_HEAP_TLSF_USE_ROM_IMPL #endif // CONFIG_HEAP_POISONING_LIGHT && CONFIG_HEAP_LIGHT_POISONING