TX_ENABLE_RANDOM_NUMBER_STACK_FILLING is broken because the fill value is cleared during thread creation

[step70]

Describe the bug

When both TX_ENABLE_STACK_CHECKING and TX_ENABLE_RANDOM_NUMBER_STACK_FILLING are enabled, the random stack fill value stored in the thread control block is cleared later during _tx_thread_create().

The function generates a random 8-bit value, replicates it into the four bytes of
thread_ptr->tx_thread_stack_fill_value, and uses it as the stack fill pattern.

However, later in the same function, the entire TX_THREAD control block is cleared with:

TX_MEMSET(thread_ptr, 0, (sizeof(TX_THREAD)));

This resets:

thread_ptr->tx_thread_stack_fill_value

to zero.

The stack memory still contains the previously generated random pattern, but the thread control block no longer contains the value used to fill the stack.

As a consequence, the stack checking and stack analysis code compares the stack against an incorrect fill value.

Affected configuration

The issue occurs when the following options are enabled:

#define TX_ENABLE_STACK_CHECKING
#define TX_ENABLE_RANDOM_NUMBER_STACK_FILLING

and stack filling is not disabled:

/* TX_DISABLE_STACK_FILLING must not be defined. */

Affected versions

I originally found the issue in Azure RTOS ThreadX 6.4.0.

The same initialization order also appears to be present in the current Eclipse ThreadX source code, including the latest 6.5.x source and the current master branch.

Relevant source file

common/src/tx_thread_create.c

Function:

_tx_thread_create()

Current initialization sequence

The current sequence is effectively:

thread_ptr->tx_thread_stack_fill_value =
    ((ULONG) TX_RAND()) & 0xFFUL;

thread_ptr->tx_thread_stack_fill_value =
    thread_ptr->tx_thread_stack_fill_value |
    (thread_ptr->tx_thread_stack_fill_value << 8) |
    (thread_ptr->tx_thread_stack_fill_value << 16) |
    (thread_ptr->tx_thread_stack_fill_value << 24);

TX_MEMSET(stack_start, ((UCHAR) TX_STACK_FILL), stack_size);

/* Later in the same function. */
TX_MEMSET(thread_ptr, 0, (sizeof(TX_THREAD)));

After the last operation:

thread_ptr->tx_thread_stack_fill_value == 0

while the stack contains the random fill pattern.

For example, if the generated byte is 0x37, the stack contains 0x37373737, but the value stored in the thread control block becomes 0x00000000.

Expected behavior

At the end of _tx_thread_create(), thread_ptr->tx_thread_stack_fill_value should contain the exact value used to initialize the unused stack memory.

For example:

thread_ptr->tx_thread_stack_fill_value == 0x37373737UL

when the stack was filled with byte 0x37.

Actual behavior

The random fill pattern is written to the stack, but the corresponding field in the thread control block is subsequently cleared to zero.

The stack analysis routine therefore searches for a pattern different from the one used to initialize the stack.

Suggested fix

A simple fix would be to move:

TX_MEMSET(thread_ptr, 0, (sizeof(TX_THREAD)));

to the beginning of _tx_thread_create(), immediately after the local variable declarations and before any field of thread_ptr is initialized.

The original TX_MEMSET() at its current location should then be removed.

The initialization sequence would become:

TX_MEMSET(thread_ptr, 0, (sizeof(TX_THREAD)));

thread_ptr->tx_thread_stack_fill_value =
    ((ULONG) TX_RAND()) & 0xFFUL;

thread_ptr->tx_thread_stack_fill_value =
    thread_ptr->tx_thread_stack_fill_value |
    (thread_ptr->tx_thread_stack_fill_value << 8) |
    (thread_ptr->tx_thread_stack_fill_value << 16) |
    (thread_ptr->tx_thread_stack_fill_value << 24);

TX_MEMSET(stack_start, ((UCHAR) TX_STACK_FILL), stack_size);

This preserves the current implementation and guarantees that the random fill value remains valid in the thread control block.

An alternative fix would be to preserve the random fill value in a local variable and copy it into the TCB after the TCB has been cleared, but moving the TCB initialization before any field assignment appears simpler.

Reproduction

1. Enable:

   #define TX_ENABLE_STACK_CHECKING
   #define TX_ENABLE_RANDOM_NUMBER_STACK_FILLING

2. Make TX_RAND() return a deterministic nonzero byte, for example
   0x5A.

3. Create a thread using tx_thread_create().

4. Inspect the unused stack memory and:

   thread.tx_thread_stack_fill_value

Expected result:

thread.tx_thread_stack_fill_value == 0x5A5A5A5AUL

Actual result:

thread.tx_thread_stack_fill_value == 0x00000000UL

while the stack remains filled with byte 0x5A.

Additional notes

If TX_RAND() returns zero, the problem may be accidentally hidden because both the stack fill pattern and the cleared TCB field are zero.
This does not make the implementation correct and occurs only for that specific random result.

I could not find an existing issue or release note documenting this problem.

[fdesbiens]

Hi @step70.

Thank you for the detailed report. I will look into this right away.

[fdesbiens]

Hi @step70.

Confirmed, and thank you for a report that pinpointed the problem precisely. Your analysis of the sequence in _tx_thread_create is exactly right.

I found the same defect in two more places: the SMP kernel in common_smp/src/tx_thread_create.c, and txm_module_manager_thread_create.c in the module manager. In the module manager it has a second consequence, because the kernel stack of a user mode module thread is filled after the control block has been cleared, so that stack was being filled with zeros rather than with the random pattern.

I went with your alternative fix rather than moving the TX_MEMSET to the top of the function. The module manager validates its arguments by walking the created thread list before it initializes the control block, so clearing the block first would not be safe there, and I preferred to have the three copies of this code stay identical in shape.

I have also added a regression test, threadx_thread_stack_fill_value_test, to both the tx and the SMP suites. It creates threads that are never started and checks that the fill value in the control block is the pattern actually present in the stack. It fails on the current dev in the stack_checking_rand_fill_build configuration and passes with the fix.

The fix is in PR #732. If you can, I would appreciate it if you would review it and confirm that it resolves the problem in your own configuration before we merge it. I will close this issue once the PR is merged.