Ver Fonte

heap: Fix regression in `heap_caps_add_region` API related to address range checks

Regression was introduced in 32408b718f7c7699c0ef4af1fc768a696e2d6b74, which disallowed
addition of heap region with following condition:

`new_start < start && new_end == start`

This caused issues in Bluetooth APIs `esp_bt_mem_release` or `esp_bt_controller_mem_release`.

This commit fixes the problem and also adds API documentation for supported memory address
ranges in heap add region APIs.
Mahavir Jain há 3 anos atrás
pai
commit
f13e25d156

+ 23 - 18
components/heap/heap_caps_init.c

@@ -162,15 +162,8 @@ esp_err_t heap_caps_add_region(intptr_t start, intptr_t end)
     return ESP_ERR_NOT_FOUND;
 }
 
-esp_err_t heap_caps_add_region_with_caps(const uint32_t caps[], 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)
 {
-    esp_err_t err = ESP_FAIL;
-    if (caps == NULL || start == 0 || end == 0 || end <= start) {
-        return ESP_ERR_INVALID_ARG;
-    }
-
-    //Check if region overlaps the start and/or end of an existing region. If so, the
-    //region is invalid (or maybe added twice)
     /*
      *  We assume that in any region, the "start" must be stictly less than the end.
      *  Specially, the 3rd scenario can be allowed. For example, allocate memory from heap,
@@ -183,27 +176,39 @@ esp_err_t heap_caps_add_region_with_caps(const uint32_t caps[], intptr_t start,
      *  the existing heap region                                  s(tart)                e(nd)
      *                                                            |----------------------|
      *
-     *  1.add region  (e1<s)                                |-----|                                      correct: bool condition_1 = end < heap->start;
+     *  1.add region  (e1<s)                                |-----|                                      correct: bool condition_1 = end < heap_start;
      *
-     *  2.add region  (s2<s && e2>=s)                       |-----------------|                          wrong:   bool condition_2 = start < heap->start && end >= heap->start;
+     *  2.add region  (s2<s && e2>s)                        |-----------------|                          wrong:   bool condition_2 = start < heap_start && end > heap_start;
      *                                                      |---------------------------------|          wrong
      *
-     *  3.add region  (s3>=s && e3<e)                             |---------------|                      correct: bool condition_3 = start >= heap->start && end < heap->end;
+     *  3.add region  (s3>=s && e3<e)                             |---------------|                      correct: bool condition_3 = start >= heap_start && end < heap_end;
      *                                                                  |--------------|                 correct
      *
-     *  4.add region  (s4<e && e4>=e)                             |----------------------|               wrong:   bool condition_4 = start < heap->end && end >= heap->end;
+     *  4.add region  (s4<e && e4>e)                              |------------------------|             wrong:   bool condition_4 = start < heap_end && end > heap_end;
      *                                                                  |---------------------|          wrong
      *
-     *  5.add region  (s5>=e)                                                            |----|          correct: bool condition_5 = start >= heap->end;
+     *  5.add region  (s5>=e)                                                            |----|          correct: bool condition_5 = start >= 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
+
+    return (condition_2 || condition_4) ? false: true;
+}
+
+esp_err_t heap_caps_add_region_with_caps(const uint32_t caps[], intptr_t start, intptr_t end)
+{
+    esp_err_t err = ESP_FAIL;
+    if (caps == NULL || start == 0 || end == 0 || end <= start) {
+        return ESP_ERR_INVALID_ARG;
+    }
+
+    //Check if region overlaps the start and/or end of an existing region. If so, the
+    //region is invalid (or maybe added twice)
     heap_t *heap;
     SLIST_FOREACH(heap, &registered_heaps, next) {
-        bool condition_2 = start < heap->start && end >= heap->start;       //if 1, wrong
-        bool condition_4 = start < heap->end && end >= heap->end;           //if 1, wrong
-        bool wrong_region = condition_2 || condition_4;
-
-        if (wrong_region) {
+        if (!heap_caps_check_add_region_allowed(heap->start, heap->end, start, end)) {
+            ESP_EARLY_LOGD(TAG, "invalid overlap detected with existing heap region");
             return ESP_FAIL;
         }
     }

+ 28 - 0
components/heap/include/esp_heap_caps_init.h

@@ -49,6 +49,20 @@ void heap_caps_enable_nonos_stack_heaps(void);
  *
  * Use heap_caps_add_region_with_caps() to register a region with custom capabilities.
  *
+ * @note Please refer to following example for memory regions allowed for addition to heap based on an existing region
+ * (address range for demonstration purpose only):
+ @verbatim
+       Existing region: 0x1000 <-> 0x3000
+       New region:      0x1000 <-> 0x3000 (Allowed)
+       New region:      0x1000 <-> 0x2000 (Allowed)
+       New region:      0x0000 <-> 0x1000 (Allowed)
+       New region:      0x3000 <-> 0x4000 (Allowed)
+       New region:      0x0000 <-> 0x2000 (NOT Allowed)
+       New region:      0x0000 <-> 0x4000 (NOT Allowed)
+       New region:      0x1000 <-> 0x4000 (NOT Allowed)
+       New region:      0x2000 <-> 0x4000 (NOT Allowed)
+ @endverbatim
+ *
  * @param start Start address of new region.
  * @param end End address of new region.
  *
@@ -63,6 +77,20 @@ esp_err_t heap_caps_add_region(intptr_t start, intptr_t end);
  *
  * Similar to heap_caps_add_region(), only custom memory capabilities are specified by the caller.
  *
+ * @note Please refer to following example for memory regions allowed for addition to heap based on an existing region
+ * (address range for demonstration purpose only):
+ @verbatim
+       Existing region: 0x1000 <-> 0x3000
+       New region:      0x1000 <-> 0x3000 (Allowed)
+       New region:      0x1000 <-> 0x2000 (Allowed)
+       New region:      0x0000 <-> 0x1000 (Allowed)
+       New region:      0x3000 <-> 0x4000 (Allowed)
+       New region:      0x0000 <-> 0x2000 (NOT Allowed)
+       New region:      0x0000 <-> 0x4000 (NOT Allowed)
+       New region:      0x1000 <-> 0x4000 (NOT Allowed)
+       New region:      0x2000 <-> 0x4000 (NOT Allowed)
+ @endverbatim
+ *
  * @param caps Ordered array of capability masks for the new region, in order of priority. Must have length
  * SOC_MEMORY_TYPE_NO_PRIOS. Does not need to remain valid after the call returns.
  * @param start Start address of new region.