diff --git a/components/heap/heap_caps.c b/components/heap/heap_caps.c index 72d88ccaa8..9dea55673c 100644 --- a/components/heap/heap_caps.c +++ b/components/heap/heap_caps.c @@ -871,7 +871,7 @@ typedef struct walker_data { heap_t *heap; } walker_data_t; -__attribute__((noinline)) static void heap_caps_walker(void* block_ptr, size_t block_size, int block_used, void *user_data) +__attribute__((noinline)) static bool heap_caps_walker(void* block_ptr, size_t block_size, int block_used, void *user_data) { walker_data_t *walker_data = (walker_data_t*)user_data; @@ -885,7 +885,7 @@ __attribute__((noinline)) static void heap_caps_walker(void* block_ptr, size_t b (bool)block_used }; - walker_data->cb_func(heap_info, block_info, walker_data->opaque_ptr); + return walker_data->cb_func(heap_info, block_info, walker_data->opaque_ptr); } void heap_caps_walk(uint32_t caps, heap_caps_walker_cb_t walker_func, void *user_data) diff --git a/components/heap/include/esp_heap_caps.h b/components/heap/include/esp_heap_caps.h index 17101424b4..96e9b88977 100644 --- a/components/heap/include/esp_heap_caps.h +++ b/components/heap/include/esp_heap_caps.h @@ -464,10 +464,18 @@ typedef struct walker_block_info { } walker_block_info_t; /** - * @brief Function callback used to walk the heaps - * @see Definition in multi_heap.h + * @brief Function callback used to get information of memory block + * during calls to heap_caps_walk or heap_caps_walk_all + * + * @param heap_info See walker_heap_into_t + * @param block_info See walker_block_info_t + * @param user_data Opaque pointer to user defined data + * + * @return True to proceed with the heap traversal + * False to stop th traversal of the current heap and continue + * with the traversal of the next heap (if any) */ -typedef void (*heap_caps_walker_cb_t)(walker_heap_into_t heap_info, walker_block_info_t block_info, void *user_data); +typedef bool (*heap_caps_walker_cb_t)(walker_heap_into_t heap_info, walker_block_info_t block_info, void *user_data); /** * @brief Function called to walk through the heaps with the given set of capabilities diff --git a/components/heap/include/multi_heap.h b/components/heap/include/multi_heap.h index b7a2170b2f..f769fa4a7f 100644 --- a/components/heap/include/multi_heap.h +++ b/components/heap/include/multi_heap.h @@ -214,8 +214,11 @@ void multi_heap_restore_minimum_free_bytes(multi_heap_handle_t heap, const size_ * @param block_size The size of the block * @param block_used Block status. 0 if free, else, false * @param user_data Opaque pointer to user defined data + * + * @return True if the walker is expected to continue the heap traversal + * False if the walker is expected to stop the traversal of the heap */ -typedef void (*multi_heap_walker_cb_t)(void *block_ptr, size_t block_size, int block_used, void *user_data); +typedef bool (*multi_heap_walker_cb_t)(void *block_ptr, size_t block_size, int block_used, void *user_data); /** * @brief Call the tlsf_walk_pool function of the heap given as parameter with diff --git a/components/heap/multi_heap.c b/components/heap/multi_heap.c index f112f36a61..a92b4aa564 100644 --- a/components/heap/multi_heap.c +++ b/components/heap/multi_heap.c @@ -355,10 +355,11 @@ bool multi_heap_check(multi_heap_handle_t heap, bool print_errors) return valid; } -__attribute__((noinline)) static void multi_heap_dump_tlsf(void *ptr, size_t size, int used, void *user) +__attribute__((noinline)) static bool multi_heap_dump_tlsf(void *ptr, size_t size, int used, void *user) { (void)user; MULTI_HEAP_STDERR_PRINTF("Block %p data, size: %d bytes, Free: %s \n", (void *)ptr, size, used ? "No" : "Yes"); + return true; } void multi_heap_dump(multi_heap_handle_t heap) @@ -389,7 +390,7 @@ size_t multi_heap_minimum_free_size_impl(multi_heap_handle_t heap) return heap->minimum_free_bytes; } -__attribute__((noinline)) static void multi_heap_get_info_tlsf(void* ptr, size_t size, int used, void* user) +__attribute__((noinline)) static bool multi_heap_get_info_tlsf(void* ptr, size_t size, int used, void* user) { multi_heap_info_t *info = user; @@ -404,6 +405,7 @@ __attribute__((noinline)) static void multi_heap_get_info_tlsf(void* ptr, size_t } info->total_blocks++; + return true; } void multi_heap_get_info_impl(multi_heap_handle_t heap, multi_heap_info_t *info) diff --git a/components/heap/test_apps/heap_tests/main/test_walker.c b/components/heap/test_apps/heap_tests/main/test_walker.c index fdabc5bb85..3a96df7df2 100644 --- a/components/heap/test_apps/heap_tests/main/test_walker.c +++ b/components/heap/test_apps/heap_tests/main/test_walker.c @@ -7,25 +7,39 @@ #include "stdio.h" #include "esp_heap_caps.h" - +#include "esp_rom_sys.h" #define PASS_CODE 0x9876BAAB #define ALLOC_SIZE 16 -static bool block_found = false; -static void heap_walker(walker_heap_into_t heap_info, walker_block_info_t block_info, void* user_data) +static void calculate_block_metadata_size(size_t *header, size_t *footer) { + *header = 0; + *footer = 0; +#if !CONFIG_HEAP_POISONING_DISABLED + *header += 8; // sizeof(poison_head_t) + *footer += 4; // sizeof(poison_tail_t) +#endif +#if CONFIG_HEAP_TASK_TRACKING + *header += 4; // sizeof(TaskHandle_t) +#endif +} + +static bool block_found = false; +static bool heap_corrupted = false; +static bool heap_walker(walker_heap_into_t heap_info, walker_block_info_t block_info, void* user_data) +{ + if ((intptr_t)heap_info.end - (intptr_t)block_info.ptr < block_info.size) + { + heap_corrupted = true; + return false; + } + TEST_ASSERT(*(uint32_t*)user_data == PASS_CODE); TEST_ASSERT_NOT_NULL(block_info.ptr); - uint32_t metadata_size_head = 0; - uint32_t metadata_size_tail = 0; -#if !CONFIG_HEAP_POISONING_DISABLED - metadata_size_head += 8; // sizeof(poison_head_t) - metadata_size_tail += 4; // sizeof(poison_tail_t) -#endif -#if CONFIG_HEAP_TASK_TRACKING - metadata_size_head += 4; // sizeof(TaskHandle_t) -#endif + size_t metadata_size_head = 0; + size_t metadata_size_tail = 0; + calculate_block_metadata_size(&metadata_size_head, &metadata_size_tail); /* look for the first 4 bytes pass code to identify the memory allocated in the test */ const uint32_t pass_code = *((uint32_t*)block_info.ptr + (metadata_size_head / sizeof(uint32_t))); @@ -34,9 +48,14 @@ static void heap_walker(walker_heap_into_t heap_info, walker_block_info_t block_ TEST_ASSERT_TRUE(block_info.used); block_found = true; } + + return true; } /* This test assures the proper functioning of heap_caps_walk and heap_caps_walk_all + * when a corruption occurs in a random heap. The callback which detects the corruption + * returns false to the walker which should result in the interruption of the heap traversal + * by the walker instead of a crash. */ TEST_CASE("heap walker", "[heap]") { @@ -53,3 +72,27 @@ TEST_CASE("heap walker", "[heap]") heap_caps_walk_all(heap_walker, &user_code); TEST_ASSERT_TRUE(block_found); } + +/* This test assures the proper functioning of heap_caps_walk and heap_caps_walk_all + */ +TEST_CASE("heap walker corrupted heap detection", "[heap]") +{ + /* Allocate memory using the MALLOC_CAP_DEFAULT capability */ + void *default_ptr = heap_caps_malloc(ALLOC_SIZE, MALLOC_CAP_DEFAULT); + TEST_ASSERT_NOT_NULL(default_ptr); + + size_t metadata_size_head = 0; + size_t metadata_size_tail = 0; + calculate_block_metadata_size(&metadata_size_head, &metadata_size_tail); + (void)metadata_size_tail; + *((uint32_t*)default_ptr - (metadata_size_head / 4) - 1) = 0xFF000000; + + /* Write the pass code in the first word of the allocated memory */ + *((uint32_t*)default_ptr) = (uint32_t)PASS_CODE; + + /* call the heap_caps_walker to make sure the hook function is triggered + * and check that the allocated memory is found while walking the heap */ + uint32_t user_code = PASS_CODE; + heap_caps_walk_all(heap_walker, &user_code); + TEST_ASSERT_TRUE(heap_corrupted); +} diff --git a/components/heap/tlsf b/components/heap/tlsf index d2e28f8724..e282f0be5a 160000 --- a/components/heap/tlsf +++ b/components/heap/tlsf @@ -1 +1 @@ -Subproject commit d2e28f872472ffc6a704faae65ddee1f24e2dfba +Subproject commit e282f0be5a8488f48bf04cdc74514553c03e3c64