Brijesh-Thakkar opened a new pull request, #21487:
URL: https://github.com/apache/nuttx/pull/21487
## Summary
Without `CONFIG_DEBUG_FEATURES`, `mq_send()` succeeds on a descriptor opened
`O_RDONLY` and `mq_receive()` succeeds on one opened `O_WRONLY`. Oversized
messages and a priority above the maximum are accepted as well. POSIX
requires
`EBADF`, `EMSGSIZE` and `EINVAL` for these, so conforming behavior depended
on a
debug option.
Root cause: `nxmq_verify_send()` / `nxmq_verify_receive()` and their calls
are
compiled only under `#ifdef CONFIG_DEBUG_FEATURES`. d09dc08eab
("sched/mqueue: do
sanity check if DEBUG_FEATURES is enabled") put them there, and 1e7631e695
kept
the guard when moving them into `mq_send.c` / `mq_receive.c`. The VFS paths
(`fs/vfs/fs_read.c`, `fs/vfs/fs_write.c`) check the access mode
unconditionally.
Fix: always build the verify functions and always call them. Only the pure
sanity checks stay under `CONFIG_DEBUG_FEATURES` (NULL inode, NULL message
pointer, NULL queue). `mq_send`, `mq_timedsend`, `mq_receive`,
`mq_timedreceive`
and every `nxmq_*` / `file_mq_*` variant go through
`file_mq_timedsend_internal()` / `file_mq_timedreceive_internal()`, so they
all
get the checks. The order of the checks in debug builds is unchanged.
Fixes #21248
## Impact
- User-visible: in builds without `DEBUG_FEATURES`, `mq_send()` /
`mq_receive()`
(and the timed, `nxmq_*` and `file_mq_*` variants) now fail with `EBADF` /
`EMSGSIZE` / `EINVAL` as POSIX specifies. Code that relied on sending
through a
read-only descriptor, or on oversized messages or `prio >= MQ_PRIO_MAX`
being
accepted, will now get an error. Builds with `DEBUG_FEATURES` behave as
before.
- No API, Kconfig or documentation change.
- In-tree kernel callers audited (every `file_mq_*` / `nxmq_*` / `_MQ_*`
user in
`drivers/`, `audio/`, `arch/`, `wireless/`, `graphics/`, `libs/libnx`):
all use `O_RDWR` queues, except NX, where the `O_RDONLY` queue only
receives
and the `O_WRONLY` queues only send. Every kernel receive passes a length
equal
to the queue's `mq_msgsize`, and no send exceeds it. None is affected.
`apps/` was spot-checked: the remaining `O_RDONLY` / `O_WRONLY` opens are
reader/writer pairs.
- Size (text, `DEBUG_FEATURES` disabled, plain config plus a small test app
in
both columns):
| config | before | after | delta |
|---|---|---|---|
| rv-virt:nsh64 | 234454 | 234516 | +62 |
| rv-virt:nsh | 228196 | 228252 | +56 |
| sim:nsh | 1193316 | 1193300 | -16 |
The numbers in #21248 (+56 / +62 / +568) were measured on an older
baseline;
the table above is current master. With `DEBUG_FEATURES` enabled the size
is
unchanged apart from noise (0 / -2 bytes on nsh64 / nsh).
## Testing
Host: Ubuntu 26.04 LTS, Linux 7.0.0. Toolchains: xPack `riscv-none-elf-gcc`
14.3.0 (RISC-V), host gcc 12.5.0 (sim), QEMU 10.2.1 (`qemu-system-riscv64` /
`-riscv32`). All tests below were run on apache/nuttx `1cc41cbcb1` with
nuttx-apps `54374b3b2`. The commit was then rebased cleanly onto
upstream/master
`6e1126a236` (the upstream changes in `sched/mqueue/` only touch the System V
`msg*.c` files) and `sim:nsh` was rebuilt without warnings.
**Testing was done on QEMU (rv-virt) and the sim only; no real hardware.**
Configs: `rv-virt:nsh64`, `rv-virt:nsh`, `sim:nsh`, `rv-virt:citest64`,
`sim:ostest`, each with `CONFIG_DEBUG_FEATURES` disabled, and the first three
also with the stock `=y`. A test app (not part of this PR) runs 13 cases:
`mq_send` / `mq_timedsend` on `O_RDONLY`, `mq_receive` / `mq_timedreceive` on
`O_WRONLY` (`EBADF`), send with `msglen > mq_msgsize` and receive with a
short
buffer (`EMSGSIZE`), `prio == MQ_PRIO_MAX` and `prio > MQ_PRIO_MAX`
(`EINVAL`),
and valid-use controls.
| build | DEBUG off, before | DEBUG off, after | DEBUG on, before / after |
|---|---|---|---|
| rv-virt:nsh64 | 5/13 | 13/13 | 13/13 / 13/13 |
| rv-virt:nsh | 5/13 | 13/13 | 13/13 / 13/13 |
| sim:nsh | 5/13 | 13/13 | 13/13 / 13/13 |
Before (DEBUG off, nsh64): `mq_send` on `O_RDONLY` returns 0, `mq_receive` on
`O_WRONLY` returns 10.
After:
```
[PASS] mq_send on O_RDONLY ret=-1 errno=9 (want ret=-1
errno=9)
[PASS] mq_receive on O_WRONLY ret=-1 errno=9 (want ret=-1
errno=9)
[PASS] mq_send msglen > mq_msgsize ret=-1 errno=90 (want ret=-1
errno=90)
[PASS] mq_send prio == MQ_PRIO_MAX ret=-1 errno=22 (want ret=-1
errno=22)
RESULT: 13/13 passed (sizeof(long)=8)
```
LTP `open_posix_testsuite` `mq_*` (93 cases) on `rv-virt:citest64` with LTP
enabled, `CONFIG_MQ_MAXMSGSIZE=128`, `DEBUG_FEATURES` off, each case run in
nsh under QEMU:
| | pass | fail | timeout |
|---|---|---|---|
| before | 74 | 18 | 1 |
| after | 93 | 0 | 0 |
19 cases are fixed (`mq_open_7_1`, `mq_open_8_1`,
`mq_receive_{2_1,11_2,12_1}`,
`mq_send_{2_1,4_1,4_2,11_2,13_1,14_1}`, `mq_timedreceive_{2_1,15_1}`,
`mq_timedsend_{2_1,4_1,4_2,11_2,13_1,14_1}`) and none regressed.
ostest (`sim:ostest`, `DEBUG_FEATURES` disabled): before and after are
identical, every `nerrors=0`, and the mqueue and timedmqueue tests complete
in
every loop. The run was cut after 4 of the 5 configured loops by the
900 s timeout of my test harness, on unmodified master as well.
All builds are free of compiler warnings (the RISC-V linker prints its usual
"LOAD segment with RWX permissions" notice, before and after).
`./tools/checkpatch.sh -c -u -m -g upstream/master..HEAD` 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]
