Improve heap2 bounds checking (#224)

cobusve · web-flow · commit c7a9a01c9498 · 2020-12-07T10:36:27.000-08:00
* Improve heap bounds checking in pvPortMalloc
diff --git a/portable/MemMang/heap_1.c b/portable/MemMang/heap_1.c
@@ -22,7 +22,6 @@
  * https://www.FreeRTOS.org
  * https://github.com/FreeRTOS
  *
- * 1 tab == 4 spaces!
  */
 
 
@@ -72,13 +71,20 @@ void * pvPortMalloc( size_t xWantedSize )
     void * pvReturn = NULL;
     static uint8_t * pucAlignedHeap = NULL;
 
-    /* Ensure that blocks are always aligned to the required number of bytes. */
+    /* Ensure that blocks are always aligned. */
     #if ( portBYTE_ALIGNMENT != 1 )
         {
             if( xWantedSize & portBYTE_ALIGNMENT_MASK )
             {
-                /* Byte alignment required. */
-                xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );
+                /* Byte alignment required. Check for overflow. */
+                if ( (xWantedSize + ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) )) > xWantedSize )
+                {
+                    xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );
+                } 
+                else 
+                {
+                    xWantedSize = 0;
+                }
             }
         }
     #endif
@@ -91,8 +97,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
@@ -22,7 +22,6 @@
  * https://www.FreeRTOS.org
  * https://github.com/FreeRTOS
  *
- * 1 tab == 4 spaces!
  */
 
 /*
@@ -132,21 +131,32 @@ void * pvPortMalloc( size_t xWantedSize )
             xHeapHasBeenInitialised = pdTRUE;
         }
 
-        /* The wanted size is increased so it can contain a BlockLink_t
+        /* 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 )
+        if( ( xWantedSize > 0 ) && 
+            ( ( xWantedSize + heapSTRUCT_SIZE ) >  xWantedSize ) ) /* Overflow check */
         {
             xWantedSize += heapSTRUCT_SIZE;
 
-            /* Ensure that blocks are always aligned to the required number of bytes. */
-            if( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) != 0 )
+            /* Byte alignment required. Check for overflow. */
+            if( ( xWantedSize + ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) ) ) 
+                    > xWantedSize )
             {
-                /* Byte alignment required. */
                 xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );
+                configASSERT( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) == 0 );
             }
+            else
+            {
+                xWantedSize = 0;
+            }       
+        }
+        else 
+        {
+            xWantedSize = 0; 
         }
 
-        if( ( xWantedSize > 0 ) && ( xWantedSize < configADJUSTED_HEAP_SIZE ) )
+
+        if( ( xWantedSize > 0 ) && ( xWantedSize <= xFreeBytesRemaining ) )
         {
             /* 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
@@ -136,34 +136,42 @@ void * pvPortMalloc( size_t xWantedSize )
          * kernel, so it must be free. */
         if( ( xWantedSize & xBlockAllocatedBit ) == 0 )
         {
-            /* The wanted size is increased so it can contain a BlockLink_t
+            /* 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 )
+            if( ( xWantedSize > 0 ) && 
+                ( ( xWantedSize + xHeapStructSize ) >  xWantedSize ) ) /* Overflow check */
             {
                 xWantedSize += xHeapStructSize;
 
-                /* Ensure that blocks are always aligned to the required number
-                 * of bytes. */
+                /* Ensure that blocks are always aligned. */
                 if( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) != 0x00 )
                 {
-                    /* Byte alignment required. */
-                    xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );
-                    configASSERT( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) == 0 );
+                    /* Byte alignment required. Check for overflow. */
+                    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
+            } 
+            else 
             {
-                mtCOVERAGE_TEST_MARKER();
+                xWantedSize = 0;
             }
 
             if( ( xWantedSize > 0 ) && ( xWantedSize <= xFreeBytesRemaining ) )
             {
                 /* Traverse the list from the start	(lowest address) block until
-                 * one	of adequate size is found. */
+                 * one of adequate size is found. */
                 pxPreviousBlock = &xStart;
                 pxBlock = xStart.pxNextFreeBlock;
 
@@ -174,7 +182,7 @@ void * pvPortMalloc( size_t xWantedSize )
                 }
 
                 /* If the end marker was reached then a block of adequate size
-                 * was	not found. */
+                 * was not found. */
                 if( pxBlock != pxEnd )
                 {
                     /* Return the memory space pointed to - jumping over the
diff --git a/portable/MemMang/heap_5.c b/portable/MemMang/heap_5.c
@@ -22,7 +22,6 @@
  * https://www.FreeRTOS.org
  * https://github.com/FreeRTOS
  *
- * 1 tab == 4 spaces!
  */
 
 /*
@@ -150,16 +149,24 @@ 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 ) ) /* Overflow check */
             {
                 xWantedSize += xHeapStructSize;
 
-                /* Ensure that blocks are always aligned to the required number
-                 * of bytes. */
+                /* Ensure that blocks are always aligned */
                 if( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) != 0x00 )
                 {
-                    /* Byte alignment required. */
-                    xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );
+                    /* Byte alignment required. Check for overflow */
+                    if( ( xWantedSize + ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) ) ) >
+                         xWantedSize )
+                    {
+                        xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );
+                    } 
+                    else 
+                    {
+                        xWantedSize = 0;
+                    }
                 }
                 else
                 {
@@ -168,13 +175,13 @@ void * pvPortMalloc( size_t xWantedSize )
             }
             else
             {
-                mtCOVERAGE_TEST_MARKER();
+                xWantedSize = 0;
             }
 
             if( ( xWantedSize > 0 ) && ( xWantedSize <= xFreeBytesRemaining ) )
             {
                 /* Traverse the list from the start	(lowest address) block until
-                 * one	of adequate size is found. */
+                 * one of adequate size is found. */
                 pxPreviousBlock = &xStart;
                 pxBlock = xStart.pxNextFreeBlock;
 
@@ -185,7 +192,7 @@ void * pvPortMalloc( size_t xWantedSize )
                 }
 
                 /* If the end marker was reached then a block of adequate size
-                 * was	not found. */
+                 * was not found. */
                 if( pxBlock != pxEnd )
                 {
                     /* Return the memory space pointed to - jumping over the

Original file line number	Diff line number	Diff line change
`@@ -22,7 +22,6 @@`
`22`	`22`	`* https://www.FreeRTOS.org`
`23`	`23`	`* https://github.com/FreeRTOS`
`24`	`24`	`*`
`25`		`- * 1 tab == 4 spaces!`
`26`	`25`	`*/`
`27`	`26`
`28`	`27`
`@@ -72,13 +71,20 @@ void * pvPortMalloc( size_t xWantedSize )`
`72`	`71`	`void * pvReturn = NULL;`
`73`	`72`	`static uint8_t * pucAlignedHeap = NULL;`
`74`	`73`
`75`		`- /* Ensure that blocks are always aligned to the required number of bytes. */`
	`74`	`+ /* Ensure that blocks are always aligned. */`
`76`	`75`	`#if ( portBYTE_ALIGNMENT != 1 )`
`77`	`76`	`{`
`78`	`77`	`if( xWantedSize & portBYTE_ALIGNMENT_MASK )`
`79`	`78`	`{`
`80`		`- /* Byte alignment required. */`
`81`		`- xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );`
	`79`	`+ /* Byte alignment required. Check for overflow. */`
	`80`	`+ if ( (xWantedSize + ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) )) > xWantedSize )`
	`81`	`+ {`
	`82`	`+ xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );`
	`83`	`+ }`
	`84`	`+ else`
	`85`	`+ {`
	`86`	`+ xWantedSize = 0;`
	`87`	`+ }`
`82`	`88`	`}`
`83`	`89`	`}`
`84`	`90`	`#endif`
`@@ -91,8 +97,9 @@ void * pvPortMalloc( size_t xWantedSize )`
`91`	`97`	`pucAlignedHeap = ( uint8_t * ) ( ( ( portPOINTER_SIZE_TYPE ) & ucHeap[ portBYTE_ALIGNMENT ] ) & ( ~( ( portPOINTER_SIZE_TYPE ) portBYTE_ALIGNMENT_MASK ) ) );`
`92`	`98`	`}`
`93`	`99`
`94`		`- /* Check there is enough room left for the allocation. */`
`95`		`- if( ( ( xNextFreeByte + xWantedSize ) < configADJUSTED_HEAP_SIZE ) &&`
	`100`	`+ /* Check there is enough room left for the allocation and. */`
	`101`	`+ if( ( xWantedSize > 0 ) && /* valid size */`
	`102`	`+ ( ( xNextFreeByte + xWantedSize ) < configADJUSTED_HEAP_SIZE ) &&`
`96`	`103`	`( ( xNextFreeByte + xWantedSize ) > xNextFreeByte ) ) /* Check for overflow. */`
`97`	`104`	`{`
`98`	`105`	`/* Return the next free byte then increment the index past this`
Original file line number	Diff line number	Diff line change
`@@ -136,34 +136,42 @@ void * pvPortMalloc( size_t xWantedSize )`
`136`	`136`	`* kernel, so it must be free. */`
`137`	`137`	`if( ( xWantedSize & xBlockAllocatedBit ) == 0 )`
`138`	`138`	`{`
`139`		`- /* The wanted size is increased so it can contain a BlockLink_t`
	`139`	`+ /* The wanted size must be increased so it can contain a BlockLink_t`
`140`	`140`	`* structure in addition to the requested amount of bytes. */`
`141`		`- if( xWantedSize > 0 )`
	`141`	`+ if( ( xWantedSize > 0 ) &&`
	`142`	`+ ( ( xWantedSize + xHeapStructSize ) > xWantedSize ) ) /* Overflow check */`
`142`	`143`	`{`
`143`	`144`	`xWantedSize += xHeapStructSize;`
`144`	`145`
`145`		`- /* Ensure that blocks are always aligned to the required number`
`146`		`- * of bytes. */`
	`146`	`+ /* Ensure that blocks are always aligned. */`
`147`	`147`	`if( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) != 0x00 )`
`148`	`148`	`{`
`149`		`- /* Byte alignment required. */`
`150`		`- xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );`
`151`		`- configASSERT( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) == 0 );`
	`149`	`+ /* Byte alignment required. Check for overflow. */`
	`150`	`+ if( ( xWantedSize + ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) ) )`
	`151`	`+ > xWantedSize )`
	`152`	`+ {`
	`153`	`+ xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );`
	`154`	`+ configASSERT( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) == 0 );`
	`155`	`+ }`
	`156`	`+ else`
	`157`	`+ {`
	`158`	`+ xWantedSize = 0;`
	`159`	`+ }`
`152`	`160`	`}`
`153`	`161`	`else`
`154`	`162`	`{`
`155`	`163`	`mtCOVERAGE_TEST_MARKER();`
`156`	`164`	`}`
`157`		`- }`
`158`		`- else`
	`165`	`+ }`
	`166`	`+ else`
`159`	`167`	`{`
`160`		`- mtCOVERAGE_TEST_MARKER();`
	`168`	`+ xWantedSize = 0;`
`161`	`169`	`}`
`162`	`170`
`163`	`171`	`if( ( xWantedSize > 0 ) && ( xWantedSize <= xFreeBytesRemaining ) )`
`164`	`172`	`{`
`165`	`173`	`/* Traverse the list from the start (lowest address) block until`
`166`		`- * one of adequate size is found. */`
	`174`	`+ * one of adequate size is found. */`
`167`	`175`	`pxPreviousBlock = &xStart;`
`168`	`176`	`pxBlock = xStart.pxNextFreeBlock;`
`169`	`177`
`@@ -174,7 +182,7 @@ void * pvPortMalloc( size_t xWantedSize )`
`174`	`182`	`}`
`175`	`183`
`176`	`184`	`/* If the end marker was reached then a block of adequate size`
`177`		`- * was not found. */`
	`185`	`+ * was not found. */`
`178`	`186`	`if( pxBlock != pxEnd )`
`179`	`187`	`{`
`180`	`188`	`/* Return the memory space pointed to - jumping over the`