casaroli opened a new pull request, #21367:
URL: https://github.com/apache/nuttx/pull/21367
## Summary
A `read()` from a descriptor that is open only for writing, or a `write()`
to a descriptor that is open only for reading, failed with `EACCES`. POSIX
specifies `EBADF` for both: read(): "[EBADF] The fildes argument is not a valid
file descriptor open for reading."; write(): "[EBADF] The fildes argument is
not a valid file descriptor open for writing." The comments of `write()` and
`writev()` in `fs/vfs/fs_write.c`, and the `aio_read()` comment, already
document `EBADF`. Before #3491 (2021), `write()` returned `EBADF`; that PR
changed it to `EACCES` to match `read()`.
The two commits:
- **fs/vfs: Return EBADF for a read or write in the wrong access mode.**
`file_read()`, `file_readv()`, `file_write()` and `file_writev()` return
`-EBADF`. This covers `read()`, `readv()`, `pread()`, `write()`, `writev()`,
`pwrite()`, the pipe and FIFO ends, `sendfile()` and the `aio_read()` worker,
which all use these functions.
- **libs/libc/stdio: Set errno to EBADF in fread() on a write-only stream.**
`fread()` (and `fgetc()`) on a stream that is open only for writing set the
error indicator but did not set `errno`: `_NX_SETERRNO(EBADF)` does nothing in
user space and in the flat build, and sets `errno` to `-EBADF` in the kernel
copy of libc. It now uses `set_errno(EBADF)`, as `fwrite()` does.
## Impact
- All build modes and architectures: the change is in common VFS and libc
code.
- A program that writes to a read-only descriptor or reads from a write-only
descriptor now gets `EBADF`, not `EACCES`. This is a change of an error code
that flat-build programs can see. Nothing in nuttx tests for `EACCES` from
these calls: the in-tree users of `-EACCES` (binfmt execute permission, the
mx8mp I2C driver, the OP-TEE errno conversion, `nftw()`, the sim host file
system) do not get it from a read or a write. In nuttx-apps, only the cmocka
syscall test `pipe02` expects `EACCES`; apache/nuttx-apps#XXXX changes it to
`EBADF`. No CI configuration enables that test (`CONFIG_CM_SYSCALL_TEST`).
- `stdio`: `fwrite()` on a read-only stream already gave `EBADF`. `fread()`
on a write-only stream now sets `errno` to `EBADF`.
- The `open()` checks of the file systems and the procfs entries keep
`EACCES`: there it is the correct error for an access mode that the file does
not allow. `mmap()` keeps `EACCES`, as POSIX specifies. The read, write and
truncate methods of fat, hostfs, nxffs, rpmsgfs, smartfs and spiffs have their
own access mode checks with `EACCES`; the VFS check runs first, so a program
never gets those. This PR does not change them.
- No documentation change: Documentation/ does not list the error codes of
`read()` and `write()`.
## Testing
New test program `rw_ebadf.c` (23 checks): `write()`, `writev()` and
`pwrite()` on an `O_RDONLY` file; `read()`, `readv()` and `pread()` on an
`O_WRONLY` file; `sendfile()` with an `O_WRONLY` input and with an `O_RDONLY`
output; `write()`/`writev()` on the read end and `read()`/`readv()` on the
write end of a pipe; `fwrite()` on `fopen("r")` and `fread()` on `fopen("w")`.
Each must fail with `EBADF`. The normal reads and writes of the same files and
pipe must still work. The files are on tmpfs.
| configuration | master | this PR |
| --- | --- | --- |
| qemu-armv8a:knsh (kernel, arm64, QEMU with HVF) | 10 PASS, 13 FAIL: the 12
wrong-mode calls give `EACCES`, `fread()` leaves `errno` at 0 | 23 PASS |
| qemu-armv8a:nsh (flat, arm64, HVF) | 10 PASS, 13 FAIL (same) | 23 PASS |
| mps2-an500:knsh (protected, Cortex-M7, QEMU) | 10 PASS, 13 FAIL (same) |
23 PASS |
`fwrite()` on `fopen("r")` gives `EBADF` on master already. With only the
first commit, the 12 calls give `EBADF` and only `fread()` still fails
(qemu-armv8a:knsh and :nsh).
ostest exits with status 0 on master and with this PR: qemu-armv8a:knsh and
qemu-armv8a:nsh (both with `CONFIG_PIPES` and `CONFIG_FS_TMPFS`), and the flat
qemu-armv8a:nsh build with the test.
The cmocka syscall test `pipe02` (qemu-armv8a:nsh with
`CONFIG_CM_SYSCALL_TEST`, `cmocka -s cmocka_syscall_test -t
test_nuttx_syscall_pipe0.*`): passes on master with nuttx-apps master; fails
with this PR and nuttx-apps master (`pipe_test.c:92`, the `EACCES` check);
passes with this PR and apache/nuttx-apps#XXXX. `pipe01` passes in all three
runs.
Builds with `EXTRAFLAGS="-Wno-cpp -Werror"`, each OK without compiler
warnings: qemu-armv8a:knsh, qemu-armv8a:nsh, mps2-an500:knsh and sim:nsh (macOS
host; the only warnings come from the host linker, `-ld_classic`).
The tests ran on master 1b0691e870d; the branch is rebased on 4a98cd7b44f
(no change in the touched files).
`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]
