From 2e897c2e74ac2a9a37e97ccea75dbc90b2fce8c1 Mon Sep 17 00:00:00 2001 From: Omar Chebib Date: Mon, 29 Aug 2022 19:46:49 +0800 Subject: [PATCH] TLSF: fix the patch for tlsf_check function in ROM tlsf_check in the patch was not called because the the TLSF functions table in ROM was still pointing to the ROM implementation. --- components/esp_rom/CMakeLists.txt | 11 ++- components/esp_rom/patches/esp_rom_tlsf.c | 97 +++++++++++++++++--- components/heap/multi_heap.c | 4 +- components/heap/multi_heap_poisoning.c | 4 +- components/heap/test/test_corruption_check.c | 10 +- 5 files changed, 103 insertions(+), 23 deletions(-) diff --git a/components/esp_rom/CMakeLists.txt b/components/esp_rom/CMakeLists.txt index 5fe5dc2f50..de4858f7e1 100644 --- a/components/esp_rom/CMakeLists.txt +++ b/components/esp_rom/CMakeLists.txt @@ -21,7 +21,8 @@ else() "patches/esp_rom_regi2c.c" "patches/esp_rom_efuse.c") - if(CONFIG_ESP_ROM_HAS_HEAP_TLSF AND CONFIG_ESP_ROM_TLSF_CHECK_PATCH) + if(CONFIG_HEAP_TLSF_USE_ROM_IMPL AND CONFIG_ESP_ROM_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() @@ -217,6 +218,14 @@ else() # Regular app build endif() if(CONFIG_HEAP_TLSF_USE_ROM_IMPL) + # After registering the component, set the tlsf_set_rom_patches symbol as undefined + # 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) + target_link_libraries(${COMPONENT_LIB} PRIVATE "-u tlsf_set_rom_patches") + endif() + rom_linker_script("heap") endif() endif() diff --git a/components/esp_rom/patches/esp_rom_tlsf.c b/components/esp_rom/patches/esp_rom_tlsf.c index aaf9d2dc17..541b9a0b82 100644 --- a/components/esp_rom/patches/esp_rom_tlsf.c +++ b/components/esp_rom/patches/esp_rom_tlsf.c @@ -14,10 +14,18 @@ #include #include +#include #include "esp_rom_caps.h" #include "rom/tlsf.h" +/*! + * @brief Opaque types for TLSF implementation + */ +typedef void* tlsf_t; +typedef void* pool_t; +typedef void* tlsf_walker; + /* ---------------------------------------------------------------- * Bring certain inline functions, macro and structures from the * tlsf ROM implementation to be able to compile the patch. @@ -166,7 +174,7 @@ void tlsf_poison_check_pfunc_set(void *pfunc) #define tlsf_insist_no_assert(x) { if (!(x)) { status--; } } -int tlsf_check(void* tlsf) +int tlsf_check(tlsf_t tlsf) { int i, j; @@ -211,25 +219,24 @@ int tlsf_check(void* tlsf) mapping_insert(block_size(block), &fli, &sli); tlsf_insist_no_assert(fli == i && sli == j && "block size indexed in wrong list"); - block = block->next_free; - /* 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; + /* 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 */)); + 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; } } } @@ -238,3 +245,63 @@ int tlsf_check(void* tlsf) } #undef tlsf_insist_no_assert + +/* Set up the TLSF ROM patches here */ + +/*! + * @brief Structure to store all the functions of a TLSF implementation. + * The goal of this table is to change any of the address here in order + * to let the ROM code call another function implementation than the one + * in ROM. + */ +struct heap_tlsf_stub_table_t { + tlsf_t (*tlsf_create)(void* mem); + tlsf_t (*tlsf_create_with_pool)(void* mem, size_t bytes); + pool_t (*tlsf_get_pool)(tlsf_t tlsf); + pool_t (*tlsf_add_pool)(tlsf_t tlsf, void* mem, size_t bytes); + void (*tlsf_remove_pool)(tlsf_t tlsf, pool_t pool); + + void* (*tlsf_malloc)(tlsf_t tlsf, size_t size); + void* (*tlsf_memalign)(tlsf_t tlsf, size_t align, size_t size); + void* (*tlsf_memalign_offs)(tlsf_t tlsf, size_t align, size_t size, size_t offset); + void* (*tlsf_realloc)(tlsf_t tlsf, void* ptr, size_t size); + void (*tlsf_free)(tlsf_t tlsf, void* ptr); + + size_t (*tlsf_block_size)(void* ptr); + size_t (*tlsf_size)(void); + size_t (*tlsf_align_size)(void); + size_t (*tlsf_block_size_min)(void); + size_t (*tlsf_block_size_max)(void); + size_t (*tlsf_pool_overhead)(void); + size_t (*tlsf_alloc_overhead)(void); + + void (*tlsf_walk_pool)(pool_t pool, tlsf_walker walker, void* user); + + int (*tlsf_check)(tlsf_t tlsf); + int (*tlsf_check_pool)(pool_t pool); +}; + +/* We need the original table from the ROM */ +extern struct heap_tlsf_stub_table_t* heap_tlsf_table_ptr; + +/* We will copy the ROM table and modify the functions we patch */ +struct heap_tlsf_stub_table_t heap_tlsf_patch_table_ptr; + +/*! + * @brief Setup the TLSF ROM patches. + * This function must be called when setting up the heap. It will put in place the function patched + * from the ROM implementation. + * This function must not be defined as static, as it is marked as "undefined" in the linker flags + * (to force the linker to integrate the functions of this file inside the final binary) + */ +void __attribute__((constructor)) tlsf_set_rom_patches(void) +{ + /* Copy the ROM default table inside our table */ + 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; + + /* 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/multi_heap.c b/components/heap/multi_heap.c index 9431a23771..8481268d55 100644 --- a/components/heap/multi_heap.c +++ b/components/heap/multi_heap.c @@ -81,7 +81,7 @@ typedef struct multi_heap_info { void* heap_data; } heap_t; -#ifdef CONFIG_HEAP_TLSF_USE_ROM_IMPL +#if CONFIG_HEAP_TLSF_USE_ROM_IMPL void _multi_heap_lock(void *lock) { @@ -103,7 +103,7 @@ void multi_heap_in_rom_init(void) multi_heap_os_funcs_init(&multi_heap_os_funcs); } -#else //#ifndef CONFIG_HEAP_TLSF_USE_ROM_IMPL +#else // CONFIG_HEAP_TLSF_USE_ROM_IMPL /* Return true if this block is free. */ static inline bool is_free(const block_header_t *block) diff --git a/components/heap/multi_heap_poisoning.c b/components/heap/multi_heap_poisoning.c index 399d5e30bf..9cb42b95d4 100644 --- a/components/heap/multi_heap_poisoning.c +++ b/components/heap/multi_heap_poisoning.c @@ -359,10 +359,10 @@ multi_heap_handle_t multi_heap_register(void *start, size_t size) memset(start, FREE_FILL_PATTERN, size); } #endif -#ifdef CONFIG_HEAP_TLSF_USE_ROM_IMPL +#if CONFIG_HEAP_TLSF_USE_ROM_IMPL tlsf_poison_fill_pfunc_set(multi_heap_internal_poison_fill_region); tlsf_poison_check_pfunc_set(multi_heap_internal_check_block_poisoning); -#endif +#endif // CONFIG_HEAP_TLSF_USE_ROM_IMPL return multi_heap_register_impl(start, size); } diff --git a/components/heap/test/test_corruption_check.c b/components/heap/test/test_corruption_check.c index 25ca70977d..9c4dd47eb2 100644 --- a/components/heap/test/test_corruption_check.c +++ b/components/heap/test/test_corruption_check.c @@ -47,15 +47,19 @@ TEST_CASE("multi_heap poisoning detection", "[heap]") ptr[i] = CORRUPTED_VALUE; bool is_heap_ok = heap_caps_check_integrity(MALLOC_CAP_8BIT, true); -#ifdef CONFIG_HEAP_POISONING_COMPREHENSIVE + + /* fix the corruption by restoring the original value at ptr + i. + * We need to do this before the ASSERT because they may print a message. + * Using print allocates memory on the heap, so the heap has to be fixed. */ + ptr[i] = original_value; + +#if CONFIG_HEAP_POISONING_COMPREHENSIVE /* check that heap_caps_check_integrity() detects the corruption */ TEST_ASSERT_FALSE(is_heap_ok); #else /* the comprehensive corruption is not checked in the heap_caps_check_integrity() */ TEST_ASSERT_TRUE(is_heap_ok); #endif - /* fix the corruption by restoring the original value at ptr + i */ - ptr[i] = original_value; /* check that heap_caps_check_integrity() stops reporting the corruption */ is_heap_ok = heap_caps_check_integrity(MALLOC_CAP_8BIT, true);