Repository navigation
Conversation
…eap. In the kernel build a vfork() child is in the address environment of its parent. group_allocate() and env_dup() put the task information and the environment of the child in the user heap of the parent, because that is the heap of the current address environment. When the child ends, task_uninit_info() returns at once for an unprivileged group, because the user heap of the group goes away with its address environment. The heap of a vfork() child is the heap of its parent, and it stays. Thus each vfork() lost these blocks in the heap of the parent. A shell runs each command with vfork(), so its heap grew with each command. Add GROUP_FLAG_VFORK. nxtask_setup_fork() sets it when a vfork() child joins the address environment of its parent. task_uninit_info() frees the task information and the environment of a group with this flag. group_leave() clears the flag when the address environment of the caller is not the one of the child (another process ends the child): a free from there goes into the heap of the caller. Then the blocks stay in the heap of the parent, as before. Flat and protected builds do not change: only CONFIG_ARCH_ADDRENV sets the flag, and only CONFIG_BUILD_KERNEL uses it. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Marco Casaroli <marco.casaroli@gmail.com>
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(), which calls kumm_free(): the kernel then 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. Seen with a thread that overflows its stack: the fatal signal ends the process, and nxsig_terminate() -> nx_pthread_exit() -> nxtask_exit() -> nxsched_release_tcb() -> up_release_stack() -> mm_free() fails DEBUGASSERT(mm_heapmember()) on the broken heap. When a user thread leaves its group, and the group ends (it is the last member, or group_kill_children() set GROUP_FLAG_EXITING), clear TCB_FLAG_FREE_STACK: every architecture frees the stack only when that flag is set, and still clears the stack fields of the TCB. This is done in group_leave(), where the group is known: when the TCB is released, it can have left the group already. The stack of a thread that ends alone (pthread_exit()) is freed as before. A vfork() child (GROUP_FLAG_VFORK) keeps the flag: its stack is in the heap of its parent, which stays. Without this, each vfork() lost a block of the size of the stack of the parent in the heap of the parent. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Marco Casaroli <marco.casaroli@gmail.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
depends-on: /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 withup_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 clearsTCB_FLAG_FREE_STACK, so thatup_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. Avfork()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
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.CONFIG_BUILD_KERNEL. No architecture code: everyup_release_stack()frees only with this flag and clears the stack fields in any case.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 withSIGSEGVand 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 thevforkandvexeccases below show that the stacks ofvfork()children are still freed.qemu-armv8a:knsh (QEMU, HVF) with #21154 applied, master 6b9b216,
forkstress.c(see #21154):ostest exits with status 0 on qemu-armv8a:knsh.
Builds with
EXTRAFLAGS="-Wno-cpp -Werror", each OK and withgroup_leave.ccompiled: qemu-armv8a:knsh, qemu-armv8a:nsh, rv-virt:pnsh64, mps2-an500:knsh, sim:nsh.tools/checkpatch.sh -c -u -m -gpasses.