casaroli opened a new pull request, #21365: URL: https://github.com/apache/nuttx/pull/21365
## Summary `pipe2()` and `file_pipe()` open the write end of the new pipe with `O_NONBLOCK`, and then clear `O_NONBLOCK` with `FIONBIO`. When `FIONBIO` failed, both went to the error path that only unregisters the pipe, so the open write end stayed open: `pipe2()` leaked a descriptor and the pipe device, and `file_pipe()` left the pipe device open. This PR sends both to the error path that also closes the write end, the same path that a failure to open the read end already uses. ## Impact Only the error path of `pipe2()` and `file_pipe()` changes. `pipe()` and `pipe2()` return the same descriptors and flags as before. `pipe2()` is a system call, so this is the same code for flat, protected and kernel builds, on every architecture. `file_pipe()` is used by the pty driver. No documentation change, and no API or ABI change. ## Testing `FIONBIO` on a new pipe fails only when `pipecommon_ioctl()` cannot take the device lock (for example, a signal during the wait), so no test can make it fail on demand. To show the leak, the test used a local test hook that is not part of this PR: `pipe2()` with `O_NOCTTY` makes the `FIONBIO` step fail with `EINTR`. The test applied the same hook to master and to this PR. `file_pipe()` has the same change; it was checked by reading the code, not with the hook. Test program `pipeleak.c` with `CONFIG_PIPES` on qemu-armv8a:knsh (kernel build, QEMU with HVF) and qemu-armv8a:nsh (flat build, the test as a builtin app), base 1b0691e870d: | Case | master | this PR | |---|---|---| | a byte written to `fd[1]` is read from `fd[0]`; `read()` returns 0 after the write end closes | PASS | PASS | | 1000 x `pipe()`, `pipe2(O_NONBLOCK)`, `pipe2(O_CLOEXEC)` + `close()`: the open descriptors do not change | PASS | PASS | | with the hook: 10 x `pipe2()` fail with `EINTR`, the open descriptors do not change | 4 -> 14 open: FAIL | 3 -> 3 open: PASS | The results are the same on knsh and on the flat nsh. Without the hook, the last case is skipped and all other cases pass on master and with this PR. The branch is rebased on 4a98cd7b44f; no commit after 1b0691e870d changes drivers/pipes. ostest exits with status 0 on qemu-armv8a:knsh, qemu-armv8a:nsh and the flat nsh with the test, on master and with this PR (without the hook). Build with `CONFIG_PIPES` and `EXTRAFLAGS="-Wno-cpp -Werror"`, each OK with no warnings: qemu-armv8a:nsh (flat, arm64, also with `CONFIG_PSEUDOTERM` for `file_pipe()`), rv-virt:pnsh64 (protected, rv64), mps2-an500:knsh (protected, Cortex-M7), qemu-armv8a:knsh (kernel, arm64), rv-virt:knsh64 (kernel, rv64). `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]
