Merge branch 'fix/add-region-allowed-checks_v5.0' into 'release/v5.0'

heap: Fix memory boundary condition checks when adding new region (backport v5.0)

See merge request espressif/esp-idf!20557

components/heap/heap_caps_init.c

@@ -60,6 +60,10 @@ void heap_caps_init(void)
     soc_memory_region_t regions[num_regions];
     num_regions = soc_get_available_memory_regions(regions);
 
+    // the following for loop will calculate the number of possible heaps
+    // based on how many regions were coalesed.
+    size_t num_heaps = num_regions;
+
     //The heap allocator will treat every region given to it as separate. In order to get bigger ranges of contiguous memory,
     //it's useful to coalesce adjacent regions that have the same type.
     for (size_t i = 1; i < num_regions; i++) {
@@ -69,14 +73,10 @@ void heap_caps_init(void)
             a->type = -1;
             b->start = a->start;
             b->size += a->size;
-        }
-    }
 
-    /* Count the heaps left after merging */
+            // remove one heap from the number of heaps as
-    size_t num_heaps = 0;
+            // 2 regions just got coalesed.
-    for (size_t i = 0; i < num_regions; i++) {
+            num_heaps--;
-        if (regions[i].type != -1) {
-            num_heaps++;
         }
     }
 
@@ -170,13 +170,13 @@ esp_err_t heap_caps_add_region(intptr_t start, intptr_t end)
 bool heap_caps_check_add_region_allowed(intptr_t heap_start, intptr_t heap_end, intptr_t start, intptr_t end)
 {
     /*
-     *  We assume that in any region, the "start" must be stictly less than the end.
+     *  We assume that in any region, the "start" must be strictly less than the end.
      *  Specially, the 3rd scenario can be allowed. For example, allocate memory from heap,
      *  then change the capability and call this function to create a new region for special
      *  application.
-     *  In the following chart, 'start = start' and 'end = end' is contained in 4th scenario.
+     *  This 'start = start' and 'end = end' scenario is incorrect because the same region
-     *  This all equal scenario is incorrect because the same region cannot be add twice. For example,
+     *  cannot be added twice. In fact, registering the same memory region as a heap twice
-     *  add the .bss memory to region twice, if not do the check, it will cause exception.
+     *  would cause a corruption and then an exception at runtime.
      *
      *  the existing heap region                                  s(tart)                e(nd)
      *                                                            |----------------------|
@@ -193,12 +193,15 @@ bool heap_caps_check_add_region_allowed(intptr_t heap_start, intptr_t heap_end,
      *                                                                  |---------------------|          wrong
      *
      *  5.add region  (s5>=e)                                                            |----|          correct: bool condition_5 = start >= heap_end;
+     *
+     * 6.add region (s6==s && e6==e)                              |----------------------|               wrong: bool condition_6 = start == heap_start && end == heap_end;
      */
 
     bool condition_2 = start < heap_start && end > heap_start;        // if true then region not allowed
     bool condition_4 = start < heap_end && end > heap_end;            // if true then region not allowed
+    bool condition_6 = start == heap_start && end == heap_end;        // if true then region not allowed
 
-    return (condition_2 || condition_4) ? false: true;
+    return (condition_2 || condition_4 || condition_6) ? false: true;
 }
 
 esp_err_t heap_caps_add_region_with_caps(const uint32_t caps[], intptr_t start, intptr_t end)

components/heap/test/test_runtime_heap_reg.c

@@ -81,10 +81,13 @@ TEST_CASE("Add heap region address range checks", "[heap]")
 
     TEST_ASSERT_TRUE(heap_caps_check_add_region_allowed(heap_start, heap_end, 0x0, 0x1000));
     TEST_ASSERT_TRUE(heap_caps_check_add_region_allowed(heap_start, heap_end, 0x1000, 0x2000));
-    TEST_ASSERT_TRUE(heap_caps_check_add_region_allowed(heap_start, heap_end, 0x1000, 0x3000));
     TEST_ASSERT_TRUE(heap_caps_check_add_region_allowed(heap_start, heap_end, 0x3000, 0x4000));
+    TEST_ASSERT_TRUE(heap_caps_check_add_region_allowed(heap_start, heap_end, 0x1000, 0x2999));
+    TEST_ASSERT_TRUE(heap_caps_check_add_region_allowed(heap_start, heap_end, 0x1001, 0x3000));
+    TEST_ASSERT_TRUE(heap_caps_check_add_region_allowed(heap_start, heap_end, 0x1001, 0x2999));
     TEST_ASSERT_FALSE(heap_caps_check_add_region_allowed(heap_start, heap_end, 0x0, 0x2000));
     TEST_ASSERT_FALSE(heap_caps_check_add_region_allowed(heap_start, heap_end, 0x0, 0x4000));
     TEST_ASSERT_FALSE(heap_caps_check_add_region_allowed(heap_start, heap_end, 0x1000, 0x4000));
     TEST_ASSERT_FALSE(heap_caps_check_add_region_allowed(heap_start, heap_end, 0x2000, 0x4000));
+    TEST_ASSERT_FALSE(heap_caps_check_add_region_allowed(heap_start, heap_end, 0x1000, 0x3000));
 }