From 15cdd2859af242f0e955d71b9b97020c3df659d3 Mon Sep 17 00:00:00 2001 From: Felipe Neves Date: Fri, 28 Feb 2020 13:17:34 -0300 Subject: [PATCH 1/2] heap: added aligned alloc implementation on multi_heap layer --- components/heap/multi_heap.c | 58 +++++++++++++++++++++++++++++ components/heap/test/test_realloc.c | 4 +- 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/components/heap/multi_heap.c b/components/heap/multi_heap.c index d131ff269..9a860d4ec 100644 --- a/components/heap/multi_heap.c +++ b/components/heap/multi_heap.c @@ -33,9 +33,15 @@ void *multi_heap_malloc(multi_heap_handle_t heap, size_t size) __attribute__((alias("multi_heap_malloc_impl"))); +void *multi_heap_aligned_alloc(multi_heap_handle_t heap, size_t size, size_t alignment) + __attribute__((alias("multi_heap_aligned_alloc_impl"))); + void multi_heap_free(multi_heap_handle_t heap, void *p) __attribute__((alias("multi_heap_free_impl"))); +void multi_heap_aligned_free(multi_heap_handle_t heap, void *p) + __attribute__((alias("multi_heap_aligned_free_impl"))); + void *multi_heap_realloc(multi_heap_handle_t heap, void *p, size_t size) __attribute__((alias("multi_heap_realloc_impl"))); @@ -66,6 +72,7 @@ void *multi_heap_get_block_owner(multi_heap_block_handle_t block) #define ALIGN(X) ((X) & ~(sizeof(void *)-1)) #define ALIGN_UP(X) ALIGN((X)+sizeof(void *)-1) +#define ALIGN_UP_BY(num, align) (((num) + ((align) - 1)) & ~((align) - 1)) struct heap_block; @@ -463,6 +470,57 @@ void *multi_heap_malloc_impl(multi_heap_handle_t heap, size_t size) return best_block->data; } +void *multi_heap_aligned_alloc_impl(multi_heap_handle_t heap, size_t size, size_t alignment) +{ + if(heap == NULL) { + return NULL; + } + + if(!size) { + return NULL; + } + + if(!alignment) { + return NULL; + } + + //Alignment must be a power of two... + if((alignment & (alignment - 1)) != 0) { + return NULL; + } + + uint32_t overhead = (sizeof(uint32_t) + (alignment - 1)); + + multi_heap_internal_lock(heap); + void *head = multi_heap_malloc_impl(heap, size + overhead); + if(head == NULL) { + multi_heap_internal_unlock(heap); + return NULL; + } + + //Lets align our new obtained block address: + //and save information to recover original block pointer + //to allow us to deallocate the memory when needed + void *ptr = (void *)ALIGN_UP_BY((uintptr_t)head + sizeof(uint32_t), alignment); + *((uint32_t *)ptr - 1) = (uint32_t)((uintptr_t)ptr - (uintptr_t)head); + + multi_heap_internal_unlock(heap); + return ptr; +} + +void multi_heap_aligned_free_impl(multi_heap_handle_t heap, void *p) +{ + if(p == NULL) { + return; + } + + multi_heap_internal_lock(heap); + uint32_t offset = *((uint32_t *)p - 1); + void *block_head = (void *)((uint8_t *)p - offset); + multi_heap_free_impl(heap, block_head); + multi_heap_internal_unlock(heap); +} + void multi_heap_free_impl(multi_heap_handle_t heap, void *p) { heap_block_t *pb = get_block(p); diff --git a/components/heap/test/test_realloc.c b/components/heap/test/test_realloc.c index 655666fe2..c4c2f5e10 100644 --- a/components/heap/test/test_realloc.c +++ b/components/heap/test/test_realloc.c @@ -14,10 +14,10 @@ /* (can't realloc in place if comprehensive is enabled) */ TEST_CASE("realloc shrink buffer in place", "[heap]") -{ +{ void *x = malloc(64); TEST_ASSERT(x); - void *y = realloc(p, 48); + void *y = realloc(x, 48); TEST_ASSERT_EQUAL_PTR(x, y); } From 5ce7ec848c43a16df317fe89b9ad0df3bb81d94c Mon Sep 17 00:00:00 2001 From: Felipe Neves Date: Fri, 28 Feb 2020 13:49:29 -0300 Subject: [PATCH 2/2] heap: pushed down all the aligned_alloc / free implementation --- components/heap/multi_heap.c | 17 +++--- components/heap/multi_heap_internal.h | 2 + components/heap/multi_heap_poisoning.c | 52 +++---------------- .../heap/test/test_aligned_alloc_caps.c | 35 ++++++++++--- .../test_multi_heap_host/test_multi_heap.cpp | 15 +++--- 5 files changed, 59 insertions(+), 62 deletions(-) diff --git a/components/heap/multi_heap.c b/components/heap/multi_heap.c index 9a860d4ec..f51f80736 100644 --- a/components/heap/multi_heap.c +++ b/components/heap/multi_heap.c @@ -472,20 +472,20 @@ void *multi_heap_malloc_impl(multi_heap_handle_t heap, size_t size) void *multi_heap_aligned_alloc_impl(multi_heap_handle_t heap, size_t size, size_t alignment) { - if(heap == NULL) { + if (heap == NULL) { return NULL; } - if(!size) { + if (!size) { return NULL; } - if(!alignment) { + if (!alignment) { return NULL; } //Alignment must be a power of two... - if((alignment & (alignment - 1)) != 0) { + if ((alignment & (alignment - 1)) != 0) { return NULL; } @@ -493,7 +493,7 @@ void *multi_heap_aligned_alloc_impl(multi_heap_handle_t heap, size_t size, size_ multi_heap_internal_lock(heap); void *head = multi_heap_malloc_impl(heap, size + overhead); - if(head == NULL) { + if (head == NULL) { multi_heap_internal_unlock(heap); return NULL; } @@ -510,13 +510,18 @@ void *multi_heap_aligned_alloc_impl(multi_heap_handle_t heap, size_t size, size_ void multi_heap_aligned_free_impl(multi_heap_handle_t heap, void *p) { - if(p == NULL) { + if (p == NULL) { return; } multi_heap_internal_lock(heap); uint32_t offset = *((uint32_t *)p - 1); void *block_head = (void *)((uint8_t *)p - offset); + +#ifdef MULTI_HEAP_POISONING_SLOW + multi_heap_internal_poison_fill_region(block_head, multi_heap_get_allocated_size_impl(heap, block_head), true /* free */); +#endif + multi_heap_free_impl(heap, block_head); multi_heap_internal_unlock(heap); } diff --git a/components/heap/multi_heap_internal.h b/components/heap/multi_heap_internal.h index 0f6a52009..998813f24 100644 --- a/components/heap/multi_heap_internal.h +++ b/components/heap/multi_heap_internal.h @@ -25,7 +25,9 @@ typedef const struct heap_block *multi_heap_block_handle_t; */ void *multi_heap_malloc_impl(multi_heap_handle_t heap, size_t size); +void *multi_heap_aligned_alloc_impl(multi_heap_handle_t heap, size_t size, size_t alignment); void multi_heap_free_impl(multi_heap_handle_t heap, void *p); +void multi_heap_aligned_free_impl(multi_heap_handle_t heap, void *p); void *multi_heap_realloc_impl(multi_heap_handle_t heap, void *p, size_t size); multi_heap_handle_t multi_heap_register_impl(void *start, size_t size); void multi_heap_get_info_impl(multi_heap_handle_t heap, multi_heap_info_t *info); diff --git a/components/heap/multi_heap_poisoning.c b/components/heap/multi_heap_poisoning.c index 8a356e649..119017d00 100644 --- a/components/heap/multi_heap_poisoning.c +++ b/components/heap/multi_heap_poisoning.c @@ -187,55 +187,28 @@ static bool verify_fill_pattern(void *data, size_t size, bool print_errors, bool void *multi_heap_aligned_alloc(multi_heap_handle_t heap, size_t size, size_t alignment) { - if(heap == NULL) { + if (size > SIZE_MAX - POISON_OVERHEAD) { return NULL; } - if(!size) { - return NULL; - } - - if(!alignment) { - return NULL; - } - - //Alignment must be a power of two... - if((alignment & (alignment - 1)) != 0) { - return NULL; - } - - if(size > SIZE_MAX - POISON_OVERHEAD) { - return NULL; - } - - uint32_t overhead = (sizeof(uint32_t) + (alignment - 1) + POISON_OVERHEAD); - multi_heap_internal_lock(heap); - poison_head_t *head = multi_heap_malloc_impl(heap, size + overhead); + poison_head_t *head = multi_heap_aligned_alloc_impl(heap, size + POISON_OVERHEAD, alignment); uint8_t *data = NULL; if (head != NULL) { - data = poison_allocated_region(head, size + (overhead - POISON_OVERHEAD)); + data = poison_allocated_region(head, size); #ifdef SLOW /* check everything we got back is FREE_FILL_PATTERN & swap for MALLOC_FILL_PATTERN */ bool ret = verify_fill_pattern(data, size, true, true, true); assert( ret ); -#else - (void)data; #endif } else { multi_heap_internal_unlock(heap); return NULL; } - //Lets align our new obtained block address: - //and save information to recover original block pointer - //to allow us to deallocate the memory when needed - void *ptr = (void *)ALIGN_UP((uintptr_t)head + sizeof(uint32_t) + sizeof(poison_head_t), alignment); - *((uint32_t *)ptr - 1) = (uint32_t)((uintptr_t)ptr - (uintptr_t)head); - multi_heap_internal_unlock(heap); - return ptr; + return data; } void *multi_heap_malloc(multi_heap_handle_t heap, size_t size) @@ -261,25 +234,16 @@ void *multi_heap_malloc(multi_heap_handle_t heap, size_t size) void multi_heap_aligned_free(multi_heap_handle_t heap, void *p) { - if(p == NULL) { - return; - } - multi_heap_internal_lock(heap); - - uint32_t offset = *((uint32_t *)p - 1); - void *block_head = (void *)((uint8_t *)p - offset); - block_head += sizeof(poison_head_t); - - poison_head_t *head = verify_allocated_region(block_head, true); + poison_head_t *head = verify_allocated_region(p, true); assert(head != NULL); - block_head -= sizeof(poison_head_t); + #ifdef SLOW /* replace everything with FREE_FILL_PATTERN, including the poison head/tail */ - memset(block_head, FREE_FILL_PATTERN, head->alloc_size + POISON_OVERHEAD); + memset(head, FREE_FILL_PATTERN, head->alloc_size + POISON_OVERHEAD); #endif - multi_heap_free_impl(heap, block_head); + multi_heap_aligned_free_impl(heap, head); multi_heap_internal_unlock(heap); } diff --git a/components/heap/test/test_aligned_alloc_caps.c b/components/heap/test/test_aligned_alloc_caps.c index 613eb1eb2..845d7cf44 100644 --- a/components/heap/test/test_aligned_alloc_caps.c +++ b/components/heap/test/test_aligned_alloc_caps.c @@ -28,7 +28,14 @@ TEST_CASE("Capabilities aligned allocator test", "[heap]") printf("[ALIGNED_ALLOC] alignment required: %u \n", alignments); printf("[ALIGNED_ALLOC] address of allocated memory: %p \n\n", (void *)buf); //Address of obtained block must be aligned with selected value - TEST_ASSERT(((intptr_t)buf & (alignments - 1)) == 0); + + if((alignments & 0x03) == 0) { + //Alignment is a multiple of four: + TEST_ASSERT(((intptr_t)buf & 0x03) == 0); + } else { + //Exotic alignments: + TEST_ASSERT(((intptr_t)buf & (alignments - 1)) == 0); + } //Write some data, if it corrupts memory probably the heap //canary verification will fail: @@ -57,8 +64,13 @@ TEST_CASE("Capabilities aligned allocator test", "[heap]") printf("[ALIGNED_ALLOC] alignment required: %u \n", alignments); printf("[ALIGNED_ALLOC] address of allocated memory: %p \n\n", (void *)buf); //Address of obtained block must be aligned with selected value - TEST_ASSERT(((intptr_t)buf & (alignments - 1)) == 0); - + if((alignments & 0x03) == 0) { + //Alignment is a multiple of four: + TEST_ASSERT(((intptr_t)buf & 0x03) == 0); + } else { + //Exotic alignments: + TEST_ASSERT(((intptr_t)buf & (alignments - 1)) == 0); + } //Write some data, if it corrupts memory probably the heap //canary verification will fail: memset(buf, 0xA5, (10*1024)); @@ -85,7 +97,13 @@ TEST_CASE("Capabilities aligned calloc test", "[heap]") printf("[ALIGNED_ALLOC] alignment required: %u \n", alignments); printf("[ALIGNED_ALLOC] address of allocated memory: %p \n\n", (void *)buf); //Address of obtained block must be aligned with selected value - TEST_ASSERT(((intptr_t)buf & (alignments - 1)) == 0); + if((alignments & 0x03) == 0) { + //Alignment is a multiple of four: + TEST_ASSERT(((intptr_t)buf & 0x03) == 0); + } else { + //Exotic alignments: + TEST_ASSERT(((intptr_t)buf & (alignments - 1)) == 0); + } //Write some data, if it corrupts memory probably the heap //canary verification will fail: @@ -126,8 +144,13 @@ TEST_CASE("Capabilities aligned calloc test", "[heap]") printf("[ALIGNED_ALLOC] alignment required: %u \n", alignments); printf("[ALIGNED_ALLOC] address of allocated memory: %p \n\n", (void *)buf); //Address of obtained block must be aligned with selected value - TEST_ASSERT(((intptr_t)buf & (alignments - 1)) == 0); - + if((alignments & 0x03) == 0) { + //Alignment is a multiple of four: + TEST_ASSERT(((intptr_t)buf & 0x03) == 0); + } else { + //Exotic alignments: + TEST_ASSERT(((intptr_t)buf & (alignments - 1)) == 0); + } //Write some data, if it corrupts memory probably the heap //canary verification will fail: memset(buf, 0xA5, (10*1024)); 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 f668dd953..c707882fe 100644 --- a/components/heap/test_multi_heap_host/test_multi_heap.cpp +++ b/components/heap/test_multi_heap_host/test_multi_heap.cpp @@ -495,8 +495,6 @@ TEST_CASE("unaligned heaps", "[multi_heap]") } } -#ifndef CONFIG_HEAP_POISONING_NONE - TEST_CASE("multi_heap aligned allocations", "[multi_heap]") { uint8_t test_heap[1024 * 1024]; @@ -527,7 +525,14 @@ TEST_CASE("multi_heap aligned allocations", "[multi_heap]") //printf("[ALIGNED_ALLOC] allocated size: %d \n", multi_heap_get_allocated_size(heap, buf)); printf("[ALIGNED_ALLOC] address of allocated memory: %p \n\n", (void *)buf); //Address of obtained block must be aligned with selected value - REQUIRE(((intptr_t)buf & (aligments - 1)) == 0); + + if((aligments & 0x03) == 0) { + //Alignment is a multiple of four: + REQUIRE(((intptr_t)buf & 0x03) == 0); + } else { + //Exotic alignments: + REQUIRE(((intptr_t)buf & (aligments - 1)) == 0); + } //Write some data, if it corrupts memory probably the heap //canary verification will fail: @@ -539,6 +544,4 @@ TEST_CASE("multi_heap aligned allocations", "[multi_heap]") printf("[ALIGNED_ALLOC] heap_size after: %d \n", multi_heap_free_size(heap)); REQUIRE((old_size - multi_heap_free_size(heap)) <= leakage); -} - -#endif \ No newline at end of file +} \ No newline at end of file