From 357913bb8fce28f1a854d1e636454a1a9e83cf30 Mon Sep 17 00:00:00 2001 From: 94xhn <87560781+94xhn@users.noreply.github.com> Date: Sun, 12 Jul 2026 02:04:06 +0800 Subject: [PATCH] Fix unreachable trigger level validation for stream batching buffers A stream batching buffer only unblocks a waiting reader when the number of bytes in the buffer becomes strictly greater than its trigger level (prvBytesInBufferMeetTriggerLevel), unlike a plain stream/message buffer which unblocks when the byte count is greater than or equal to the trigger level. The maximum number of bytes any of these buffers can ever hold is one less than their internal length (a full/empty ring buffer ambiguity). The trigger-level validation in xStreamBufferGenericCreate(), xStreamBufferGenericCreateStatic() and xStreamBufferSetTriggerLevel() did not account for this difference: it accepted a trigger level equal to the buffer's maximum achievable occupancy for batching buffers too. Once such a trigger level was set, the buffer could never satisfy its own wake predicate, so a reader blocked in xStreamBufferReceive() would hang forever even after the buffer was completely filled. Add a batching-buffer-specific bound at all three validation sites so the trigger level is rejected (via configASSERT, or pdFALSE from xStreamBufferSetTriggerLevel) when it can never be exceeded. Plain stream and message buffers are unaffected. Verified by compiling the affected tasks.c/list.c/stream_buffer.c against the POSIX/GCC simulator port and running real FreeRTOS tasks: before the fix, a batching buffer created with xStreamBatchingBufferCreate(25, 25) hangs a reader forever even when the writer fills all 25 bytes; after the fix, the same call is rejected by configASSERT at creation time, while the correct boundary (trigger = 24) and plain stream/message buffers keep working exactly as before. Signed-off-by: 94xhn <94xhn1@gmail.com> Signed-off-by: 94xhn <87560781+94xhn@users.noreply.github.com> --- stream_buffer.c | 76 ++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 65 insertions(+), 11 deletions(-) diff --git a/stream_buffer.c b/stream_buffer.c index a4c6d268d11..72f9f0386f1 100644 --- a/stream_buffer.c +++ b/stream_buffer.c @@ -371,7 +371,26 @@ static void prvInitialiseNewStreamBuffer( StreamBuffer_t * const pxStreamBuffer, configASSERT( xBufferSizeBytes > 0 ); } - configASSERT( xTriggerLevelBytes <= xBufferSizeBytes ); + /* A batching buffer only unblocks a reader when the number of bytes + * in the buffer becomes strictly greater than the trigger level (see + * prvBytesInBufferMeetTriggerLevel()), whereas a stream/message + * buffer unblocks a reader when the number of bytes is greater than + * or equal to the trigger level. The maximum number of bytes any of + * these buffers can ever hold is one less than their length (a + * full/empty ring buffer ambiguity - see prvBytesInBuffer()), which + * is compensated for non-batching buffers so xBufferSizeBytes bytes + * can be used (see the xBufferSizeBytes++ below). A batching + * buffer's trigger level must therefore be strictly less than + * xBufferSizeBytes, otherwise it could never be satisfied even when + * the buffer is completely full. */ + if( ( ucFlags & sbFLAGS_IS_BATCHING_BUFFER ) != ( uint8_t ) 0 ) + { + configASSERT( xTriggerLevelBytes < xBufferSizeBytes ); + } + else + { + configASSERT( xTriggerLevelBytes <= xBufferSizeBytes ); + } /* A trigger level of 0 would cause a waiting task to unblock even when * the buffer was empty. */ @@ -452,14 +471,6 @@ static void prvInitialiseNewStreamBuffer( StreamBuffer_t * const pxStreamBuffer, configASSERT( pucStreamBufferStorageArea ); configASSERT( pxStaticStreamBuffer ); - configASSERT( xTriggerLevelBytes <= xBufferSizeBytes ); - - /* A trigger level of 0 would cause a waiting task to unblock even when - * the buffer was empty. */ - if( xTriggerLevelBytes == ( size_t ) 0 ) - { - xTriggerLevelBytes = ( size_t ) 1; - } /* In case the stream buffer is going to be used as a message buffer * (that is, it will hold discrete messages with a little meta data that @@ -484,6 +495,31 @@ static void prvInitialiseNewStreamBuffer( StreamBuffer_t * const pxStreamBuffer, ucFlags = sbFLAGS_IS_STATICALLY_ALLOCATED; } + /* Unlike the dynamically allocated path, a statically allocated + * buffer's xLength is exactly xBufferSizeBytes (there is no "+1" + * compensation - see xStreamBufferGenericCreate()), so the maximum + * number of bytes it can ever hold is (xBufferSizeBytes - 1). A + * batching buffer only unblocks a reader when the number of bytes in + * the buffer becomes strictly greater than the trigger level (see + * prvBytesInBufferMeetTriggerLevel()), so its trigger level must be + * strictly less than that maximum, otherwise it could never be + * satisfied even when the buffer is completely full. */ + if( ( ucFlags & sbFLAGS_IS_BATCHING_BUFFER ) != ( uint8_t ) 0 ) + { + configASSERT( xTriggerLevelBytes < ( xBufferSizeBytes - 1U ) ); + } + else + { + configASSERT( xTriggerLevelBytes <= xBufferSizeBytes ); + } + + /* A trigger level of 0 would cause a waiting task to unblock even when + * the buffer was empty. */ + if( xTriggerLevelBytes == ( size_t ) 0 ) + { + xTriggerLevelBytes = ( size_t ) 1; + } + #if ( configASSERT_DEFINED == 1 ) { /* Sanity check that the size of the structure used to declare a @@ -743,8 +779,26 @@ BaseType_t xStreamBufferSetTriggerLevel( StreamBufferHandle_t xStreamBuffer, } /* The trigger level is the number of bytes that must be in the stream - * buffer before a task that is waiting for data is unblocked. */ - if( xTriggerLevel < pxStreamBuffer->xLength ) + * buffer before a task that is waiting for data is unblocked. A batching + * buffer only unblocks a reader when the number of bytes in the buffer + * becomes strictly greater than the trigger level (see + * prvBytesInBufferMeetTriggerLevel()), so its trigger level must be kept + * strictly less than the maximum number of bytes the buffer can ever hold + * (xLength - 1), otherwise it could never be satisfied even when the + * buffer is completely full. */ + if( ( pxStreamBuffer->ucFlags & sbFLAGS_IS_BATCHING_BUFFER ) != ( uint8_t ) 0 ) + { + if( xTriggerLevel < ( pxStreamBuffer->xLength - 1U ) ) + { + pxStreamBuffer->xTriggerLevelBytes = xTriggerLevel; + xReturn = pdPASS; + } + else + { + xReturn = pdFALSE; + } + } + else if( xTriggerLevel < pxStreamBuffer->xLength ) { pxStreamBuffer->xTriggerLevelBytes = xTriggerLevel; xReturn = pdPASS;