Skip to content

Check for add overflow only once #467

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 17 additions & 18 deletions portable/MemMang/heap_2.c
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,14 @@
/* Assumes 8bit bytes! */
#define heapBITS_PER_BYTE ( ( size_t ) 8 )

/* Max value that fits in a size_t type. */
#define heapSIZE_MAX ( ~( ( size_t ) 0 ) )

/* Check if multiplying a and b will result in overflow. */
#define heapMULTIPLY_WILL_OVERFLOW( a, b, max ) ( ( ( a ) > 0 ) && ( ( b ) > ( ( max ) / ( a ) ) ) )
#define heapMULTIPLY_WILL_OVERFLOW( a, b ) ( ( ( a ) > 0 ) && ( ( b ) > ( heapSIZE_MAX / ( a ) ) ) )

/* Check if adding a and b will result in overflow. */
#define heapADD_WILL_OVERFLOW( a, b ) ( ( a ) > ( heapSIZE_MAX - ( b ) ) )

/* MSB of the xBlockSize member of an BlockLink_t structure is used to track
* the allocation status of a block. When MSB of the xBlockSize member of
Expand Down Expand Up @@ -97,7 +103,7 @@ typedef struct A_BLOCK_LINK
} BlockLink_t;


static const uint16_t heapSTRUCT_SIZE = ( ( sizeof( BlockLink_t ) + ( portBYTE_ALIGNMENT - 1 ) ) & ~portBYTE_ALIGNMENT_MASK );
static const uint16_t heapSTRUCT_SIZE = ( ( sizeof( BlockLink_t ) + ( portBYTE_ALIGNMENT - 1 ) ) & ~( ( size_t ) portBYTE_ALIGNMENT_MASK ) );
#define heapMINIMUM_BLOCK_SIZE ( ( size_t ) ( heapSTRUCT_SIZE * 2 ) )

/* Create a couple of list links to mark the start and end of the list. */
Expand Down Expand Up @@ -149,6 +155,7 @@ void * pvPortMalloc( size_t xWantedSize )
BlockLink_t * pxBlock, * pxPreviousBlock, * pxNewBlockLink;
PRIVILEGED_DATA static BaseType_t xHeapHasBeenInitialised = pdFALSE;
void * pvReturn = NULL;
size_t xAdditionalRequiredSize;

vTaskSuspendAll();
{
Expand All @@ -160,29 +167,22 @@ void * pvPortMalloc( size_t xWantedSize )
xHeapHasBeenInitialised = pdTRUE;
}

/* 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 ) > xWantedSize ) ) /* Overflow check. */
if( xWantedSize > 0 )
{
xWantedSize += heapSTRUCT_SIZE;
/* The wanted size must be increased so it can contain a BlockLink_t
* structure in addition to the requested amount of bytes. Some
* additional increment may also be needed for alignment. */
xAdditionalRequiredSize = heapSTRUCT_SIZE + portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK );

/* Byte alignment required. Check for overflow. */
if( ( xWantedSize + ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) ) )
> xWantedSize )
if( heapADD_WILL_OVERFLOW( xWantedSize, xAdditionalRequiredSize ) == 0 )
{
xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );
configASSERT( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) == 0 );
xWantedSize += xAdditionalRequiredSize;
}
else
{
xWantedSize = 0;
}
}
else
{
xWantedSize = 0;
}

/* Check the block size we are trying to allocate is not so large that the
* top bit is set. The top bit of the block size member of the BlockLink_t
Expand Down Expand Up @@ -320,9 +320,8 @@ void * pvPortCalloc( size_t xNum,
size_t xSize )
{
void * pv = NULL;
const size_t xSizeMaxValue = ~( ( size_t ) 0 );

if( !heapMULTIPLY_WILL_OVERFLOW( xNum, xSize, xSizeMaxValue ) )
if( heapMULTIPLY_WILL_OVERFLOW( xNum, xSize ) == 0 )
{
pv = pvPortMalloc( xNum * xSize );

Expand Down
40 changes: 18 additions & 22 deletions portable/MemMang/heap_4.c
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,14 @@
/* Assumes 8bit bytes! */
#define heapBITS_PER_BYTE ( ( size_t ) 8 )

/* Max value that fits in a size_t type. */
#define heapSIZE_MAX ( ~( ( size_t ) 0 ) )

/* Check if multiplying a and b will result in overflow. */
#define heapMULTIPLY_WILL_OVERFLOW( a, b, max ) ( ( ( a ) > 0 ) && ( ( b ) > ( ( max ) / ( a ) ) ) )
#define heapMULTIPLY_WILL_OVERFLOW( a, b ) ( ( ( a ) > 0 ) && ( ( b ) > ( heapSIZE_MAX / ( a ) ) ) )

/* Check if adding a and b will result in overflow. */
#define heapADD_WILL_OVERFLOW( a, b ) ( ( a ) > ( heapSIZE_MAX - ( b ) ) )

/* MSB of the xBlockSize member of an BlockLink_t structure is used to track
* the allocation status of a block. When MSB of the xBlockSize member of
Expand Down Expand Up @@ -132,6 +138,7 @@ void * pvPortMalloc( size_t xWantedSize )
{
BlockLink_t * pxBlock, * pxPreviousBlock, * pxNewBlockLink;
void * pvReturn = NULL;
size_t xAdditionalRequiredSize;

vTaskSuspendAll();
{
Expand All @@ -146,35 +153,25 @@ void * pvPortMalloc( size_t xWantedSize )
mtCOVERAGE_TEST_MARKER();
}

/* 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 + xHeapStructSize ) > xWantedSize ) ) /* Overflow check. */
if( xWantedSize > 0 )
{
xWantedSize += xHeapStructSize;
/* The wanted size must be increased so it can contain a BlockLink_t
* structure in addition to the requested amount of bytes. Some
* additional increment may also be needed for alignment. */
xAdditionalRequiredSize = xHeapStructSize + portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK );

/* Ensure that blocks are always aligned. */
if( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) != 0x00 )
if( heapADD_WILL_OVERFLOW( xWantedSize, xAdditionalRequiredSize ) == 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;
}
xWantedSize += xAdditionalRequiredSize;
}
else
{
mtCOVERAGE_TEST_MARKER();
xWantedSize = 0;
}
}
else
{
xWantedSize = 0;
mtCOVERAGE_TEST_MARKER();
}

/* Check the block size we are trying to allocate is not so large that the
Expand Down Expand Up @@ -362,9 +359,8 @@ void * pvPortCalloc( size_t xNum,
size_t xSize )
{
void * pv = NULL;
const size_t xSizeMaxValue = ~( ( size_t ) 0 );

if( !heapMULTIPLY_WILL_OVERFLOW( xNum, xSize, xSizeMaxValue ) )
if( heapMULTIPLY_WILL_OVERFLOW( xNum, xSize ) == 0 )
{
pv = pvPortMalloc( xNum * xSize );

Expand Down
42 changes: 19 additions & 23 deletions portable/MemMang/heap_5.c
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,14 @@
/* Assumes 8bit bytes! */
#define heapBITS_PER_BYTE ( ( size_t ) 8 )

/* Max value that fits in a size_t type. */
#define heapSIZE_MAX ( ~( ( size_t ) 0 ) )

/* Check if multiplying a and b will result in overflow. */
#define heapMULTIPLY_WILL_OVERFLOW( a, b, max ) ( ( ( a ) > 0 ) && ( ( b ) > ( ( max ) / ( a ) ) ) )
#define heapMULTIPLY_WILL_OVERFLOW( a, b ) ( ( ( a ) > 0 ) && ( ( b ) > ( heapSIZE_MAX / ( a ) ) ) )

/* Check if adding a and b will result in overflow. */
#define heapADD_WILL_OVERFLOW( a, b ) ( ( a ) > ( heapSIZE_MAX - ( b ) ) )

/* MSB of the xBlockSize member of an BlockLink_t structure is used to track
* the allocation status of a block. When MSB of the xBlockSize member of
Expand Down Expand Up @@ -150,42 +156,33 @@ void * pvPortMalloc( size_t xWantedSize )
{
BlockLink_t * pxBlock, * pxPreviousBlock, * pxNewBlockLink;
void * pvReturn = NULL;
size_t xAdditionalRequiredSize;

/* The heap must be initialised before the first call to
* prvPortMalloc(). */
configASSERT( pxEnd );

vTaskSuspendAll();
{
/* The wanted size is increased so it can contain a BlockLink_t
* structure in addition to the requested amount of bytes. */
if( ( xWantedSize > 0 ) &&
( ( xWantedSize + xHeapStructSize ) > xWantedSize ) ) /* Overflow check. */
if( xWantedSize > 0 )
{
xWantedSize += xHeapStructSize;
/* The wanted size must be increased so it can contain a BlockLink_t
* structure in addition to the requested amount of bytes. Some
* additional increment may also be needed for alignment. */
xAdditionalRequiredSize = xHeapStructSize + portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK );

/* Ensure that blocks are always aligned */
if( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) != 0x00 )
if( heapADD_WILL_OVERFLOW( xWantedSize, xAdditionalRequiredSize ) == 0 )
{
/* 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;
}
xWantedSize += xAdditionalRequiredSize;
}
else
{
mtCOVERAGE_TEST_MARKER();
xWantedSize = 0;
}
}
else
{
xWantedSize = 0;
mtCOVERAGE_TEST_MARKER();
}

/* Check the block size we are trying to allocate is not so large that the
Expand Down Expand Up @@ -365,9 +362,8 @@ void * pvPortCalloc( size_t xNum,
size_t xSize )
{
void * pv = NULL;
const size_t xSizeMaxValue = ~( ( size_t ) 0 );

if( !heapMULTIPLY_WILL_OVERFLOW( xNum, xSize, xSizeMaxValue ) )
if( heapMULTIPLY_WILL_OVERFLOW( xNum, xSize ) == 0 )
{
pv = pvPortMalloc( xNum * xSize );

Expand Down Expand Up @@ -502,7 +498,7 @@ void vPortDefineHeapRegions( const HeapRegion_t * const pxHeapRegions )
* inserted at the end of the region space. */
xAddress = xAlignedHeap + xTotalRegionSize;
xAddress -= xHeapStructSize;
xAddress &= ~portBYTE_ALIGNMENT_MASK;
xAddress &= ~( ( size_t ) portBYTE_ALIGNMENT_MASK );
pxEnd = ( BlockLink_t * ) xAddress;
pxEnd->xBlockSize = 0;
pxEnd->pxNextFreeBlock = NULL;
Expand Down