Skip to content

Restored the random stack fill value in the thread control block after it is cleared during thread creation, so stack checking now looks for the pattern that is actually in the stack - #732

Open
fdesbiens wants to merge 1 commit into
eclipse-threadx:devfrom
fdesbiens:fix/issue-723

Conversation

@fdesbiens

Copy link
Copy Markdown
Contributor

Fixes #723

When TX_ENABLE_STACK_CHECKING and TX_ENABLE_RANDOM_NUMBER_STACK_FILLING are both enabled, TX_STACK_FILL expands to thread_ptr -> tx_thread_stack_fill_value. _tx_thread_create picked a random byte, replicated it into the four bytes of that field, and filled the stack with it. It then cleared the whole control block with TX_MEMSET(thread_ptr, 0, sizeof(TX_THREAD)), which reset the field to zero. The stack held the random pattern while the control block claimed the pattern was zero, so TX_THREAD_STACK_CHECK and _tx_thread_stack_analyze compared the stack against the wrong value for the entire life of the thread.

The random value is now computed into a local variable and stored in the control block again after the block has been cleared. Keeping the clear where it is rather than moving it to the top of the function matters for the module manager, whose error checking walks the created list before the control block may be touched.

The same defect was present in three places, all fixed here.

  • common/src/tx_thread_create.c
  • common_smp/src/tx_thread_create.c
  • common_modules/module_manager/src/txm_module_manager_thread_create.c, where it additionally left the kernel stack of a user mode module thread filled with zeros rather than with the random pattern, because that fill happens after the control block is cleared.

A regression test, threadx_thread_stack_fill_value_test, creates sixteen threads that are never started and checks that the fill value recorded in each control block is the pattern present in the stack. It also tolerates a random byte of zero without letting that hide the defect. It is added to both the tx and the SMP suites.

Verification.

  • The new test reports ERROR #3 on the current dev in the stack_checking_rand_fill_build configuration, and passes with this change.
  • All five tx configurations pass, 104 tests each.
  • All five SMP configurations pass, 117 tests each.
  • common/src/tx_thread_create.c, common_smp/src/tx_thread_create.c and the module manager file compile clean under -Wall -Wextra with every combination of TX_ENABLE_STACK_CHECKING, TX_ENABLE_RANDOM_NUMBER_STACK_FILLING, TX_DISABLE_STACK_FILLING and TX_MISRA_ENABLE.

…r it is cleared during thread creation, so stack checking now looks for the pattern that is actually in the stack

When TX_ENABLE_STACK_CHECKING and TX_ENABLE_RANDOM_NUMBER_STACK_FILLING were both
enabled, _tx_thread_create picked a random byte, replicated it into the four bytes of
tx_thread_stack_fill_value, and filled the stack with it through the TX_STACK_FILL macro.
It then cleared the whole control block with TX_MEMSET, which reset that field to zero.
The stack held the random pattern while the control block claimed the pattern was zero, so
TX_THREAD_STACK_CHECK and _tx_thread_stack_analyze compared the stack against the wrong
value for the entire life of the thread.

The random value is now computed into a local variable and stored in the control block
again after the block has been cleared. The same defect was present in the SMP kernel and
in the module manager, where it additionally left the kernel stack of a user mode module
thread filled with zeros rather than with the random pattern.

A regression test creates several threads that are never started and checks that the fill
value recorded in each control block is the pattern present in the stack. It fails on the
previous code in the stack_checking_rand_fill_build configuration and passes in all five
configurations of both the tx and the SMP suites.

Fixes eclipse-threadx#723

Assisted-by: Copilot (Opus 5) <noreply@github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant