casaroli opened a new pull request, #21154: URL: https://github.com/apache/nuttx/pull/21154
## Summary In the kernel build a `vfork()` child is in the address environment of its parent. `nxtask_setup_fork()` allocates the stack of the child (`up_create_stack()`), its task information (`group_allocate()`) and the copy of its environment (`env_dup()`) in the user heap of the current address environment: the heap of the parent. When the child ends (`_exit()`, or `exec()` and then `_exit(0)` in `execve()`), its group ends, and the kernel-build exit code assumes that the user heap of the group goes away with its address environment. For a `vfork()` child that is not true: the heap is the heap of the parent, and it stays. So each `vfork()` lost the task information and the environment of the child (1232 bytes on qemu-armv8a:knsh) in the heap of the parent. A shell that runs each command with `vfork()` grows with each command. - **sched: Free the task information of a vfork() child in the parent's heap.** A new group flag, `GROUP_FLAG_VFORK` (bit 7 of `tg_flags`, which was free), is set by `nxtask_setup_fork()` when a `vfork()` child joins the address environment of its parent. `task_uninit_info()` then frees the task information and the environment of the child. `group_leave()` clears the flag when the caller is not in the address environment of the child (code in another process ends the child): a free from there would go into the heap of the caller, so the blocks stay in the heap of the parent, as before. - **sched/group: Do not free the stacks of an ending process into its heap.** When the group of a user thread ends (the last member leaves, or the group is exiting), `group_leave()` clears `TCB_FLAG_FREE_STACK`, so that `up_release_stack()` does not free the stack into a heap that goes away with the address environment and that a stack overflow can have broken. This also stops a free into the wrong heap when code in another process ends the thread. A `vfork()` child (`GROUP_FLAG_VFORK`) keeps the flag: its stack is in the heap of its parent and is freed there. ## Impact - Kernel builds: `vfork()` no longer loses memory in the heap of the parent. Stacks of an ending process are not freed into its own heap (that heap goes away with the address environment). - A `vfork()` child that code in another process ends (`task_delete()` from the kernel, for example) still leaves its blocks in the heap of the parent, as before. A child that ends itself (also by a fatal signal) frees them. - Flat and protected builds: no change. Only `CONFIG_ARCH_ADDRENV` sets the flag, and only `CONFIG_BUILD_KERNEL` code uses it. No architecture code. - No ABI change (`tg_flags` is kernel-internal). Not a breaking change. ## Testing Test program `forkstress.c`: N rounds of a case, then the page pool and the kernel heap (`/proc/meminfo`), the processes in `/proc`, the lowest free file descriptor and the heap of the test (`mallinfo()`) are compared with the values after the first round. Cases: `fork()` + `_exit()`; `fork()` + `exec()`; two children joined by a pipe; the same in a forked child; `posix_spawn()`; `vfork()` + `_exit()`; `vfork()` + `exec()`. qemu-armv8a:knsh (QEMU, HVF) with `CONFIG_PIPES`, `CONFIG_SCHED_WAITPID`, `CONFIG_SCHED_CHILD_STATUS`: | Case | master | this PR | |---|---|---| | fork 1000, exec 300, pipe 200, nest 200, spawn 300 | PASS (no growth) | PASS | | vfork 200 | FAIL: heap in use +246408 bytes (1232 per child) | PASS (+0) | | vexec 200 | FAIL: heap in use +246400 bytes | PASS (+0) | rv-virt:knsh64 (QEMU TCG): master vexec 200 +168016 bytes, vexec 200 +168000, vfork 100 +84000 (840 per child); this PR +0 in each case. rv-virt:ksmp64 (SMP): master vfork 100 +87104, vexec 100 +80920; this PR: heap in use +0 in vfork 100, vexec 200, vexec 200 and vfork 100. (On rv-virt:ksmp64 the page pool grows once by 146 pages in the first `exec()` batch, on master and with this PR, and not in later batches. That is not from this change.) ostest exits with status 0 on qemu-armv8a:knsh, qemu-armv8a:nsh, rv-virt:knsh64 and rv-virt:ksmp64, on master and with this PR. No test fails on master for the second commit alone: a stack overflow test (a child that recurses, one that writes below a large `alloca()`, a thread that recurses) ends each child and the system goes on, on master and with this PR. With the second commit no stack is freed into a heap that goes away, and the `vfork` and `vexec` cases above show that the stacks of `vfork()` children are still freed. Build check, each OK and with `group_leave.c` compiled: qemu-armv8a:knsh, qemu-armv8a:nsh, qemu-armv8a:nsh_smp (arm64 kernel, flat, flat SMP), rv-virt:knsh64, rv-virt:ksmp64, rv-virt:pnsh64 (rv64 kernel, kernel SMP, protected), qemu-armv7a:knsh (armv7-a kernel), qemu-intel64:knsh_romfs (x86_64 kernel), mps2-an500:knsh and lm3s6965-ek:qemu-protected (Cortex-M protected), 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]
