From 2345ff7c881f091451d6b364240b7c94d91cbac7 Mon Sep 17 00:00:00 2001 From: AniruddhaKanhere <60444055+AniruddhaKanhere@users.noreply.github.com> Date: Tue, 7 Jul 2026 21:45:29 -0700 Subject: [PATCH 1/5] heap_1: reject configTOTAL_HEAP_SIZE <= portBYTE_ALIGNMENT at compile time Defect: pvPortMalloc() in heap_1.c can return a pointer outside the ucHeap array when configTOTAL_HEAP_SIZE is configured smaller than or equal to portBYTE_ALIGNMENT. Root cause: configADJUSTED_HEAP_SIZE is defined as ( configTOTAL_HEAP_SIZE - portBYTE_ALIGNMENT ). When configTOTAL_HEAP_SIZE is not larger than portBYTE_ALIGNMENT this unsigned subtraction underflows to a very large size_t value. The "enough room left" check in pvPortMalloc() compares an unsigned index against configADJUSTED_HEAP_SIZE, so the underflowed value defeats the check and an allocation can be handed out past the end of the heap array. Fix: add a compile-time (compiler, not preprocessor) size check so a nonsensical, too-small heap is rejected during the build. The compiler-level check still works when configTOTAL_HEAP_SIZE is defined with a cast such as ( ( size_t ) 0x2000 ). A host regression test kept outside this repository demonstrates the fault before the change and its absence afterwards (red then green). --- portable/MemMang/heap_1.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/portable/MemMang/heap_1.c b/portable/MemMang/heap_1.c index f697c907ca9..6e3b6e47bf1 100644 --- a/portable/MemMang/heap_1.c +++ b/portable/MemMang/heap_1.c @@ -53,6 +53,17 @@ /* A few bytes might be lost to byte aligning the heap start address. */ #define configADJUSTED_HEAP_SIZE ( configTOTAL_HEAP_SIZE - portBYTE_ALIGNMENT ) +/* configTOTAL_HEAP_SIZE must be larger than portBYTE_ALIGNMENT, otherwise the + * configADJUSTED_HEAP_SIZE subtraction above underflows. The "enough room left" + * check in pvPortMalloc() compares an unsigned size_t index against + * configADJUSTED_HEAP_SIZE, so an underflowed (very large) value silently + * defeats that check and pvPortMalloc() can return a pointer outside the ucHeap + * array. Reject such a nonsensical, too-small heap at compile time. This is a + * compiler (not preprocessor) check so that it still works when + * configTOTAL_HEAP_SIZE is defined with a cast, e.g. ( ( size_t ) 0x2000 ). */ +typedef char heapCHECK_configTOTAL_HEAP_SIZE_EXCEEDS_portBYTE_ALIGNMENT + [ ( ( configTOTAL_HEAP_SIZE ) > ( portBYTE_ALIGNMENT ) ) ? 1 : -1 ]; + /* Max value that fits in a size_t type. */ #define heapSIZE_MAX ( ~( ( size_t ) 0 ) ) From 3ac4495945453e8301b63799f5550df2a9e09326 Mon Sep 17 00:00:00 2001 From: AniruddhaKanhere <60444055+AniruddhaKanhere@users.noreply.github.com> Date: Tue, 7 Jul 2026 21:45:38 -0700 Subject: [PATCH 2/5] tasks: reject stack depth whose byte size overflows size_t Defect: xTaskCreate() / prvCreateTask() can under-allocate a task stack when a caller supplies a very large uxStackDepth, leading to out-of-bounds writes when the initial stack frame is set up. Root cause: the stack is allocated as ( ( size_t ) uxStackDepth ) * sizeof( StackType_t ). If that product wraps size_t, the allocation is far smaller than requested, yet prvInitialiseNewTask() still computes the top of stack from the full uxStackDepth and writes the initial context past the end of the allocation. Fix: before allocating, reject creation when ( ( size_t ) uxStackDepth ) > ( SIZE_MAX / sizeof( StackType_t ) ), returning NULL (task not created) instead of proceeding with an under-sized buffer. A host regression test kept outside this repository demonstrates the fault before the change and its absence afterwards (red then green). --- tasks.c | 118 ++++++++++++++++++++++++++++++++------------------------ 1 file changed, 68 insertions(+), 50 deletions(-) diff --git a/tasks.c b/tasks.c index 461271fcffb..8f69fff1951 100644 --- a/tasks.c +++ b/tasks.c @@ -1653,77 +1653,95 @@ STATIC void prvAddNewTaskToReadyList( TCB_t * pxNewTCB ) PRIVILEGED_FUNCTION; { TCB_t * pxNewTCB; - /* If the stack grows down then allocate the stack then the TCB so the stack - * does not grow into the TCB. Likewise if the stack grows up then allocate - * the TCB then the stack. */ - #if ( portSTACK_GROWTH > 0 ) - { - /* Allocate space for the TCB. Where the memory comes from depends on - * the implementation of the port malloc function and whether or not static - * allocation is being used. */ - /* MISRA Ref 11.5.1 [Malloc memory assignment] */ - /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-115 */ - /* coverity[misra_c_2012_rule_11_5_violation] */ - pxNewTCB = ( TCB_t * ) pvPortMalloc( sizeof( TCB_t ) ); - - if( pxNewTCB != NULL ) + /* Guard against the stack size calculation overflowing. The stack is + * allocated as ( ( size_t ) uxStackDepth ) * sizeof( StackType_t ). If a + * caller-supplied uxStackDepth is large enough that this product wraps + * size_t, the allocation is far smaller than requested while + * prvInitialiseNewTask() still computes the top of stack from the full + * uxStackDepth, causing out-of-bounds writes when the initial stack frame + * is written. The wrap is detected by multiplying and dividing back: + * if dividing the byte size by sizeof( StackType_t ) does not recover + * uxStackDepth then the multiplication overflowed. Leave pxNewTCB NULL + * and skip allocation instead of under-allocating, so the failure is + * reported through the single return at the end of the function. */ + if( ( ( size_t ) uxStackDepth ) != ( ( ( ( size_t ) uxStackDepth ) * sizeof( StackType_t ) ) / sizeof( StackType_t ) ) ) + { + pxNewTCB = NULL; + } + else + { + /* If the stack grows down then allocate the stack then the TCB so the stack + * does not grow into the TCB. Likewise if the stack grows up then allocate + * the TCB then the stack. */ + #if ( portSTACK_GROWTH > 0 ) { - ( void ) memset( ( void * ) pxNewTCB, 0x00, sizeof( TCB_t ) ); - - /* Allocate space for the stack used by the task being created. - * The base of the stack memory stored in the TCB so the task can - * be deleted later if required. */ + /* Allocate space for the TCB. Where the memory comes from depends on + * the implementation of the port malloc function and whether or not static + * allocation is being used. */ /* MISRA Ref 11.5.1 [Malloc memory assignment] */ /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-115 */ /* coverity[misra_c_2012_rule_11_5_violation] */ - pxNewTCB->pxStack = ( StackType_t * ) pvPortMallocStack( ( ( ( size_t ) uxStackDepth ) * sizeof( StackType_t ) ) ); + pxNewTCB = ( TCB_t * ) pvPortMalloc( sizeof( TCB_t ) ); - if( pxNewTCB->pxStack == NULL ) + if( pxNewTCB != NULL ) { - /* Could not allocate the stack. Delete the allocated TCB. */ - vPortFree( pxNewTCB ); - pxNewTCB = NULL; - } - } - } - #else /* portSTACK_GROWTH */ - { - StackType_t * pxStack; + ( void ) memset( ( void * ) pxNewTCB, 0x00, sizeof( TCB_t ) ); - /* Allocate space for the stack used by the task being created. */ - /* MISRA Ref 11.5.1 [Malloc memory assignment] */ - /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-115 */ - /* coverity[misra_c_2012_rule_11_5_violation] */ - pxStack = ( StackType_t * ) pvPortMallocStack( ( ( ( size_t ) uxStackDepth ) * sizeof( StackType_t ) ) ); + /* Allocate space for the stack used by the task being created. + * The base of the stack memory stored in the TCB so the task can + * be deleted later if required. */ + /* MISRA Ref 11.5.1 [Malloc memory assignment] */ + /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-115 */ + /* coverity[misra_c_2012_rule_11_5_violation] */ + pxNewTCB->pxStack = ( StackType_t * ) pvPortMallocStack( ( ( ( size_t ) uxStackDepth ) * sizeof( StackType_t ) ) ); - if( pxStack != NULL ) + if( pxNewTCB->pxStack == NULL ) + { + /* Could not allocate the stack. Delete the allocated TCB. */ + vPortFree( pxNewTCB ); + pxNewTCB = NULL; + } + } + } + #else /* portSTACK_GROWTH */ { - /* Allocate space for the TCB. */ + StackType_t * pxStack; + + /* Allocate space for the stack used by the task being created. */ /* MISRA Ref 11.5.1 [Malloc memory assignment] */ /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-115 */ /* coverity[misra_c_2012_rule_11_5_violation] */ - pxNewTCB = ( TCB_t * ) pvPortMalloc( sizeof( TCB_t ) ); + pxStack = ( StackType_t * ) pvPortMallocStack( ( ( ( size_t ) uxStackDepth ) * sizeof( StackType_t ) ) ); - if( pxNewTCB != NULL ) + if( pxStack != NULL ) { - ( void ) memset( ( void * ) pxNewTCB, 0x00, sizeof( TCB_t ) ); + /* Allocate space for the TCB. */ + /* MISRA Ref 11.5.1 [Malloc memory assignment] */ + /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-115 */ + /* coverity[misra_c_2012_rule_11_5_violation] */ + pxNewTCB = ( TCB_t * ) pvPortMalloc( sizeof( TCB_t ) ); - /* Store the stack location in the TCB. */ - pxNewTCB->pxStack = pxStack; + if( pxNewTCB != NULL ) + { + ( void ) memset( ( void * ) pxNewTCB, 0x00, sizeof( TCB_t ) ); + + /* Store the stack location in the TCB. */ + pxNewTCB->pxStack = pxStack; + } + else + { + /* The stack cannot be used as the TCB was not created. Free + * it again. */ + vPortFreeStack( pxStack ); + } } else { - /* The stack cannot be used as the TCB was not created. Free - * it again. */ - vPortFreeStack( pxStack ); + pxNewTCB = NULL; } } - else - { - pxNewTCB = NULL; - } + #endif /* portSTACK_GROWTH */ } - #endif /* portSTACK_GROWTH */ if( pxNewTCB != NULL ) { From 5b6c2f29db006c05be302a3f7692cbca2651b666 Mon Sep 17 00:00:00 2001 From: AniruddhaKanhere <60444055+AniruddhaKanhere@users.noreply.github.com> Date: Tue, 7 Jul 2026 21:45:46 -0700 Subject: [PATCH 3/5] MSVC-MingW: bounds-check interrupt number in FromWindowsThread path Defect: vPortGenerateSimulatedInterruptFromWindowsThread() in the MSVC-MingW simulator port performs a shift by a caller-supplied interrupt number without range checking it, giving undefined behavior for out-of-range values. Root cause: the function pends an interrupt via ( 1UL << ulInterruptNumber ) into ulPendingInterrupts, but does not verify ulInterruptNumber is within the width of that variable. A value greater than or equal to portMAX_INTERRUPTS makes the shift undefined. The task-context sibling vPortGenerateSimulatedInterrupt() already performs this bounds check. Fix: gate the operation on ( ulInterruptNumber < portMAX_INTERRUPTS ) in addition to the existing xPortRunning check, mirroring the task-context sibling so both entry points are consistent. A host regression test kept outside this repository demonstrates the fault before the change and its absence afterwards (red then green). --- portable/MSVC-MingW/port.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/portable/MSVC-MingW/port.c b/portable/MSVC-MingW/port.c index 40458682b80..2c62986ae1f 100644 --- a/portable/MSVC-MingW/port.c +++ b/portable/MSVC-MingW/port.c @@ -637,7 +637,11 @@ void vPortGenerateSimulatedInterrupt( uint32_t ulInterruptNumber ) void vPortGenerateSimulatedInterruptFromWindowsThread( uint32_t ulInterruptNumber ) { - if( xPortRunning == pdTRUE ) + /* Reject out-of-range interrupt numbers before the shift below. Mirrors the + * bounds check the task-context sibling vPortGenerateSimulatedInterrupt already + * performs: ( 1UL << ulInterruptNumber ) is undefined when ulInterruptNumber is + * >= portMAX_INTERRUPTS (the width of ulPendingInterrupts). */ + if( ( xPortRunning == pdTRUE ) && ( ulInterruptNumber < portMAX_INTERRUPTS ) ) { /* Can't proceed if in a critical section as pvInterruptEventMutex won't * be available. */ From bc0572329aa1f26bdfee3d3d36b81fc147f62a9a Mon Sep 17 00:00:00 2001 From: AniruddhaKanhere <60444055+AniruddhaKanhere@users.noreply.github.com> Date: Tue, 7 Jul 2026 21:46:07 -0700 Subject: [PATCH 4/5] stream_buffer: reject required-space overflow at runtime in message send Defect: xStreamBufferSend() and xStreamBufferSendFromISR() on a message buffer can perform an out-of-bounds write when the required-space computation for a message overflows size_t and configASSERT() is compiled out. Root cause: for message buffers the code computes xRequiredSpace = xDataLengthBytes + sbBYTES_TO_STORE_MESSAGE_LENGTH and guards it only with configASSERT( xRequiredSpace > xDataLengthBytes ). When the addition wraps size_t, the wrapped (smaller) xRequiredSpace passes the subsequent space checks and the send proceeds to copy xDataLengthBytes bytes, writing past the buffer. With asserts disabled there is no guard at all. Fix: add a runtime check that returns 0 (nothing sent) when xRequiredSpace <= xDataLengthBytes, in both the task and FromISR send paths, so a wrapping message length is rejected regardless of the configASSERT() setting. A host regression test kept outside this repository demonstrates the fault before the change and its absence afterwards (red then green). --- stream_buffer.c | 49 +++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 43 insertions(+), 6 deletions(-) diff --git a/stream_buffer.c b/stream_buffer.c index a4c6d268d11..0e7478a7e15 100644 --- a/stream_buffer.c +++ b/stream_buffer.c @@ -820,10 +820,12 @@ size_t xStreamBufferSend( StreamBufferHandle_t xStreamBuffer, TickType_t xTicksToWait ) { StreamBuffer_t * const pxStreamBuffer = xStreamBuffer; - size_t xReturn, xSpace = 0; + size_t xReturn = 0; + size_t xSpace = 0; size_t xRequiredSpace = xDataLengthBytes; TimeOut_t xTimeOut; size_t xMaxReportedSpace = 0; + BaseType_t xSendAllowed = pdTRUE; traceENTER_xStreamBufferSend( xStreamBuffer, pvTxData, xDataLengthBytes, xTicksToWait ); @@ -845,9 +847,22 @@ size_t xStreamBufferSend( StreamBufferHandle_t xStreamBuffer, /* Overflow? */ configASSERT( xRequiredSpace > xDataLengthBytes ); + /* Enforce the overflow check at runtime as well, so that a message + * length whose required-space addition wraps size_t is rejected even + * when configASSERT() is compiled out. Without this the wrapped + * (smaller) xRequiredSpace would pass the space checks below and the + * send would proceed to copy xDataLengthBytes bytes, causing an + * out-of-bounds write. Reject the send instead: leave xReturn 0, skip + * blocking and the write, and exit through the single return below. */ + if( xRequiredSpace <= xDataLengthBytes ) + { + xSendAllowed = pdFALSE; + xTicksToWait = ( TickType_t ) 0; + } + /* If this is a message buffer then it must be possible to write the * whole message. */ - if( xRequiredSpace > xMaxReportedSpace ) + else if( xRequiredSpace > xMaxReportedSpace ) { /* The message would not fit even if the entire buffer was empty, * so don't wait for space. */ @@ -921,7 +936,10 @@ size_t xStreamBufferSend( StreamBufferHandle_t xStreamBuffer, mtCOVERAGE_TEST_MARKER(); } - xReturn = prvWriteMessageToBuffer( pxStreamBuffer, pvTxData, xDataLengthBytes, xSpace, xRequiredSpace ); + if( xSendAllowed == pdTRUE ) + { + xReturn = prvWriteMessageToBuffer( pxStreamBuffer, pvTxData, xDataLengthBytes, xSpace, xRequiredSpace ); + } if( xReturn > ( size_t ) 0 ) { @@ -955,8 +973,10 @@ size_t xStreamBufferSendFromISR( StreamBufferHandle_t xStreamBuffer, BaseType_t * const pxHigherPriorityTaskWoken ) { StreamBuffer_t * const pxStreamBuffer = xStreamBuffer; - size_t xReturn, xSpace; + size_t xReturn = 0; + size_t xSpace; size_t xRequiredSpace = xDataLengthBytes; + BaseType_t xSendAllowed = pdTRUE; traceENTER_xStreamBufferSendFromISR( xStreamBuffer, pvTxData, xDataLengthBytes, pxHigherPriorityTaskWoken ); @@ -973,14 +993,31 @@ size_t xStreamBufferSendFromISR( StreamBufferHandle_t xStreamBuffer, /* Overflow? */ configASSERT( xRequiredSpace > xDataLengthBytes ); + + /* Enforce the overflow check at runtime as well (see xStreamBufferSend), + * so a wrapping message length is rejected when configASSERT() is + * compiled out rather than proceeding to an out-of-bounds write. Reject + * the send by leaving xReturn 0 and skipping the write below, so the + * function still exits through its single return. */ + if( xRequiredSpace <= xDataLengthBytes ) + { + xSendAllowed = pdFALSE; + } + else + { + mtCOVERAGE_TEST_MARKER(); + } } else { mtCOVERAGE_TEST_MARKER(); } - xSpace = xStreamBufferSpacesAvailable( pxStreamBuffer ); - xReturn = prvWriteMessageToBuffer( pxStreamBuffer, pvTxData, xDataLengthBytes, xSpace, xRequiredSpace ); + if( xSendAllowed == pdTRUE ) + { + xSpace = xStreamBufferSpacesAvailable( pxStreamBuffer ); + xReturn = prvWriteMessageToBuffer( pxStreamBuffer, pvTxData, xDataLengthBytes, xSpace, xRequiredSpace ); + } if( xReturn > ( size_t ) 0 ) { From e2b6581cf39367d089449ac93d83234170564a91 Mon Sep 17 00:00:00 2001 From: AniruddhaKanhere <60444055+AniruddhaKanhere@users.noreply.github.com> Date: Tue, 7 Jul 2026 21:46:35 -0700 Subject: [PATCH 5/5] stream_buffer: reject message length truncation at runtime in writer Defect: prvWriteMessageToBuffer() can write a truncated message-length header into a message buffer when the data length does not fit the configured configMESSAGE_BUFFER_LENGTH_TYPE and configASSERT() is compiled out, desynchronizing framing at the receiver. Root cause: the writer stores the length as a configMESSAGE_BUFFER_LENGTH_TYPE value xMessageLength and only guards the "fits without truncation" condition with configASSERT( ( size_t ) xMessageLength == xDataLengthBytes ). The subsequent write was gated solely on available space, so with asserts disabled a length that overflows the length type is written truncated while the full data is copied, corrupting the message framing seen by the receiver. Fix: gate the length-plus-data write on ( ( size_t ) xMessageLength == xDataLengthBytes ) in addition to the space check. When the length cannot be represented without truncation, take the "not enough space" path and write nothing. This runtime check is the only guard against truncation when asserts are disabled. A host regression test kept outside this repository demonstrates the fault before the change and its absence afterwards (red then green). --- stream_buffer.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/stream_buffer.c b/stream_buffer.c index 0e7478a7e15..087838c5ec8 100644 --- a/stream_buffer.c +++ b/stream_buffer.c @@ -1065,16 +1065,21 @@ static size_t prvWriteMessageToBuffer( StreamBuffer_t * const pxStreamBuffer, /* Ensure the data length given fits within configMESSAGE_BUFFER_LENGTH_TYPE. */ configASSERT( ( size_t ) xMessageLength == xDataLengthBytes ); - if( xSpace >= xRequiredSpace ) + if( ( ( size_t ) xMessageLength == xDataLengthBytes ) && ( xSpace >= xRequiredSpace ) ) { - /* There is enough space to write both the message length and the message + /* The message length fits within configMESSAGE_BUFFER_LENGTH_TYPE and + * there is enough space to write both the message length and the message * itself into the buffer. Start by writing the length of the data, the data * itself will be written later in this function. */ xNextHead = prvWriteBytesToBuffer( pxStreamBuffer, ( const uint8_t * ) &( xMessageLength ), sbBYTES_TO_STORE_MESSAGE_LENGTH, xNextHead ); } else { - /* Not enough space, so do not write data to the buffer. */ + /* Either there is not enough space, or the message length cannot be + * represented by configMESSAGE_BUFFER_LENGTH_TYPE without truncation. + * Writing a truncated length header would corrupt the message + * framing at the receiver, so do not write any data to the buffer. When asserts are + * disabled this runtime check is the only guard against truncation. */ xDataLengthBytes = 0; } }