Просмотр исходного кода

Merge branch 'bugfix/fix_heap_memory_corruption' into 'master'

Heap: Fix a bug in the TLSF allocator

See merge request espressif/esp-idf!16296
Jiang Jiang Jian 4 лет назад
Родитель
Сommit
acee790f98

+ 12 - 6
components/heap/heap_tlsf.c

@@ -801,10 +801,14 @@ void* tlsf_memalign_offs(tlsf_t tlsf, size_t align, size_t size, size_t data_off
     */
 	const size_t size_with_gap = adjust_request_size(adjust + align + gap_minimum - off_adjust, align);
 
-	/*
-	** If alignment is less than or equals base alignment, we're done.
-	** If we requested 0 bytes, return null, as tlsf_malloc(0) does.
-	*/
+    /*
+    ** If alignment is less than or equal to base alignment, we're done, because
+    ** we are guaranteed that the size is at least sizeof(block_header_t), enough
+    ** to store next blocks' metadata. Plus, all pointers allocated will all be
+    ** aligned on a 4-byte bound, so ptr + data_offset will also have this
+    ** alignment constraint. Thus, the gap is not required.
+    ** If we requested 0 bytes, return null, as tlsf_malloc(0) does.
+    */
 	const size_t aligned_size = (adjust && align > ALIGN_SIZE) ? size_with_gap : adjust;
 
 	block_header_t* block = block_locate_free(control, aligned_size);
@@ -820,10 +824,12 @@ void* tlsf_memalign_offs(tlsf_t tlsf, size_t align, size_t size, size_t data_off
 			tlsf_cast(tlsfptr_t, aligned) - tlsf_cast(tlsfptr_t, ptr));
 
        /*
-        ** If gap size is too small or if there is not gap but we need one,
+        ** If gap size is too small or if there is no gap but we need one,
         ** offset to next aligned boundary.
+        ** NOTE: No need for a gap if the alignment required is less than or is
+        ** equal to ALIGN_SIZE.
         */
-		if ((gap && gap < gap_minimum) || (!gap && off_adjust))
+		if ((gap && gap < gap_minimum) || (!gap && off_adjust && align > ALIGN_SIZE))
 		{
 			const size_t gap_remain = gap_minimum - gap;
 			const size_t offset = tlsf_max(gap_remain, align);

+ 16 - 0
components/heap/test_multi_heap_host/test_multi_heap.cpp

@@ -484,6 +484,22 @@ TEST_CASE("multi_heap aligned allocations", "[multi_heap]")
         }
     }
 
+    /* Check that TLSF doesn't allocate a memory space smaller than required.
+     * In any case, TLSF will write data in the previous block than the one
+     * allocated. Thus, we should try to get/allocate this previous block. If
+     * the poisoned filled pattern has beeen overwritten by TLSF, then this
+     * previous block will trigger an exception.
+     * More info on this bug in !16296. */
+    const size_t size = 50; /* TLSF will round the size up */
+    uint8_t *buf1 = (uint8_t *)multi_heap_aligned_alloc(heap, size, 4);
+    uint8_t *buf2 = (uint8_t *)multi_heap_aligned_alloc(heap, size, 4);
+    multi_heap_free(heap, buf1);
+    /* By specifying a size equal of the gap between buf1 and buf2. We are
+     * trying to force TLSF to allocate two consecutive blocks. */
+    buf1 = (uint8_t *)multi_heap_aligned_alloc(heap, buf2 - buf1, 4);
+    multi_heap_free(heap, buf2);
+
+
     printf("[ALIGNED_ALLOC] heap_size after: %d \n", multi_heap_free_size(heap));
     REQUIRE((old_size - multi_heap_free_size(heap)) <= leakage);
 }