From d00ae499864d76458665c3e65b5ceff5c67e2bd7 Mon Sep 17 00:00:00 2001 From: Cobus van Eeden <35851496+cobusve@users.noreply.github.com> Date: Sat, 5 Dec 2020 15:54:11 -0800 Subject: [PATCH] Improve heap bounds checking in pvPortMalloc --- portable/MemMang/heap_1.c | 14 ++++++++++---- portable/MemMang/heap_2.c | 28 +++++++++++++++++---------- portable/MemMang/heap_4.c | 40 +++++++++++++++++++++++++++------------ portable/MemMang/heap_5.c | 16 ++++++++++++---- 4 files changed, 68 insertions(+), 30 deletions(-) diff --git a/portable/MemMang/heap_1.c b/portable/MemMang/heap_1.c index 4a7f8c2e5..149ea2a88 100644 --- a/portable/MemMang/heap_1.c +++ b/portable/MemMang/heap_1.c @@ -77,8 +77,13 @@ void * pvPortMalloc( size_t xWantedSize ) { if( xWantedSize & portBYTE_ALIGNMENT_MASK ) { - /* Byte alignment required. */ - xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) ); + /* Byte alignment required, check for overflow first */ + if ( (xWantedSize + ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) )) > xWantedSize ) + { + xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) ); + } else { + xWantedSize = 0; + } } } #endif @@ -91,8 +96,9 @@ void * pvPortMalloc( size_t xWantedSize ) pucAlignedHeap = ( uint8_t * ) ( ( ( portPOINTER_SIZE_TYPE ) & ucHeap[ portBYTE_ALIGNMENT ] ) & ( ~( ( portPOINTER_SIZE_TYPE ) portBYTE_ALIGNMENT_MASK ) ) ); } - /* Check there is enough room left for the allocation. */ - if( ( ( xNextFreeByte + xWantedSize ) < configADJUSTED_HEAP_SIZE ) && + /* Check there is enough room left for the allocation and. */ + if( ( xWantedSize > 0 ) && /* valid size */ + ( ( xNextFreeByte + xWantedSize ) < configADJUSTED_HEAP_SIZE ) && ( ( xNextFreeByte + xWantedSize ) > xNextFreeByte ) ) /* Check for overflow. */ { /* Return the next free byte then increment the index past this diff --git a/portable/MemMang/heap_2.c b/portable/MemMang/heap_2.c index 640cd54fd..bdf05c62d 100644 --- a/portable/MemMang/heap_2.c +++ b/portable/MemMang/heap_2.c @@ -125,28 +125,36 @@ void * pvPortMalloc( size_t xWantedSize ) vTaskSuspendAll(); { /* If this is the first call to malloc then the heap will require - * initialisation to setup the list of free blocks. */ + * initialisation to setup the list of free blocks. */ if( xHeapHasBeenInitialised == pdFALSE ) { prvHeapInit(); xHeapHasBeenInitialised = pdTRUE; } - /* The wanted size is increased so it can contain a BlockLink_t - * structure in addition to the requested amount of bytes. */ + /* The wanted size must be increased so it can contain a BlockLink_t + * structure in addition to the requested amount of bytes. */ if( xWantedSize > 0 ) { - xWantedSize += heapSTRUCT_SIZE; - - /* Ensure that blocks are always aligned to the required number of bytes. */ - if( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) != 0 ) + /* Ensure xWantedSize will never wrap after adjustment, even if we need + * an alignment adjustment */ + if ( xWantedSize < ( SIZE_MAX - heapSTRUCT_SIZE - portBYTE_ALIGNMENT - 1) ) { - /* Byte alignment required. */ - xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) ); + xWantedSize += heapSTRUCT_SIZE; + + /* Ensure that blocks are always aligned to the required number of bytes. */ + if( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) != 0 ) + { + /* Byte alignment required. */ + xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) ); + } + } else { + /* If the requested size is too large to handle we force an error */ + xWantedSize = 0; } } - if( ( xWantedSize > 0 ) && ( xWantedSize < configADJUSTED_HEAP_SIZE ) ) + if( xWantedSize > 0 ) { /* Blocks are stored in byte order - traverse the list from the start * (smallest) block until one of adequate size is found. */ diff --git a/portable/MemMang/heap_4.c b/portable/MemMang/heap_4.c index 7e5357ccf..dea300104 100644 --- a/portable/MemMang/heap_4.c +++ b/portable/MemMang/heap_4.c @@ -140,19 +140,35 @@ void * pvPortMalloc( size_t xWantedSize ) * structure in addition to the requested amount of bytes. */ if( xWantedSize > 0 ) { - xWantedSize += xHeapStructSize; + /* Check for overflow */ + if ( ( xWantedSize + xHeapStructSize ) > xWantedSize ) + { + xWantedSize += xHeapStructSize; - /* Ensure that blocks are always aligned to the required number - * of bytes. */ - if( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) != 0x00 ) + /* Ensure that blocks are always aligned to the required number + * of bytes. */ + if( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) != 0x00 ) + { + /* Byte alignment required. (check for overflow again) */ + if( ( xWantedSize + ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) ) ) + > xWantedSize ) + { + xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) ); + configASSERT( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) == 0 ); + } + else + { + xWantedSize = 0; + } + } + else + { + mtCOVERAGE_TEST_MARKER(); + } + } + else { - /* Byte alignment required. */ - xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) ); - configASSERT( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) == 0 ); - } - else - { - mtCOVERAGE_TEST_MARKER(); + xWantedSize = 0; } } else @@ -160,7 +176,7 @@ void * pvPortMalloc( size_t xWantedSize ) mtCOVERAGE_TEST_MARKER(); } - if( ( xWantedSize > 0 ) && ( xWantedSize <= xFreeBytesRemaining ) ) + if( ( xWantedSize > 0 ) && ( xWantedSize < xFreeBytesRemaining ) ) { /* Traverse the list from the start (lowest address) block until * one of adequate size is found. */ diff --git a/portable/MemMang/heap_5.c b/portable/MemMang/heap_5.c index 90c0b5bdf..848a1516b 100644 --- a/portable/MemMang/heap_5.c +++ b/portable/MemMang/heap_5.c @@ -150,7 +150,7 @@ void * pvPortMalloc( size_t xWantedSize ) { /* The wanted size is increased so it can contain a BlockLink_t * structure in addition to the requested amount of bytes. */ - if( xWantedSize > 0 ) + if( ( xWantedSize > 0 ) && ( ( xWantedSize + xHeapStructSize ) > xWantedSize ) ) { xWantedSize += xHeapStructSize; @@ -159,7 +159,15 @@ void * pvPortMalloc( size_t xWantedSize ) if( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) != 0x00 ) { /* Byte alignment required. */ - xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) ); + if( ( xWantedSize + ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) ) ) > + xWantedSize ) + { + xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) ); + } + else + { + xWantedSize = 0; + } } else { @@ -168,10 +176,10 @@ void * pvPortMalloc( size_t xWantedSize ) } else { - mtCOVERAGE_TEST_MARKER(); + xWantedSize = 0; } - if( ( xWantedSize > 0 ) && ( xWantedSize <= xFreeBytesRemaining ) ) + if( ( xWantedSize > 0 ) && ( xWantedSize < xFreeBytesRemaining ) ) { /* Traverse the list from the start (lowest address) block until * one of adequate size is found. */