Skip to content

Commit

Permalink
heap: Fix memory boundary condition checks when adding new region
Browse files Browse the repository at this point in the history
Previously, condition_4 was making impossible the valid scenario
start > heap_start && end == heap_end.

Now, the end == heap_end and start == heap_start case is handled
separately allowing the case start > heap_start && ned == heap_end
to be considered a valid scenario
  • Loading branch information
SoucheSouche committed Oct 12, 2022
1 parent 829340d commit 5ec5faf
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 13 deletions.
27 changes: 15 additions & 12 deletions components/heap/heap_caps_init.c
Original file line number Diff line number Diff line change
Expand Up @@ -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++) {
Expand All @@ -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 */
size_t num_heaps = 0;
for (size_t i = 0; i < num_regions; i++) {
if (regions[i].type != -1) {
num_heaps++;
// remove one heap from the number of heaps as
// 2 regions just got coalesed.
num_heaps--;
}
}

Expand Down Expand Up @@ -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 all equal scenario is incorrect because the same region cannot be add twice. For example,
* add the .bss memory to region twice, if not do the check, it will cause exception.
* This 'start = start' and 'end = end' scenario is incorrect because the same region
* cannot be added twice. In fact, registering the same memory region as a heap twice
* would cause a corruption and then an exception at runtime.
*
* the existing heap region s(tart) e(nd)
* |----------------------|
Expand All @@ -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)
Expand Down
5 changes: 4 additions & 1 deletion components/heap/test/test_runtime_heap_reg.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

0 comments on commit 5ec5faf

Please sign in to comment.