casaroli opened a new pull request, #21465: URL: https://github.com/apache/nuttx/pull/21465
depends-on: apache/nuttx/pull/21154 This branch holds the commit of #21154 and one commit of its own, because that commit changes the code that #21154 adds. Please review only the last commit. ## Summary In the kernel build the stack of a thread of an unprivileged group is a block of the user heap of the group. When a thread ends, `nxsched_release_tcb()` frees it with `up_release_stack()`, so the kernel works on the heap structures of the program. When the whole process ends, that heap goes away with the address environment anyway, and the program can have overwritten it: a stack overflow runs down through the heap, over the heap structure at its start. When the group of a user thread ends (the last member leaves, or the group is exiting), `group_leave()` now clears `TCB_FLAG_FREE_STACK`, so that `up_release_stack()` does not free the stack into that heap. This also stops a free into the wrong heap when code in another process ends the thread. A `vfork()` child (`GROUP_FLAG_VFORK`, from #21154) keeps the flag: its stack is in the heap of its parent and is freed there. The stack of a thread that ends alone (`pthread_exit()`) is freed as before. ## Impact - Kernel builds: stacks of an ending process are not freed into its own heap. - A `task_restart()` of a multi-threaded user process: the stacks of the killed threads are not freed any more. Before, they were freed from the context of the caller, into the heap of the caller. Read in the code, not run. - Flat and protected builds: no change. The code is under `CONFIG_BUILD_KERNEL`. No architecture code: every `up_release_stack()` frees only with this flag and clears the stack fields in any case. - No ABI change. Not a breaking change. ## Testing No test fails on master for this commit alone: a stack overflow test (a child that recurses, one that writes below a large `alloca()`, a thread that recurses) ends each child with `SIGSEGV` and the system goes on, with the same output on master and with this PR. With this commit no stack is freed into a heap that goes away, and the `vfork` and `vexec` cases below show that the stacks of `vfork()` children are still freed. qemu-armv8a:knsh (QEMU, HVF) with #21154 applied, master 6b9b216ff30, `forkstress.c` (see #21154): | Case | #21154 and this PR | |---|---| | fork 1000, exec 300, pipe 200, nest 200, spawn 300 | PASS, +0 | | vfork 200 | PASS, +0 | | vexec 200 | PASS, +0 | ostest exits with status 0 on qemu-armv8a:knsh. Builds with `EXTRAFLAGS="-Wno-cpp -Werror"`, each OK and with `group_leave.c` compiled: qemu-armv8a:knsh, qemu-armv8a:nsh, rv-virt:pnsh64, mps2-an500:knsh, sim:nsh. `tools/checkpatch.sh -c -u -m -g` passes. -- 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]
