From 47d94075be512a492d3b5d74883e3d4d5a3b8665 Mon Sep 17 00:00:00 2001 From: Sudeep Mohanty Date: Fri, 6 Dec 2024 10:41:54 +0530 Subject: [PATCH 1/3] fix(freertos): Limit idle task name copy operation and ensure null termination This commit: - Limits the idle task name length copy operation to prevent Out-of-bounds memory access warnings from static code analyzers. - Fixes a bug where in the idle task name could be non null-terminated string for SMP configuration. Signed-off-by: Sudeep Mohanty --- tasks.c | 51 ++++++++++++++++++--------------------------------- 1 file changed, 18 insertions(+), 33 deletions(-) diff --git a/tasks.c b/tasks.c index 22e11f0fb21..04dfd4dc537 100644 --- a/tasks.c +++ b/tasks.c @@ -3521,27 +3521,28 @@ static BaseType_t prvCreateIdleTasks( void ) { BaseType_t xReturn = pdPASS; BaseType_t xCoreID; - char cIdleName[ configMAX_TASK_NAME_LEN ]; + char cIdleName[ configMAX_TASK_NAME_LEN ] = { 0 }; TaskFunction_t pxIdleTaskFunction = NULL; BaseType_t xIdleTaskNameIndex; + BaseType_t xIdleNameLen; + BaseType_t xCopyLen; - for( xIdleTaskNameIndex = ( BaseType_t ) 0; xIdleTaskNameIndex < ( BaseType_t ) configMAX_TASK_NAME_LEN; xIdleTaskNameIndex++ ) + configASSERT( configIDLE_TASK_NAME != NULL && configMAX_TASK_NAME_LEN > 3 ); + + /* The length of the idle task name is limited to the minimum of the length + * of configIDLE_TASK_NAME and configMAX_TASK_NAME_LEN - 2, keeping space + * for the core ID suffix and the null-terminator. */ + xIdleNameLen = sizeof( configIDLE_TASK_NAME ) - 1; + xCopyLen = ( xIdleNameLen < configMAX_TASK_NAME_LEN - 2 ) ? xIdleNameLen : configMAX_TASK_NAME_LEN - 2; + + for( xIdleTaskNameIndex = ( BaseType_t ) 0; xIdleTaskNameIndex < xCopyLen; xIdleTaskNameIndex++ ) { cIdleName[ xIdleTaskNameIndex ] = configIDLE_TASK_NAME[ xIdleTaskNameIndex ]; - - /* Don't copy all configMAX_TASK_NAME_LEN if the string is shorter than - * configMAX_TASK_NAME_LEN characters just in case the memory after the - * string is not accessible (extremely unlikely). */ - if( cIdleName[ xIdleTaskNameIndex ] == ( char ) 0x00 ) - { - break; - } - else - { - mtCOVERAGE_TEST_MARKER(); - } } + /* Ensure null termination. */ + cIdleName[ xIdleTaskNameIndex ] = '\0'; + /* Add each idle task at the lowest priority. */ for( xCoreID = ( BaseType_t ) 0; xCoreID < ( BaseType_t ) configNUMBER_OF_CORES; xCoreID++ ) { @@ -3570,25 +3571,9 @@ static BaseType_t prvCreateIdleTasks( void ) * only one idle task. */ #if ( configNUMBER_OF_CORES > 1 ) { - /* Append the idle task number to the end of the name if there is space. */ - if( xIdleTaskNameIndex < ( BaseType_t ) configMAX_TASK_NAME_LEN ) - { - cIdleName[ xIdleTaskNameIndex ] = ( char ) ( xCoreID + '0' ); - - /* And append a null character if there is space. */ - if( ( xIdleTaskNameIndex + 1 ) < ( BaseType_t ) configMAX_TASK_NAME_LEN ) - { - cIdleName[ xIdleTaskNameIndex + 1 ] = '\0'; - } - else - { - mtCOVERAGE_TEST_MARKER(); - } - } - else - { - mtCOVERAGE_TEST_MARKER(); - } + /* Append the idle task number to the end of the name. */ + cIdleName[ xIdleTaskNameIndex ] = ( char ) ( xCoreID + '0' ); + cIdleName[ xIdleTaskNameIndex + 1 ] = '\0'; } #endif /* if ( configNUMBER_OF_CORES > 1 ) */ From 2dfe9e683250dd6d57e19f2b79b458cc2be5e2c0 Mon Sep 17 00:00:00 2001 From: Gaurav Aggarwal Date: Thu, 19 Dec 2024 16:29:34 +0000 Subject: [PATCH 2/3] Minor code review suggestions Signed-off-by: Gaurav Aggarwal --- tasks.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tasks.c b/tasks.c index 5600f4a9323..013694dd340 100644 --- a/tasks.c +++ b/tasks.c @@ -3530,13 +3530,13 @@ static BaseType_t prvCreateIdleTasks( void ) BaseType_t xIdleNameLen; BaseType_t xCopyLen; - configASSERT( configIDLE_TASK_NAME != NULL && configMAX_TASK_NAME_LEN > 3 ); + configASSERT( ( configIDLE_TASK_NAME != NULL ) && ( configMAX_TASK_NAME_LEN > 3 ) ); /* The length of the idle task name is limited to the minimum of the length * of configIDLE_TASK_NAME and configMAX_TASK_NAME_LEN - 2, keeping space * for the core ID suffix and the null-terminator. */ xIdleNameLen = sizeof( configIDLE_TASK_NAME ) - 1; - xCopyLen = ( xIdleNameLen < configMAX_TASK_NAME_LEN - 2 ) ? xIdleNameLen : configMAX_TASK_NAME_LEN - 2; + xCopyLen = xIdleNameLen < ( configMAX_TASK_NAME_LEN - 2 ) ? xIdleNameLen : ( configMAX_TASK_NAME_LEN - 2 ); for( xIdleTaskNameIndex = ( BaseType_t ) 0; xIdleTaskNameIndex < xCopyLen; xIdleTaskNameIndex++ ) { From 8dc4c8338aa1ed295ae523130bbc95bfa7019692 Mon Sep 17 00:00:00 2001 From: Gaurav Aggarwal Date: Thu, 19 Dec 2024 18:36:05 +0000 Subject: [PATCH 3/3] Use strlen instead of sizeof. This allows using pointer to string for configIDLE_TASK_NAME. Coverage tests do that. Signed-off-by: Gaurav Aggarwal --- tasks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tasks.c b/tasks.c index 013694dd340..f6af37bab8a 100644 --- a/tasks.c +++ b/tasks.c @@ -3535,7 +3535,7 @@ static BaseType_t prvCreateIdleTasks( void ) /* The length of the idle task name is limited to the minimum of the length * of configIDLE_TASK_NAME and configMAX_TASK_NAME_LEN - 2, keeping space * for the core ID suffix and the null-terminator. */ - xIdleNameLen = sizeof( configIDLE_TASK_NAME ) - 1; + xIdleNameLen = strlen( configIDLE_TASK_NAME ); xCopyLen = xIdleNameLen < ( configMAX_TASK_NAME_LEN - 2 ) ? xIdleNameLen : ( configMAX_TASK_NAME_LEN - 2 ); for( xIdleTaskNameIndex = ( BaseType_t ) 0; xIdleTaskNameIndex < xCopyLen; xIdleTaskNameIndex++ )