casaroli opened a new pull request, #21330: URL: https://github.com/apache/nuttx/pull/21330
## Summary In the kernel build, `malloc()`, `memalign()`, `realloc()` and `calloc()` grow the heap of the process with `sbrk()` when the heap has no free chunk that is large enough. A request that the process cannot get did not fail correctly: - When `sbrk()` failed, the allocation returned NULL with the errno of `sbrk()`, `EAGAIN`, and not `ENOMEM`. `calloc()` with an overflow of `n * elem_size` returned NULL and did not set errno. - A size above `INTPTR_MAX` became a negative `sbrk()` increment, which fails the `DEBUGASSERT()` in `sbrk()` (or gives `ENOSYS` without assertions). - `sbrk()` keeps the page count in an `unsigned int`. On a 64-bit target, `sbrk(INTPTR_MAX)` gives a page count of 0, maps nothing and returns success, and `mm_extend()` gets a size of 0. So `malloc(SIZE_MAX / 2)` fails the assertion in `mm_extend.c:69` and kills the process; without assertions it corrupts the end of the heap. The two commits: - **mm/umm_heap: Fail an sbrk() increment with too many pages.** `sbrk()` fails with `ENOMEM` when the page count does not fit. - **mm/umm_heap: Set errno to ENOMEM when a process heap cannot grow.** `memalign()` and `realloc()` do not call `sbrk()` for a size above `INTPTR_MAX`, and set `ENOMEM` when the heap cannot grow. `calloc()` sets `ENOMEM` on an overflow. This is what the flat and protected builds do. ## Impact - Kernel build (`CONFIG_ARCH_ADDRENV` and `CONFIG_BUILD_KERNEL`) only: a failed allocation now gives `ENOMEM`, and a very large request fails instead of an assertion or a corrupt heap. The successful paths do not change. - Flat and protected builds: no change (the code is not built there). Not a breaking change. - All architectures with a kernel build (arm, arm64, risc-v, x86_64, xtensa): the change is in common code. - No documentation change: Documentation/ does not describe these errno values. ## Testing Base: master 5c441693fa8. "master" in the results is this base. A test program asks for more memory than the process can get: `malloc()`, `memalign(16, ...)`, `calloc(1, ...)` and `realloc(p, ...)` of 4 GiB (64-bit) or `SIZE_MAX / 2` (32-bit), then of `SIZE_MAX / 2`, then `malloc()` and `memalign()` of `SIZE_MAX`, and `calloc(SIZE_MAX / 2, 4)`. Each case must return NULL with errno `ENOMEM`; after `realloc()` fails, the old block must be unchanged; at the end `malloc(100000)` must work. | Configuration | master | this PR | | --- | --- | --- | | qemu-armv8a:knsh (kernel, arm64, HVF) | 0 of 5 pass, then the assertion in `mm_extend.c:69` | 11 of 11 pass | | rv-virt:knsh64 (kernel, rv64, TCG) | 0 of 5 pass, then the assertion in `mm_extend.c:69` | 11 of 11 pass | | qemu-armv8a:nsh (flat, builtin app) | 11 of 11 pass | 11 of 11 pass | | mps2-an500:knsh (protected, Cortex-M7, TCG) | 11 of 11 pass | 11 of 11 pass | A second program with only `malloc(SIZE_MAX)`: on master on qemu-armv8a:knsh it fails the assertion in `umm_sbrk.c:84` and the process is killed; with this PR the same case in the first program passes. On master the 5 cases before the crash give errno `EAGAIN` (4 cases) or 0 (`calloc()` overflow). ostest exits with status 0 on qemu-armv8a:knsh, qemu-armv8a:nsh and rv-virt:knsh64, on master and with this PR. qemu-armv7a:knsh (kernel, 32-bit) builds. `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]
