casaroli opened a new pull request, #20796: URL: https://github.com/apache/nuttx/pull/20796
## Summary `struct pollfd` had 32-bit `events` and `revents` and three kernel fields (`arg`, `cb`, `priv`): 40 bytes on a 64-bit target and 24 bytes on a 32-bit target. POSIX gives it `int fd`, `short events` and `short revents` (8 bytes). A program or library that builds its own array with the standard layout and calls `poll()` gets its neighbouring entries overwritten. The Rust `libc` crate, for example, already uses the 8-byte layout for NuttX. In flat and protected builds, `poll()` also used the caller's array directly: it wrote a semaphore address and a callback pointer into it, and `poll_notify()` called that callback, often from an interrupt handler. In a protected build the array is user memory, so user code could change the function pointer that the kernel calls. Two threads could not poll the same array (REVISIT comment in `poll.h`). This PR has three commits: 1. `fs/poll: Use struct pollfd_s for the kernel side of poll.` A new kernel header, `include/nuttx/poll.h`, holds `struct pollfd_s` (the same fields as the old `struct pollfd`), `pollevent_t`, `pollcb_t`, `POLLALWAYS`, `poll_notify()` and `poll_default_cb()`. `include/nuttx/fs/fs.h` includes it. The poll methods of drivers, sockets and mount points, `file_poll()`, `psock_poll()`, the waiter arrays and the poll callbacks use `struct pollfd_s` (a mechanical rename in 147 files). `poll()` still hands the user entries to the drivers, so the behaviour does not change. 2. `!fs/poll: Give struct pollfd the POSIX layout.` `struct pollfd` gets only the three POSIX fields. `poll()` copies `fd` and `events` of each entry into a `struct pollfd_s` array (on the stack for up to 4 entries, on the fs heap for more) and copies `revents` back, also when the wait is interrupted. The cancellation cleanup frees the heap copy. The kernel build already copied the array on every call. 3. `Documentation: Describe struct pollfd and struct pollfd_s.` `select()`, `pselect()`, `ppoll()`, `epoll` and the system call interface do not change. `epoll` already keeps its own kernel entry for each file descriptor. This PR depends on apache/nuttx#20794 (whitespace-only: the older nxstyle errors in 60 of the touched files) and apache/nuttx#20795 (`fusb302.c`: unbalanced parentheses and its nxstyle errors). Until they merge, this branch contains their commits. depends-on: [apache/nuttx/pull/20794 apache/nuttx/pull/20795] ## BREAKING CHANGE `struct pollfd` has the POSIX layout (`int fd`, `short events`, `short revents`) and no NuttX fields. The kernel-only declarations moved from `sys/poll.h` to `nuttx/poll.h`. Quick fix: - Applications: rebuild. Code that uses only `fd`, `events` and `revents` needs no change. Print `events` and `revents` with `%x`, not `PRIx32` (apache/nuttx-apps#3878 fixes the programs in nuttx-apps). - Out-of-tree drivers: rename `struct pollfd` to `struct pollfd_s` in the poll methods, the waiter arrays and the poll callbacks. Include `<nuttx/poll.h>` if the file does not include `<nuttx/fs/fs.h>`. The compiler reports every place that is not renamed. - apache/nuttx-apps#BBBBB updates the rpmsgdev test driver. ## Impact - Users: `sizeof(struct pollfd)` is 8 on every target. Applications must be rebuilt. - Size (`qemu-armv8a`, text): +168 bytes in a flat build (`nsh`), +48 bytes in a kernel build (`knsh`). The stack of `poll()` grows by 4 x `sizeof(struct pollfd_s)`: 160 bytes on a 64-bit target, 96 bytes on a 32-bit target. - Documentation: `Documentation/os/filesystem/index.rst` (poll method) and `Documentation/reference/user/10_filesystem.rst` (`poll()`). ## Testing A test program (`pollabi.c`) on `qemu-armv8a:knsh` (QEMU with HVF, `CONFIG_PIPES=y`): | Check | master | this PR | |---|---|---| | `sizeof(struct pollfd) == 8`, offsets 0/4/6 | FAIL (40 bytes) | PASS | | `poll()` on 3 entries with the standard layout (pipe read end, write end, `fd = -1`) | FAIL (returns 1, `revents` not written) | PASS (returns 2, `revents` 1/4/0) | | memory before and after that array not written | PASS | PASS | | `poll()` on 6 entries (heap copy) | PASS | PASS | | `ppoll()` timeout clears `revents` | FAIL | PASS | | `select()`, `epoll` | PASS | PASS | | two threads `poll()` the same array | PASS (kernel build copies) | PASS | Real hardware, flat builds, the same `pollabi.c` as an NSH app (logs attached): | Board | master | this PR | |---|---|---| | Raspberry Pi RP2350 (Cortex-M33), `pimoroni-pico-2-plus:nsh` + `CONFIG_PIPES` | `pollabi` FAIL (layout, 3 standard entries, `ppoll` timeout); `ostest` status 0 | `pollabi` PASS; `ostest` status 0 | | ESP32-S3 (Xtensa LX7, WROOM-2 N32R8V), `esp32s3-devkit:nsh` + octal flash + `CONFIG_PIPES` | `pollabi` FAIL (layout), then the two-thread test crashes the board: store to address 0 in `pipecommon_poll()` (`drivers/pipes/pipe_common.c:773`) | `pollabi` PASS, no crash | On the ESP32-S3, `ostest` stops with an assertion in its libc tests on master and with this PR alike (status 256): an older failure on this board, not related to `poll()`. `ostest` ends with `ostest_main: Exiting with status 0` on `qemu-armv8a:knsh` and `qemu-armv8a:nsh`. The two-thread case passes on master in a kernel build (`poll()` copies the array there) and crashes on master in a flat build (ESP32-S3 above). `tools/checkpatch.sh -c -u -m -g` passes. Builds with `-Werror`: `qemu-armv8a:nsh`, `knsh`, `citest`, `netnsh`, `rpserver`, `rpproxy`, `fb`, and a `qemu-armv8a:rpserver` configuration with about 100 more options (rpmsg device client and server, usrsock rpmsg server, local sockets, eventfd, timerfd, signalfd, inotify, pty, packet, CAN, sensors, input and touchscreen drivers, ramlog, noteram, GPIO, procfs pressure, tun, cryptodev, lirc, timer, ADC, comparator, audio, MTD config, SPI/I2C slave, SLIP, battery, Bluetooth UART shim, IPCC, USB, LCD, modem and wireless drivers). That configuration compiles 1560 objects and gives the same compiler errors before and after the PR; all of them are older errors in drivers that no defconfig enables. -- 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]
