diff --git a/components/esp_rom/CMakeLists.txt b/components/esp_rom/CMakeLists.txt index 5938c3a823b..5fe5dc2f508 100644 --- a/components/esp_rom/CMakeLists.txt +++ b/components/esp_rom/CMakeLists.txt @@ -20,6 +20,11 @@ else() "patches/esp_rom_spiflash.c" "patches/esp_rom_regi2c.c" "patches/esp_rom_efuse.c") + + if(CONFIG_ESP_ROM_HAS_HEAP_TLSF AND CONFIG_ESP_ROM_TLSF_CHECK_PATCH) + list(APPEND sources "patches/esp_rom_tlsf.c") + endif() + list(APPEND private_required_comp soc hal) endif() diff --git a/components/esp_rom/esp32c2/Kconfig.soc_caps.in b/components/esp_rom/esp32c2/Kconfig.soc_caps.in index b6e13da8229..d676d6ce036 100644 --- a/components/esp_rom/esp32c2/Kconfig.soc_caps.in +++ b/components/esp_rom/esp32c2/Kconfig.soc_caps.in @@ -38,3 +38,7 @@ config ESP_ROM_HAS_HAL_SYSTIMER config ESP_ROM_HAS_HEAP_TLSF bool default y + +config ESP_ROM_TLSF_CHECK_PATCH + bool + default y diff --git a/components/esp_rom/esp32c2/esp_rom_caps.h b/components/esp_rom/esp32c2/esp_rom_caps.h index eee69daaaaf..60706ac706d 100644 --- a/components/esp_rom/esp32c2/esp_rom_caps.h +++ b/components/esp_rom/esp32c2/esp_rom_caps.h @@ -15,3 +15,4 @@ #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() diff --git a/components/esp_rom/include/esp32c2/rom/tlsf.h b/components/esp_rom/include/esp32c2/rom/tlsf.h index 516822190ee..347ef5eddde 100644 --- a/components/esp_rom/include/esp32c2/rom/tlsf.h +++ b/components/esp_rom/include/esp32c2/rom/tlsf.h @@ -18,6 +18,15 @@ extern "C" { */ void tlsf_poison_fill_pfunc_set(void *pfunc); +/*! + * @brief Set the function to call for checking memory region when + * poisoning is configured. + * + * @param pfunc The callback function to trigger for checking + * the content of a memory region. + */ +void tlsf_poison_check_pfunc_set(void *pfunc); + #ifdef __cplusplus } #endif diff --git a/components/esp_rom/patches/esp_rom_tlsf.c b/components/esp_rom/patches/esp_rom_tlsf.c new file mode 100644 index 00000000000..aaf9d2dc179 --- /dev/null +++ b/components/esp_rom/patches/esp_rom_tlsf.c @@ -0,0 +1,240 @@ +/* + * SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ + +/* + * This file is a patch for the tlsf implementation stored in ROM + * - tlsf_check() now implements a call to a hook giving the user the possibility + * to implement specific checks on the memory of every free blocks. + * - The function tlsf_poison_check_pfunc_set() was added to allow the user to + * register the hook function called in tlsf_check(). + */ + +#include +#include + +#include "esp_rom_caps.h" +#include "rom/tlsf.h" + +/* ---------------------------------------------------------------- + * Bring certain inline functions, macro and structures from the + * tlsf ROM implementation to be able to compile the patch. + * ---------------------------------------------------------------- */ + +#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 +{ + /* Points to the previous physical block. */ + struct block_header_t* prev_phys_block; + + /* The size of this block, excluding the block header. */ + size_t size; + + /* Next and previous free blocks. */ + struct block_header_t* next_free; + 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); +} + +static inline __attribute__((__always_inline__)) int block_is_free(const block_header_t* block) +{ + return tlsf_cast(int, block->size & block_header_free_bit); +} + +static inline __attribute__((__always_inline__)) int block_is_prev_free(const block_header_t* block) +{ + 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) +{ + 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; +} + +/* ---------------------------------------------------------------- + * End of the environment necessary to compile and link the patch + * defined below + * ---------------------------------------------------------------- */ + +typedef bool (*poison_check_pfunc_t)(void *start, size_t size, bool is_free, bool print_errors); +static poison_check_pfunc_t s_poison_check_region = NULL; + +void tlsf_poison_check_pfunc_set(void *pfunc) +{ + s_poison_check_region = (poison_check_pfunc_t)pfunc; +} + +#define tlsf_insist_no_assert(x) { if (!(x)) { status--; } } + +int tlsf_check(void* tlsf) +{ + int i, j; + + control_t* control = tlsf_cast(control_t*, tlsf); + int status = 0; + + /* 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]; + + /* Check that first- and second-level lists agree. */ + if (!fl_map) + { + tlsf_insist_no_assert(!sl_map && "second-level map must be null"); + } + + if (!sl_map) + { + tlsf_insist_no_assert(block == &control->block_null && "block list must be null"); + continue; + } + + /* 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"); + + 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"); + + 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; + + 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 */)); + } + + + } + } + } + + return status; +} + +#undef tlsf_insist_no_assert diff --git a/components/heap/include/multi_heap.h b/components/heap/include/multi_heap.h index 6b0990c72dc..e2aa6672d74 100644 --- a/components/heap/include/multi_heap.h +++ b/components/heap/include/multi_heap.h @@ -125,6 +125,8 @@ void multi_heap_dump(multi_heap_handle_t heap); * can be optionally printed to stderr. Print behaviour can be overridden at compile time by defining * MULTI_CHECK_FAIL_PRINTF in multi_heap_platform.h. * + * @note This function is not thread-safe as it sets a global variable with the value of print_errors. + * * @param heap Handle to a registered heap. * @param print_errors If true, errors will be printed to stderr. * @return true if heap is valid, false otherwise. diff --git a/components/heap/multi_heap.c b/components/heap/multi_heap.c index 1a0871f4571..9431a237716 100644 --- a/components/heap/multi_heap.c +++ b/components/heap/multi_heap.c @@ -309,13 +309,46 @@ void *multi_heap_aligned_alloc_impl(multi_heap_handle_t heap, size_t size, size_ return multi_heap_aligned_alloc_impl_offs(heap, size, alignment, 0); } +#ifdef MULTI_HEAP_POISONING +/*! + * @brief Global definition of print_errors set in multi_heap_check() when + * MULTI_HEAP_POISONING is active. Allows the transfer of the value to + * multi_heap_poisoning.c without having to propagate it to the tlsf submodule + * and back. + */ +static bool g_print_errors = false; + +/*! + * @brief Definition of the weak function declared in TLSF repository. + * The call of this function execute a check for block poisoning on the memory + * chunk passed as parameter. + * + * @param start: pointer to the start of the memory region to check for corruption + * @param size: size of the memory region to check for corruption + * @param is_free: indicate if the pattern to use the fill the region should be + * an after free or after allocation pattern. + * + * @return bool: true if the the memory is not corrupted, false if the memory if corrupted. + */ +bool tlsf_check_hook(void *start, size_t size, bool is_free) +{ + return multi_heap_internal_check_block_poisoning(start, size, is_free, g_print_errors); +} +#endif // MULTI_HEAP_POISONING + bool multi_heap_check(multi_heap_handle_t heap, bool print_errors) { - (void)print_errors; bool valid = true; assert(heap != NULL); multi_heap_internal_lock(heap); + +#ifdef MULTI_HEAP_POISONING + g_print_errors = print_errors; +#else + (void) print_errors; +#endif + if(tlsf_check(heap->heap_data)) { valid = false; } diff --git a/components/heap/multi_heap_poisoning.c b/components/heap/multi_heap_poisoning.c index f59ef6766d5..399d5e30bfb 100644 --- a/components/heap/multi_heap_poisoning.c +++ b/components/heap/multi_heap_poisoning.c @@ -25,8 +25,8 @@ #include "tlsf.h" #else /* Header containing the declaration of tlsf_poison_fill_pfunc_set() - * used to register multi_heap_internal_poison_fill_region() as a - * callback to fill memory region with given patterns in the heap + * and tlsf_poison_check_pfunc_set() used to register callbacks to + * fill and check memory region with given patterns in the heap * components. */ #include "rom/tlsf.h" @@ -361,6 +361,7 @@ multi_heap_handle_t multi_heap_register(void *start, size_t size) #endif #ifdef 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 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 new file mode 100644 index 00000000000..25ca70977d3 --- /dev/null +++ b/components/heap/test/test_corruption_check.c @@ -0,0 +1,64 @@ +/* + * SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Unlicense OR CC0-1.0 + */ +#include "unity.h" +#include "stdio.h" + +#include "esp_heap_caps.h" + +/* executing multi_heap_internal_check_block_poisoning() + * takes longer on external RAM and therefore the timeout + * in the test of 30 seconds is exceeded. Execute the test + * on a smaller memory chunk + */ +#ifdef CONFIG_SPIRAM +const size_t MALLOC_SIZE = 16; +#else +const size_t MALLOC_SIZE = 64; +#endif +const uint8_t CORRUPTED_VALUE = 0xaa; + +/* This test will corrupt the memory of a free block in the heap and check + * that in the case of comprehensive poisoning the heap corruption is detected + * by heap_caps_check_integrity(). For light poisoning and no poisoning, the test will + * check that heap_caps_check_integrity() does not report the corruption. + */ +TEST_CASE("multi_heap poisoning detection", "[heap]") +{ + /* malloc some memory to get a pointer */ + uint8_t *ptr = heap_caps_malloc(MALLOC_SIZE, MALLOC_CAP_8BIT); + + /* free the memory to free the block but keep the pointer in mind */ + heap_caps_free(ptr); + + /* variable used in the test */ + uint8_t original_value = 0x00; + + for (size_t i = 0; i < MALLOC_SIZE; i++) + { + /* keep the good value in store in order to check that when we set the byte back + * to its original value, heap_caps_check_integrity() no longer returns the + * heap corruption. */ + original_value = ptr[i]; + + /* set corrupted value in the free memory*/ + ptr[i] = CORRUPTED_VALUE; + + bool is_heap_ok = heap_caps_check_integrity(MALLOC_CAP_8BIT, true); +#ifdef 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); + TEST_ASSERT_TRUE(is_heap_ok); + } +} diff --git a/components/heap/test_multi_heap_host/Makefile b/components/heap/test_multi_heap_host/Makefile index 6c10f8035ae..7d97cbcc6fd 100644 --- a/components/heap/test_multi_heap_host/Makefile +++ b/components/heap/test_multi_heap_host/Makefile @@ -17,7 +17,7 @@ INCLUDE_FLAGS = -I../include -I../../../tools/catch -I../tlsf GCOV ?= gcov -CPPFLAGS += $(INCLUDE_FLAGS) -D CONFIG_LOG_DEFAULT_LEVEL -g -fstack-protector-all -m32 -DCONFIG_HEAP_POISONING_COMPREHENSIVE +CPPFLAGS += $(INCLUDE_FLAGS) -D CONFIG_LOG_DEFAULT_LEVEL -g -fstack-protector-all -m32 CFLAGS += -Wall -Werror -fprofile-arcs -ftest-coverage CXXFLAGS += -std=c++11 -Wall -Werror -fprofile-arcs -ftest-coverage LDFLAGS += -lstdc++ -fprofile-arcs -ftest-coverage -m32 diff --git a/components/heap/test_multi_heap_host/test_multi_heap.cpp b/components/heap/test_multi_heap_host/test_multi_heap.cpp index e85cceddc6b..c3cac1cad17 100644 --- a/components/heap/test_multi_heap_host/test_multi_heap.cpp +++ b/components/heap/test_multi_heap_host/test_multi_heap.cpp @@ -2,6 +2,8 @@ #include "multi_heap.h" #include "../multi_heap_config.h" +#include "../tlsf/tlsf_common.h" +#include "../tlsf/tlsf_block_functions.h" #include #include @@ -523,3 +525,65 @@ TEST_CASE("multi_heap allocation overhead", "[multi_heap]") multi_heap_free(heap, x); } + +/* This test will corrupt the memory of a free block in the heap and check + * that in the case of comprehensive poisoning the heap corruption is detected + * by multi_heap_check(). For light poisoning and no poisoning, the test will + * check that multi_heap_check() does not report the corruption. + */ +TEST_CASE("multi_heap poisoning detection", "[multi_heap]") +{ + const size_t HEAP_SIZE = 4 * 1024; + + /* define heap related data */ + uint8_t heap_mem[HEAP_SIZE]; + memset(heap_mem, 0x00, HEAP_SIZE); + + /* register the heap memory. One free block only will be available */ + multi_heap_handle_t heap = multi_heap_register(heap_mem, HEAP_SIZE); + + /* offset in memory at which to find the first free memory byte */ + const size_t free_memory_offset = sizeof(multi_heap_info_t) + sizeof(control_t) + block_header_overhead; + + /* block header of the free block under test in the heap () */ + const block_header_t* block = (block_header_t*)(heap_mem + free_memory_offset - sizeof(block_header_t)); + + /* actual number of bytes potentially filled with the free pattern in the free block under test */ + const size_t effective_free_size = block_size(block) - block_header_overhead - offsetof(block_header_t, next_free); + + /* variable used in the test */ + size_t affected_byte = 0x00; + uint8_t original_value = 0x00; + uint8_t corrupted_value = 0x00; + + /* repeat the corruption a few times to cover more of the free memory */ + for (size_t i = 0; i < effective_free_size; i++) + { + /* corrupt random bytes in the heap (it needs to be bytes from free memory in + * order to check that the comprehensive poisoning is doing its job) */ + affected_byte = free_memory_offset + i; + corrupted_value = (rand() % UINT8_MAX) | 1; + + /* keep the good value in store in order to check that when we set the byte back + * to its original value, multi_heap_check() no longer returns the heap corruption. */ + original_value = heap_mem[affected_byte]; + + /* make sure we are not replacing the original value with the same value */ + heap_mem[affected_byte] ^= corrupted_value; + + bool is_heap_ok = multi_heap_check(heap, true); +#ifdef CONFIG_HEAP_POISONING_COMPREHENSIVE + /* check that multi_heap_check() detects the corruption */ + REQUIRE(is_heap_ok == false); +#else + /* the comprehensive corruption is not checked in the multi_heap_check() */ + REQUIRE(is_heap_ok == true); +#endif + /* fix the corruption */ + heap_mem[affected_byte] = original_value; + + /* check that multi_heap_check() stops reporting the corruption */ + is_heap_ok = multi_heap_check(heap, true); + REQUIRE(is_heap_ok == true); + } +} diff --git a/components/heap/tlsf b/components/heap/tlsf index ff11688f242..ab17d6798d1 160000 --- a/components/heap/tlsf +++ b/components/heap/tlsf @@ -1 +1 @@ -Subproject commit ff11688f242b28b3918c2cdaa20738d12d73b5f4 +Subproject commit ab17d6798d1561758827b6553d56d57f19aa4d66