From b7e37b6ca81085754de3abab0467c5c5d9954fbf Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Mon, 23 Mar 2026 19:01:52 +0800 Subject: [PATCH] Fix reporter wraparound and add bounds checks Reporter tolerance check: (average - 1) wraps to ULONG_MAX when average is 0 (first cycle or short TM_TEST_DURATION runs), producing spurious ERROR messages. Guard with average > 0 before the check. Reporter snapshot: volatile counters were read multiple times per report cycle: once for the total sum, again for each per-counter tolerance comparison. Workers keep incrementing between reads, making the check self-inconsistent. Snapshot all counters into locals in a single pass, then compute total/average/tolerance from the snapshot. Porting layer bounds checks: all 13 API entry points in both ThreadX and FreeRTOS ports now validate thread_id, queue_id, semaphore_id, pool_id, and priority range, returning TM_ERROR for out-of-bounds values instead of silently indexing past array ends. --- ports/freertos/tm_port.c | 45 +++++++++++++++++++++++++-- ports/threadx/tm_port.c | 26 ++++++++++++++++ src/cooperative_scheduling.c | 34 +++++++++++--------- src/interrupt_preemption_processing.c | 29 ++++++++--------- src/interrupt_processing.c | 23 ++++++++------ src/preemptive_scheduling.c | 35 +++++++++++---------- 6 files changed, 135 insertions(+), 57 deletions(-) diff --git a/ports/freertos/tm_port.c b/ports/freertos/tm_port.c index 1067fee..c8d70a8 100644 --- a/ports/freertos/tm_port.c +++ b/ports/freertos/tm_port.c @@ -165,6 +165,10 @@ int tm_thread_create(int thread_id, int priority, void (*entry_function)(void)) BaseType_t status; UBaseType_t freertos_prio; + if (thread_id < 0 || thread_id >= TM_FREERTOS_MAX_THREADS || priority < 1 || + priority > 31) + return TM_ERROR; + /* Invert priority: TM 1 (highest) -> configMAX_PRIORITIES-2, * TM 31 (lowest) -> 0. Reserve configMAX_PRIORITIES-1 for ISR. */ @@ -189,6 +193,9 @@ int tm_thread_create(int thread_id, int priority, void (*entry_function)(void)) int tm_thread_resume(int thread_id) { + if (thread_id < 0 || thread_id >= TM_FREERTOS_MAX_THREADS) + return TM_ERROR; + #if defined(__arm__) && !defined(TM_ISR_VIA_THREAD) /* Cortex-M: detect ISR context for ISR-safe resume. */ if (xPortIsInsideInterrupt()) { @@ -204,6 +211,9 @@ int tm_thread_resume(int thread_id) int tm_thread_suspend(int thread_id) { + if (thread_id < 0 || thread_id >= TM_FREERTOS_MAX_THREADS) + return TM_ERROR; + vTaskSuspend(tm_thread_array[thread_id]); return TM_SUCCESS; } @@ -223,6 +233,9 @@ void tm_thread_sleep(int seconds) int tm_queue_create(int queue_id) { + if (queue_id < 0 || queue_id >= TM_FREERTOS_MAX_QUEUES) + return TM_ERROR; + tm_queue_array[queue_id] = xQueueCreate(TM_FREERTOS_QUEUE_DEPTH, TM_FREERTOS_QUEUE_MSG_SIZE); @@ -234,6 +247,9 @@ int tm_queue_create(int queue_id) int tm_queue_send(int queue_id, unsigned long *message_ptr) { + if (queue_id < 0 || queue_id >= TM_FREERTOS_MAX_QUEUES) + return TM_ERROR; + if (xQueueSendToBack(tm_queue_array[queue_id], (const void *) message_ptr, 0) != pdTRUE) return TM_ERROR; @@ -243,6 +259,9 @@ int tm_queue_send(int queue_id, unsigned long *message_ptr) int tm_queue_receive(int queue_id, unsigned long *message_ptr) { + if (queue_id < 0 || queue_id >= TM_FREERTOS_MAX_QUEUES) + return TM_ERROR; + if (xQueueReceive(tm_queue_array[queue_id], (void *) message_ptr, 0) != pdTRUE) return TM_ERROR; @@ -255,6 +274,9 @@ int tm_queue_receive(int queue_id, unsigned long *message_ptr) int tm_semaphore_create(int semaphore_id) { + if (semaphore_id < 0 || semaphore_id >= TM_FREERTOS_MAX_SEMAPHORES) + return TM_ERROR; + tm_semaphore_array[semaphore_id] = xSemaphoreCreateBinary(); if (tm_semaphore_array[semaphore_id] == NULL) @@ -268,6 +290,9 @@ int tm_semaphore_create(int semaphore_id) int tm_semaphore_get(int semaphore_id) { + if (semaphore_id < 0 || semaphore_id >= TM_FREERTOS_MAX_SEMAPHORES) + return TM_ERROR; + if (xSemaphoreTake(tm_semaphore_array[semaphore_id], 0) != pdTRUE) return TM_ERROR; @@ -276,6 +301,9 @@ int tm_semaphore_get(int semaphore_id) int tm_semaphore_put(int semaphore_id) { + if (semaphore_id < 0 || semaphore_id >= TM_FREERTOS_MAX_SEMAPHORES) + return TM_ERROR; + #if defined(__arm__) && !defined(TM_ISR_VIA_THREAD) if (xPortIsInsideInterrupt()) { BaseType_t yield = pdFALSE; @@ -298,7 +326,12 @@ int tm_semaphore_put(int semaphore_id) int tm_memory_pool_create(int pool_id) { int i; - unsigned char *base = tm_pool_area[pool_id]; + unsigned char *base; + + if (pool_id < 0 || pool_id >= TM_FREERTOS_MAX_POOLS) + return TM_ERROR; + + base = tm_pool_area[pool_id]; /* Build freelist: each block stores a next pointer. */ tm_pool_free[pool_id] = (void *) base; @@ -318,7 +351,12 @@ int tm_memory_pool_create(int pool_id) int tm_memory_pool_allocate(int pool_id, unsigned char **memory_ptr) { - void *block = tm_pool_free[pool_id]; + void *block; + + if (pool_id < 0 || pool_id >= TM_FREERTOS_MAX_POOLS) + return TM_ERROR; + + block = tm_pool_free[pool_id]; if (block == NULL) return TM_ERROR; @@ -332,6 +370,9 @@ int tm_memory_pool_allocate(int pool_id, unsigned char **memory_ptr) int tm_memory_pool_deallocate(int pool_id, unsigned char *memory_ptr) { + if (pool_id < 0 || pool_id >= TM_FREERTOS_MAX_POOLS) + return TM_ERROR; + /* Push onto freelist head. */ *((void **) memory_ptr) = tm_pool_free[pool_id]; tm_pool_free[pool_id] = (void *) memory_ptr; diff --git a/ports/threadx/tm_port.c b/ports/threadx/tm_port.c index 8b60774..1d0e829 100644 --- a/ports/threadx/tm_port.c +++ b/ports/threadx/tm_port.c @@ -151,6 +151,10 @@ int tm_thread_create(int thread_id, int priority, void (*entry_function)(void)) { UINT status; + if (thread_id < 0 || thread_id >= TM_THREADX_MAX_THREADS || priority < 1 || + priority > 31) + return TM_ERROR; + /* Remember the actual thread entry. */ tm_thread_entry_functions[thread_id] = (void *) entry_function; @@ -177,6 +181,8 @@ int tm_thread_resume(int thread_id) { UINT status; + if (thread_id < 0 || thread_id >= TM_THREADX_MAX_THREADS) + return TM_ERROR; /* Attempt to resume the thread. */ status = tx_thread_resume(&tm_thread_array[thread_id]); @@ -196,6 +202,8 @@ int tm_thread_suspend(int thread_id) { UINT status; + if (thread_id < 0 || thread_id >= TM_THREADX_MAX_THREADS) + return TM_ERROR; /* Attempt to suspend the thread. */ status = tx_thread_suspend(&tm_thread_array[thread_id]); @@ -236,6 +244,8 @@ int tm_queue_create(int queue_id) { UINT status; + if (queue_id < 0 || queue_id >= TM_THREADX_MAX_QUEUES) + return TM_ERROR; /* Create the specified queue. Message size is computed from the * actual width of unsigned long so LP64 and ILP32 both work. @@ -262,6 +272,8 @@ int tm_queue_send(int queue_id, unsigned long *message_ptr) { UINT status; + if (queue_id < 0 || queue_id >= TM_THREADX_MAX_QUEUES) + return TM_ERROR; /* Send the message to the specified queue. */ status = tx_queue_send(&tm_queue_array[queue_id], message_ptr, TX_NO_WAIT); @@ -282,6 +294,8 @@ int tm_queue_receive(int queue_id, unsigned long *message_ptr) { UINT status; + if (queue_id < 0 || queue_id >= TM_THREADX_MAX_QUEUES) + return TM_ERROR; /* Receive the message from the specified queue. */ status = @@ -302,6 +316,8 @@ int tm_semaphore_create(int semaphore_id) { UINT status; + if (semaphore_id < 0 || semaphore_id >= TM_THREADX_MAX_SEMAPHORES) + return TM_ERROR; /* Create semaphore. */ status = tx_semaphore_create(&tm_semaphore_array[semaphore_id], @@ -322,6 +338,8 @@ int tm_semaphore_get(int semaphore_id) { UINT status; + if (semaphore_id < 0 || semaphore_id >= TM_THREADX_MAX_SEMAPHORES) + return TM_ERROR; /* Get the semaphore. */ status = tx_semaphore_get(&tm_semaphore_array[semaphore_id], TX_NO_WAIT); @@ -341,6 +359,8 @@ int tm_semaphore_put(int semaphore_id) { UINT status; + if (semaphore_id < 0 || semaphore_id >= TM_THREADX_MAX_SEMAPHORES) + return TM_ERROR; /* Put the semaphore. */ status = tx_semaphore_put(&tm_semaphore_array[semaphore_id]); @@ -361,6 +381,8 @@ int tm_memory_pool_create(int pool_id) { UINT status; + if (pool_id < 0 || pool_id >= TM_THREADX_MAX_MEMORY_POOLS) + return TM_ERROR; /* Create the memory pool. */ status = tx_block_pool_create( @@ -384,6 +406,8 @@ int tm_memory_pool_allocate(int pool_id, unsigned char **memory_ptr) { UINT status; + if (pool_id < 0 || pool_id >= TM_THREADX_MAX_MEMORY_POOLS) + return TM_ERROR; /* Allocate a 128-byte block from the specified memory pool. */ status = tx_block_allocate(&tm_block_pool_array[pool_id], @@ -405,6 +429,8 @@ int tm_memory_pool_deallocate(int pool_id, unsigned char *memory_ptr) { UINT status; + if (pool_id < 0 || pool_id >= TM_THREADX_MAX_MEMORY_POOLS) + return TM_ERROR; /* Release the 128-byte block back to the specified memory pool. */ status = tx_block_release((void *) memory_ptr); diff --git a/src/cooperative_scheduling.c b/src/cooperative_scheduling.c index 7dc7ee4..947b65c 100644 --- a/src/cooperative_scheduling.c +++ b/src/cooperative_scheduling.c @@ -143,6 +143,7 @@ void tm_cooperative_thread_report(void) unsigned long relative_time; unsigned long last_total; unsigned long average; + unsigned long c0, c1, c2, c3, c4; /* Initialize the last total. */ last_total = 0; @@ -164,26 +165,29 @@ void tm_cooperative_thread_report(void) "Time: %lu\n", relative_time); + /* Snapshot counters so the tolerance check uses values consistent with + * the total (workers keep incrementing). + */ + c0 = tm_cooperative_thread_0_counter; + c1 = tm_cooperative_thread_1_counter; + c2 = tm_cooperative_thread_2_counter; + c3 = tm_cooperative_thread_3_counter; + c4 = tm_cooperative_thread_4_counter; + /* Calculate the total of all the counters. */ - total = - tm_cooperative_thread_0_counter + tm_cooperative_thread_1_counter + - tm_cooperative_thread_2_counter + tm_cooperative_thread_3_counter + - tm_cooperative_thread_4_counter; + total = c0 + c1 + c2 + c3 + c4; /* Calculate the average of all the counters. */ average = total / 5; - /* See if there are any errors. */ - if ((tm_cooperative_thread_0_counter < (average - 1)) || - (tm_cooperative_thread_0_counter > (average + 1)) || - (tm_cooperative_thread_1_counter < (average - 1)) || - (tm_cooperative_thread_1_counter > (average + 1)) || - (tm_cooperative_thread_2_counter < (average - 1)) || - (tm_cooperative_thread_2_counter > (average + 1)) || - (tm_cooperative_thread_3_counter < (average - 1)) || - (tm_cooperative_thread_3_counter > (average + 1)) || - (tm_cooperative_thread_4_counter < (average - 1)) || - (tm_cooperative_thread_4_counter > (average + 1))) { + /* See if there are any errors. Skip when average is 0 to avoid unsigned + * wraparound on (average - 1). + */ + if (average > 0 && ((c0 < (average - 1)) || (c0 > (average + 1)) || + (c1 < (average - 1)) || (c1 > (average + 1)) || + (c2 < (average - 1)) || (c2 > (average + 1)) || + (c3 < (average - 1)) || (c3 > (average + 1)) || + (c4 < (average - 1)) || (c4 > (average + 1)))) { tm_printf( "ERROR: Invalid counter value(s). Cooperative counters should " "not be more that 1 different than the average!\n"); diff --git a/src/interrupt_preemption_processing.c b/src/interrupt_preemption_processing.c index 1d692fc..ac8ea2e 100644 --- a/src/interrupt_preemption_processing.c +++ b/src/interrupt_preemption_processing.c @@ -129,7 +129,7 @@ void tm_interrupt_preemption_thread_report(void) unsigned long relative_time; unsigned long last_total; unsigned long average; - + unsigned long c0, c1, ch; /* Initialize the last total. */ last_total = 0; @@ -151,32 +151,33 @@ void tm_interrupt_preemption_thread_report(void) "Relative Time: %lu\n", relative_time); + /* Snapshot counters for consistent total and tolerance check. */ + c0 = tm_interrupt_preemption_thread_0_counter; + c1 = tm_interrupt_preemption_thread_1_counter; + ch = tm_interrupt_preemption_handler_counter; + /* Calculate the total of all the counters. */ - total = tm_interrupt_preemption_thread_0_counter + - tm_interrupt_preemption_thread_1_counter + - tm_interrupt_preemption_handler_counter; + total = c0 + c1 + ch; /* Calculate the average of all the counters. */ average = total / 3; - /* See if there are any errors. */ - if ((tm_interrupt_preemption_thread_0_counter < (average - 1)) || - (tm_interrupt_preemption_thread_0_counter > (average + 1)) || - (tm_interrupt_preemption_thread_1_counter < (average - 1)) || - (tm_interrupt_preemption_thread_1_counter > (average + 1)) || - (tm_interrupt_preemption_handler_counter < (average - 1)) || - (tm_interrupt_preemption_handler_counter > (average + 1))) { + /* See if there are any errors. Skip when average is 0 to avoid unsigned + * wraparound on (average - 1). + */ + if (average > 0 && ((c0 < (average - 1)) || (c0 > (average + 1)) || + (c1 < (average - 1)) || (c1 > (average + 1)) || + (ch < (average - 1)) || (ch > (average + 1)))) { tm_printf( "ERROR: Invalid counter value(s). Interrupt processing test " "has failed!\n"); } /* Show the total interrupts for the time period. */ - tm_printf("Time Period Total: %lu\n\n", - tm_interrupt_preemption_handler_counter - last_total); + tm_printf("Time Period Total: %lu\n\n", ch - last_total); /* Save the last total number of interrupts. */ - last_total = tm_interrupt_preemption_handler_counter; + last_total = ch; } TM_REPORT_FINISH; diff --git a/src/interrupt_processing.c b/src/interrupt_processing.c index efced3c..d9633a8 100644 --- a/src/interrupt_processing.c +++ b/src/interrupt_processing.c @@ -130,7 +130,7 @@ void tm_interrupt_thread_report(void) unsigned long last_total; unsigned long relative_time; unsigned long average; - + unsigned long ct, ch; /* Initialize the last total. */ last_total = 0; @@ -152,28 +152,31 @@ void tm_interrupt_thread_report(void) "%lu\n", relative_time); + /* Snapshot counters for consistent total and tolerance check. */ + ct = tm_interrupt_thread_0_counter; + ch = tm_interrupt_handler_counter; + /* Calculate the total of all the counters. */ - total = tm_interrupt_thread_0_counter + tm_interrupt_handler_counter; + total = ct + ch; /* Calculate the average of all the counters. */ average = total / 2; - /* See if there are any errors. */ - if ((tm_interrupt_thread_0_counter < (average - 1)) || - (tm_interrupt_thread_0_counter > (average + 1)) || - (tm_interrupt_handler_counter < (average - 1)) || - (tm_interrupt_handler_counter > (average + 1))) { + /* See if there are any errors. Skip when average is 0 to + * avoid unsigned wraparound on (average - 1). + */ + if (average > 0 && ((ct < (average - 1)) || (ct > (average + 1)) || + (ch < (average - 1)) || (ch > (average + 1)))) { tm_printf( "ERROR: Invalid counter value(s). Interrupt processing test " "has failed!\n"); } /* Show the total interrupts for the time period. */ - tm_printf("Time Period Total: %lu\n\n", - tm_interrupt_handler_counter - last_total); + tm_printf("Time Period Total: %lu\n\n", ch - last_total); /* Save the last total number of interrupts. */ - last_total = tm_interrupt_handler_counter; + last_total = ch; } TM_REPORT_FINISH; diff --git a/src/preemptive_scheduling.c b/src/preemptive_scheduling.c index c81293b..966d147 100644 --- a/src/preemptive_scheduling.c +++ b/src/preemptive_scheduling.c @@ -172,7 +172,7 @@ void tm_preemptive_thread_report(void) unsigned long relative_time; unsigned long last_total; unsigned long average; - + unsigned long c0, c1, c2, c3, c4; /* Initialize the last total. */ last_total = 0; @@ -194,26 +194,29 @@ void tm_preemptive_thread_report(void) "%lu\n", relative_time); + /* Snapshot counters so the tolerance check uses values consistent with + * the total (workers keep incrementing). + */ + c0 = tm_preemptive_thread_0_counter; + c1 = tm_preemptive_thread_1_counter; + c2 = tm_preemptive_thread_2_counter; + c3 = tm_preemptive_thread_3_counter; + c4 = tm_preemptive_thread_4_counter; + /* Calculate the total of all the counters. */ - total = tm_preemptive_thread_0_counter + - tm_preemptive_thread_1_counter + - tm_preemptive_thread_2_counter + - tm_preemptive_thread_3_counter + tm_preemptive_thread_4_counter; + total = c0 + c1 + c2 + c3 + c4; /* Calculate the average of all the counters. */ average = total / 5; - /* See if there are any errors. */ - if ((tm_preemptive_thread_0_counter < (average - 1)) || - (tm_preemptive_thread_0_counter > (average + 1)) || - (tm_preemptive_thread_1_counter < (average - 1)) || - (tm_preemptive_thread_1_counter > (average + 1)) || - (tm_preemptive_thread_2_counter < (average - 1)) || - (tm_preemptive_thread_2_counter > (average + 1)) || - (tm_preemptive_thread_3_counter < (average - 1)) || - (tm_preemptive_thread_3_counter > (average + 1)) || - (tm_preemptive_thread_4_counter < (average - 1)) || - (tm_preemptive_thread_4_counter > (average + 1))) { + /* See if there are any errors. Skip when average is 0 to avoid unsigned + * wraparound on (average - 1). + */ + if (average > 0 && ((c0 < (average - 1)) || (c0 > (average + 1)) || + (c1 < (average - 1)) || (c1 > (average + 1)) || + (c2 < (average - 1)) || (c2 > (average + 1)) || + (c3 < (average - 1)) || (c3 > (average + 1)) || + (c4 < (average - 1)) || (c4 > (average + 1)))) { tm_printf( "ERROR: Invalid counter value(s). Preemptive counters should " "not be more that 1 different than the average!\n");