94xhn opened a new pull request, #19398: URL: https://github.com/apache/nuttx/pull/19398
Fixes #19308. ## Problem On a non-SMP build, `nxtask_exit()` identifies the exiting task with `dtcb = this_task()` - the head of the ready-to-run list. If a higher-priority task becomes ready and is placed at the ready-to-run head while the context switch away from the exiting task is still deferred (so the head no longer equals the task that is actually running), `nxtask_exit()` calls `nxsched_remove_self(dtcb)` and `nxsched_release_tcb(dtcb)` on that higher-priority task instead of on the task that is actually exiting. This frees the TCB/stack of a **live** task - a use-after-free that hard-faults or locks up the board. The SMP path of the same function already avoids this by using the actually-running task (`current_task(this_cpu())` == `g_assignedtasks[cpu]`); only the non-SMP path used the ready-to-run head. Note that `this_task()` and `current_task(cpu)` are literally the same expression on non-SMP (both resolve to the ready-to-run list head), so the divergence isn't about SMP vs non-SMP semantics per se - it's that the non-SMP path never had a correct "actually running" accessor to fall back to. ## Fix The correct non-SMP accessor for the running task is `g_running_tasks[this_cpu()]` (`this_cpu()` is `(0)` on non-SMP), which is updated only at real context switches and so stays correct exactly when the ready-to-run head has diverged due to a deferred higher-priority switch. Note this is deliberately **not** the same as the existing `running_task()` macro (`running_task() == up_interrupt_context() ? g_running_tasks[this_cpu()] : this_task()`), since that macro only prefers `g_running_tasks[]` during interrupt-level context switches - the race this issue describes happens in a perfectly ordinary (non-interrupt) call chain (`exit()` -> `_exit()` -> `nxtask_exithook()` -> `up_exit()` -> `nxtask_exit()`), so `running_task()` would still resolve to `this_task()` here and would not fix anything. Removing the correct task is safe: the exiting task is in `TSTATE_TASK_READYTORUN` and present in the ready-to-run list, so `nxsched_remove_readytorun()` takes its `dq_rem` path, and the CPU then switches to the pending higher-priority task. `nxtask_exit()` is the only exit-path site that derives the exiting task from `this_task()` (`nxtask_terminate()` uses an explicit pid), so this one line is the complete fix. ## Verification I don't have the affected hardware (RP2350/non-SMP) set up to reproduce the exact race, but the issue report itself includes extensive on-target validation: the reporter captured the divergence with an SRAM breadcrumb (`this_task()` resolving to a live high-priority Wi-Fi thread while `g_running_tasks[cpu]` correctly pointed at the exiting app), reproduced the resulting hard fault deterministically with `CONFIG_MM_FILL_ALLOCATIONS`, and validated this exact one-line fix survives 20+ ELF load/run/exit cycles where the pre-fix build hard-faults on the first run. I independently re-verified the code citations against the current upstream master (confirmed the non-SMP path still uses `this_task()`, confirmed `g_running_tasks[]`/`this_cpu()` are unconditionally available per `include/nuttx/sched.h`, and confirmed `running_task()` would not have fixed this given the non-interrupt call path), and verified the core divergence logic (`this_task()` resolving to the wrong task vs. `g_running_tasks[this_cpu()]` resolving to the actually-exiting task) with a standalone C reproduction of the two accessors under the described race condition. ## Disclosure Generative AI (Claude) was used to help investigate this issue and implement this fix. All changes were reviewed by me before submission. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
