From: Angus Gratton <angus@espressif.com>
Date: Wed, 19 Dec 2018 22:34:24 +0000 (+1100)
Subject: multi_heap: Fix bug when start of heap is unaligned
X-Git-Tag: v3.2-beta3~61^2~1
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=348d6d5d3f53e6cee3122f5cc6a8238ff8718868;p=esp-idf

multi_heap: Fix bug when start of heap is unaligned

Alignment was accounted for in one place only.

TW27954
---

diff --git a/components/heap/multi_heap.c b/components/heap/multi_heap.c
index 39917f4af0..d131ff269c 100644
--- a/components/heap/multi_heap.c
+++ b/components/heap/multi_heap.c
@@ -326,18 +326,21 @@ size_t multi_heap_get_allocated_size_impl(multi_heap_handle_t heap, void *p)
     return block_data_size(pb);
 }
 
-multi_heap_handle_t multi_heap_register_impl(void *start, size_t size)
+multi_heap_handle_t multi_heap_register_impl(void *start_ptr, size_t size)
 {
-    heap_t *heap = (heap_t *)ALIGN_UP((intptr_t)start);
-    uintptr_t end = ALIGN((uintptr_t)start + size);
-    if (end - (uintptr_t)start < sizeof(heap_t) + 2*sizeof(heap_block_t)) {
+    uintptr_t start = ALIGN_UP((uintptr_t)start_ptr);
+    uintptr_t end = ALIGN((uintptr_t)start_ptr + size);
+    heap_t *heap = (heap_t *)start;
+    size = end - start;
+
+    if (end < start || size < sizeof(heap_t) + 2*sizeof(heap_block_t)) {
         return NULL; /* 'size' is too small to fit a heap here */
     }
     heap->lock = NULL;
     heap->last_block = (heap_block_t *)(end - sizeof(heap_block_t));
 
     /* first 'real' (allocatable) free block goes after the heap structure */
-    heap_block_t *first_free_block = (heap_block_t *)((intptr_t)start + sizeof(heap_t));
+    heap_block_t *first_free_block = (heap_block_t *)(start + sizeof(heap_t));
     first_free_block->header = (intptr_t)heap->last_block | BLOCK_FREE_FLAG;
     first_free_block->next_free = heap->last_block;
 
@@ -356,7 +359,7 @@ multi_heap_handle_t multi_heap_register_impl(void *start, size_t size)
        - minus header of first_free_block
        - minus whole block at heap->last_block
     */
-    heap->free_bytes = ALIGN(size) - sizeof(heap_t) - sizeof(first_free_block->header) - sizeof(heap_block_t);
+    heap->free_bytes = size - sizeof(heap_t) - sizeof(first_free_block->header) - sizeof(heap_block_t);
     heap->minimum_free_bytes = heap->free_bytes;
 
     return heap;
diff --git a/components/heap/test_multi_heap_host/test_multi_heap.cpp b/components/heap/test_multi_heap_host/test_multi_heap.cpp
index 133f12536b..310ee9dcf1 100644
--- a/components/heap/test_multi_heap_host/test_multi_heap.cpp
+++ b/components/heap/test_multi_heap_host/test_multi_heap.cpp
@@ -453,3 +453,44 @@ TEST_CASE("corrupt heap block", "[multi_heap]")
     memset(a, 0xEE, 64);
     REQUIRE( !multi_heap_check(heap, true) );
 }
+
+TEST_CASE("unaligned heaps", "[multi_heap]")
+{
+    const size_t CHUNK_LEN = 256;
+    const size_t CANARY_LEN = 16;
+    const uint8_t CANARY_BYTE = 0x3E;
+    uint8_t heap_chunk[CHUNK_LEN + CANARY_LEN * 2];
+
+    /* Put some canary bytes before and after the bytes we intend to use for
+       the heap, make sure they aren't ever overwritten */
+    memset(heap_chunk, CANARY_BYTE, CANARY_LEN);
+    memset(heap_chunk + CANARY_LEN + CHUNK_LEN, CANARY_BYTE, CANARY_LEN);
+
+    for (int i = 0; i < 8; i++) {
+        printf("Testing with offset %d\n", i);
+        multi_heap_handle_t heap = multi_heap_register(heap_chunk + CANARY_LEN + i, CHUNK_LEN - i);
+        multi_heap_info_t info;
+
+        REQUIRE( multi_heap_check(heap, true) );
+
+        multi_heap_get_info(heap, &info);
+
+        REQUIRE( info.total_free_bytes > CHUNK_LEN - 64 - i );
+        REQUIRE( info.largest_free_block > CHUNK_LEN - 64 - i );
+
+        void *a = multi_heap_malloc(heap, info.largest_free_block);
+        REQUIRE( a != NULL );
+        memset(a, 0xAA, info.largest_free_block);
+
+        REQUIRE( multi_heap_check(heap, true) );
+
+        multi_heap_free(heap, a);
+
+        REQUIRE( multi_heap_check(heap, true) );
+
+        for (unsigned j = 0; j < CANARY_LEN; j++) { // check canaries
+            REQUIRE( heap_chunk[j] == CANARY_BYTE );
+            REQUIRE( heap_chunk[CHUNK_LEN + CANARY_LEN + j] == CANARY_BYTE );
+        }
+    }
+}