feat(heap): Add return value to walker callback

This return value is used by the tlsf_walk_pool
function to be notified of the need to interrupt
the ongoing traversal of the currently traversed
heap.

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)

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
+ * @brief Function callback used to get information of memory block
- * @see Definition in multi_heap.h
+ * 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

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

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)

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 calculate_block_metadata_size(size_t *header, size_t *footer)
-static void heap_walker(walker_heap_into_t heap_info, walker_block_info_t block_info, void* user_data)
 {
+    *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;
+    size_t metadata_size_head = 0;
-    uint32_t metadata_size_tail = 0;
+    size_t metadata_size_tail = 0;
-#if !CONFIG_HEAP_POISONING_DISABLED
+    calculate_block_metadata_size(&metadata_size_head, &metadata_size_tail);
-    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
 
     /* 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);
+}