casaroli opened a new pull request, #21143:
URL: https://github.com/apache/nuttx/pull/21143
## Summary
This PR fixes two results of the VFS that POSIX specifies and that programs
use to choose a fallback: `open()` with `O_TMPFILE` or with `O_DIRECTORY` and
write access, and `lseek()` of a pipe, a FIFO or a socket.
- **fs/vfs: Fail an open of a directory for writing or with O_TMPFILE.**
`open()` with `O_DIRECTORY` opened the directory whatever the access mode.
`O_TMPFILE` includes `O_DIRECTORY`, so `open(dir, O_RDWR | O_TMPFILE)` gave a
directory descriptor instead of an unnamed file: `open()` returned success and
every `write()` to it failed. A program that uses `mkstemp()` only when
`O_TMPFILE` fails (apt does) could not write its temporary file. Now an
`O_DIRECTORY` open with write access closes the directory again and fails: with
`EOPNOTSUPP` for `O_TMPFILE`, as Linux does for a file system without it (no
NuttX file system makes an unnamed file), else with `EISDIR`, as POSIX
specifies for a directory opened with `O_WRONLY` or `O_RDWR`. A path that is
not a directory still fails with `ENOTDIR`.
- **fs/vfs: Add the blank lines after declarations in fs_sendfile.c.**
Whitespace only: two older nxstyle errors in a file that the last commit edits.
- **fs/aio: Read and write a file that cannot seek at its position.** The
AIO workers used `file_pread()` and `file_pwrite()` at `aio_offset`. These
seek, so after the next commit they fail with `ESPIPE` for a pipe. POSIX
ignores `aio_offset` for a file that cannot seek: the workers now read and
write at the current position when they get `ESPIPE`. This commit comes first
so that no commit breaks AIO on a pipe.
- **fs/vfs: Fail lseek() of a pipe, a FIFO or a socket with ESPIPE.** These
drivers have no seek method, so `lseek()` set the common file position and
returned it: `lseek(fd, 10, SEEK_CUR)` of a pipe returned 10, and `SEEK_END`
failed with `ENOSYS`. POSIX specifies `ESPIPE`. A program that skips data with
`lseek()` and reads only when `lseek()` fails lost the data (dpkg does this
with the tar stream from dpkg-deb). `file_seek()` now returns `-ESPIPE` for
these inodes; other drivers without a seek method keep the common file
position. `pread()`, `pwrite()` and `sendfile()` with an offset then also fail
with `ESPIPE` for them, as POSIX and Linux specify. The `sendfile()` path of a
TCP socket (`CONFIG_NET_SENDFILE`) seeks its input to send data again: for an
input that cannot seek, `sendfile()` now uses the read and write copy, as it
does when that path returns `ENOSYS`.
## Impact
Possible breaking change (results and `errno` values of the flat API too,
every build mode and architecture):
| Call | before | after |
|---|---|---|
| `open(dir, O_RDWR \| O_TMPFILE)` | a directory descriptor that cannot be
written | -1, `EOPNOTSUPP` |
| `open(dir, O_WRONLY or O_RDWR \| O_DIRECTORY)` | a directory descriptor |
-1, `EISDIR` |
| `lseek()` of a pipe, FIFO or socket | a made-up position (`SEEK_END`:
`ENOSYS`) | -1, `ESPIPE` |
| `pread()`/`pwrite()` of a pipe, FIFO or socket | read or write, the offset
ignored | -1, `ESPIPE` |
| `ftell()` of a stream on a pipe | a made-up position | -1, `ESPIPE` |
| `sendfile()` from a pipe with an offset | the offset ignored | -1,
`ESPIPE` |
`O_RDONLY | O_DIRECTORY` (`opendir()`) does not change; reads and writes of
pipes, FIFOs and sockets do not change; `sendfile()` from a pipe without an
offset still works (also to a TCP socket with `CONFIG_NET_SENDFILE`).
In-tree users, nuttx and nuttx-apps master: no code uses `O_TMPFILE`, and
every `O_DIRECTORY` open is read-only. Of the `lseek()`, `fseek()`, `ftell()`,
`pread()`, `pwrite()` and `file_seek()` callers, these can see a pipe, a FIFO
or a socket:
- `fs/aio` and the TCP `sendfile()`: changed in this PR (see above).
- `fcntl(F_SETFL)` with `O_APPEND` seeks to the end: on a pipe it failed
before (`ENOSYS`) and fails now (`ESPIPE`).
- stdio: `fwrite()` after buffered reads and `fseek()` seek back over the
read-ahead; on a pipe this failed before (`EINVAL`: the position became
negative) and fails now (`ESPIPE`).
- `dd skip=N` with a pipe as input: before, `dd` copied from the start
without skipping; now it stops with "failed to lseek".
- NSH loops (`while`/`until`) in a script that is a FIFO: never worked; now
NSH prints an `lseek` error for each line.
All other callers seek regular files or block and character devices, which
do not change.
## Testing
Tested on master 192ac6d3fbc and on this PR rebased on it, with QEMU (HVF):
`qemu-armv8a:knsh` (kernel build, programs on v9fs) and `qemu-armv8a:nsh` (flat
build, the programs as NSH built-ins), with `CONFIG_FS_TMPFS` and
`CONFIG_PIPES`. The flat build also has `CONFIG_FS_AIO` and TCP over the
loopback device with `CONFIG_NET_SENDFILE`, for the socket cases.
`tmpfile.c`: `open()` of a tmpfs and of a v9fs directory (the same result on
both):
| Case | master | this PR |
|---|---|---|
| `open(dir, O_RDWR \| O_TMPFILE \| O_EXCL, 0600)` | a descriptor; `write()`
fails with `EINVAL`: FAIL | -1, `EOPNOTSUPP`: PASS |
| `open(dir, O_RDWR \| O_DIRECTORY)` | a descriptor: FAIL | -1, `EISDIR`:
PASS |
| `open(dir, O_RDONLY \| O_DIRECTORY)` | a descriptor: PASS | a descriptor:
PASS |
`pipeseek.c`:
| Case | master | this PR |
|---|---|---|
| `lseek(read end, 10, SEEK_CUR)` | 10: FAIL | -1, `ESPIPE`: PASS |
| `lseek(write end, 0, SEEK_END)` | -1, `ENOSYS`: FAIL | -1, `ESPIPE`: PASS |
| the 16 bytes in the pipe are read after the `lseek()` | PASS | PASS |
| `lseek()` of a FIFO | 0: FAIL | -1, `ESPIPE`: PASS |
| `lseek()` of a socket (flat build) | 0: FAIL | -1, `ESPIPE`: PASS |
`g8extra.c`, more cases:
| Case | master | this PR |
|---|---|---|
| `open(regular file, O_RDWR \| O_DIRECTORY)` | -1, `ENOTDIR` | -1,
`ENOTDIR` |
| `open(regular file, O_RDWR \| O_TMPFILE)` | -1, `ENOTDIR` | -1, `ENOTDIR` |
| `open("/dev", O_WRONLY \| O_DIRECTORY)` | a descriptor | -1, `EISDIR` |
| 200 failed directory opens on a tmpfs, then `umount()` | the opens
succeed; `umount()` 0 | the opens fail; `umount()` 0 (no reference leaked) |
| `pread()` / `pwrite()` of a pipe | 5 / 1 | -1, `ESPIPE` |
| `ftell()` of a stream on a pipe | 0 | -1, `ESPIPE` |
| `sendfile(file, pipe, NULL, 5)` | 5 | 5 |
| `sendfile(file, pipe, &off, 5)` | 5 | -1, `ESPIPE` |
| `sendfile(TCP socket, pipe, NULL, 5)` (flat build) | 5 | 5 (the read and
write copy) |
`aiotest.c` (flat build; AIO writes and reads, `lio_listio()`, and requests
on a pipe): the same result on master and with this PR (3 of 6 cases pass on
both; the other three fail with the same values on master).
ostest exits with status 0 on `qemu-armv8a:knsh` and `qemu-armv8a:nsh`, on
master and with this PR.
Builds, each OK and without new warnings: `lm3s6965-ek:nsh` (flat,
Cortex-M3) with `CONFIG_FS_AIO`, `CONFIG_NET_SENDFILE`, `CONFIG_PIPES` and
`CONFIG_FS_TMPFS`; `lm3s6965-ek:qemu-protected` (protected, Cortex-M3);
`rv-virt:pnsh64` (protected, RISC-V) and `rv-virt:knsh64` (kernel, RISC-V) with
`CONFIG_PIPES` and `CONFIG_FS_TMPFS`.
`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]
