SMP next-priority lookup shifts by an unvalidated priority, then checks the range afterwards

[fdesbiens]

common_smp/src/tx_thread_smp_utilities.c, in the TX_MAX_PRIORITIES <= 32 variant of _tx_thread_smp_next_priority_find:

local_priority_map =  _tx_thread_priority_maps[0];
local_priority_map =  local_priority_map >> priority;          /* line 139 */
next_priority =  priority;
if (local_priority_map == ((ULONG) 0))
{
    next_priority =  ((UINT) TX_MAX_PRIORITIES);
}
else
{
    if (next_priority >= ((UINT) TX_MAX_PRIORITIES))           /* line 147 */
    {
        next_priority =  ((UINT) TX_MAX_PRIORITIES);           /* line 149 */
    }
    ...

The guard at line 147 tests for exactly the input that has already caused undefined behaviour at line 139. ULONG >> 32 is undefined in C. On x86 a variable shift masks the count to five bits, so >> 32 silently leaves the map unchanged and the guard then fires — but nothing entitles the code to that behaviour, and a compiler is free to do something else with it.

The TX_MAX_PRIORITIES > 32 variant of the same function, sixty lines above in the same file, gets this right:

found_priority =  ((UINT) TX_MAX_PRIORITIES);
if (priority < ((UINT) TX_MAX_PRIORITIES))
{
    map_index =  priority/((UINT) 32);
    ...

It validates the range first and only then indexes and shifts. So this is an inconsistency between two spellings of one function rather than a considered choice.

Not currently reachable

Both callers test the return value for TX_MAX_PRIORITIES and stop before calling again with it:

• common_smp/src/tx_thread_smp_rebalance_execute_list.c:168
• common_smp/src/tx_thread_system_suspend.c:683

So this is robustness rather than a live defect, and there is no security advisory in it.

Why fix it anyway

It is a two-line reorder that makes the two variants of one function agree, and it is the only thing standing between line 149 and honest test coverage. Line 149 is one of the three lines of common_smp/src that #677 left uncovered, and it was left deliberately: a test that reached it today would have to rely on the undefined shift to pass, which is worse than an uncovered line.

Suggested change

Hoist the range test above the shift, mirroring the > 32 variant, then add a regression test that calls _tx_thread_smp_next_priority_find(TX_MAX_PRIORITIES) directly and asserts it returns TX_MAX_PRIORITIES.

That takes the merged SMP coverage report from 5167/5178 to 5168/5178 and, more to the point, retires one of the three remaining "unreachable" entries by making it reachable rather than by argument.

Follows from #677.