casaroli opened a new pull request, #21215: URL: https://github.com/apache/nuttx/pull/21215
## Summary #21067 made `mm_memalign()` of the default heap accept an alignment of 0. Two places still fail with it, and the ELF loader, which passes `sh_addralign` from the file, does not check the section headers at all. Four commits: - **mm/tlsf: Fix the nxstyle errors.** Blank lines and indentation only (`git diff -w --ignore-blank-lines` is empty); CI checks the whole file. The same commit is in #21142; whichever PR merges second drops it at the rebase. - **mm/tlsf: Handle zero memalign alignment.** The TLSF `mm_memalign()` gave 0 to `mempool_multiple_memalign()`, where `ALIGN_UP(blk, 0)` divides by zero: on arm64 the result is 0, so the pool block is lost and the call falls through to `tlsf_memalign()`. The same check as #21067: 0 means `MM_ALIGN`. - **libs/libc/elf: Treat a section alignment of 0 as no alignment.** The ELF specification says that `sh_addralign` 0 and 1 mean no alignment constraint. `_ALIGN_UP(v, 0)` in `elf_load.c` gave 0, because `~(0 - 1)` is 0. So a section with alignment 0 set the running data or text size back to 0 (the buffer was too small), and the loader set the copy address of the section to 0. - **libs/libc/elf: Reject invalid section alignments and sizes.** `libelf_elfsize()` used the values of the file without a check. An alignment that is not 0 or a power of two gave a wrong `_ALIGN_UP()` result. The sums of `sh_size`, of `p_memsz` and of the FDPIC descriptors could wrap, and a later round-up to pages could wrap too; then the loader wrote the sections past a too small buffer. In a kernel build this is the kernel, with a file that a user program gives to `exec()`. `libelf_elfsize()` now checks each value and each sum, limits them to half of the address space (no image can be that large, and the later round-ups then cannot wrap), and fails the load with `ENOEXEC`. ## Impact All build modes and architectures that use the ELF loader or the TLSF heap. No change for valid files with a non-zero alignment, and no change for `memalign()` with a non-zero alignment. - `memalign(0, n)` on the TLSF heap now behaves as `malloc(n)`, as on the default heap since #21067. - An ELF file with `sh_addralign` 0 now loads correctly (it crashed the loader before). - An ELF file with an alignment that is not 0 or a power of two, or with section sizes larger than half of the address space, now fails with `ENOEXEC`. Such a file is not valid ELF; before, the test file with alignment 3 loaded and ran. ## Testing Master here is 23f5a4f06df + #21067: master 7505b874722 does not build for arm64 (fixed by #21210). The branch is the same base + these four commits. QEMU with HVF. The macro, on the host: the `_ALIGN_UP` line of each tree, compiled into one test, against the exact result (the smallest multiple of `a` that is >= `v`; 0 and 1: `v`): | | master | this PR | |---|---|---| | `_ALIGN_UP(v, 0)`, `v` = 1 ... 4095 | 0 every time | `v` | | powers of two 2 ... 2^63 | correct | correct | | loader size loop: `.data` 16 B (align 8), a section of 12 B (align 0), `.bss` 256 B | 272, 16 B too small | 288 | | copy address of the align-0 section, `_ALIGN_UP(0x40001004, 0)` | 0x0 | 0x40001004 | (An alignment that is not a power of two is wrong in both: the fourth commit rejects it.) The ELF loader, qemu-armv8a:knsh (kernel build): a test program with a writable section `.align0`, a `.data` array and a `.bss` array checks its data after the start; a script changes one section header after the link: | Program | master | this PR | |---|---|---| | `elfalign0`: `.align0` `sh_addralign` 8 -> 0 | kernel data abort at EL1 (FAR 0x10), the system stops | runs, all data correct | | `elfalign3`: `.align0` `sh_addralign` 8 -> 3 | loads and runs (the section is placed with a wrong `_ALIGN_UP()` value) | `ENOEXEC`: "Bad size or alignment of section 8" | | `elfbigbss`: `.bss` `sh_size` 0x105 -> 0xfffffffffffff000 | kernel data abort at EL1 (FAR 0xc000e000), the system stops | `ENOEXEC`: "Bad size or alignment of section 11" | ostest exits with status 0 on qemu-armv8a:knsh and qemu-armv8a:nsh with this PR. The TLSF heap, qemu-armv8a:nsh (flat) with `CONFIG_MM_TLSF_MANAGER=y` and `CONFIG_MM_HEAP_MEMPOOL_THRESHOLD=256`: a test program calls `memalign(0, 24)` + `free()` 200 times. Master: the heap grows from 58080 to 62912 bytes in use (a lost pool block each time). This PR: no growth; ostest exits with status 0. The default heap (control, fixed by #21067): no growth on both. Build, each with `EXTRAFLAGS="-Wno-cpp -Werror"`, OK: rv-virt:elf (flat, rv32), rv-virt:pnsh64 (protected, rv64), rv-virt:knsh64 (kernel, rv64), mps3-an547:picostest (flat, Arm, PIC modules), qemu-armv8a:knsh and qemu-armv8a:nsh with TLSF (from the runs). pimoroni-pico-2-plus:xipfs-fdpic: `elf_load.c` (with the FDPIC part, which has no `#ifdef`) compiles; the build then stops at the apps FDPIC modules, which need `arm-uclinuxfdpiceabi-ld` (in the CI image, not on this host). `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]
