Brijesh-Thakkar opened a new issue, #21248:
URL: https://github.com/apache/nuttx/issues/21248
## Summary
Without `CONFIG_DEBUG_FEATURES`, `mq_send()` succeeds on a descriptor opened
`O_RDONLY` and `mq_receive()` succeeds on one opened `O_WRONLY`. POSIX
requires
`EBADF`. The checks are in `nxmq_verify_send()` / `nxmq_verify_receive()`
(`sched/mqueue/mq_send.c`, `mq_receive.c`), which are compiled only under
`#ifdef CONFIG_DEBUG_FEATURES`. The same applies to `EMSGSIZE` and to
`EINVAL`
for a priority above the maximum.
It shows up in LTP `open_posix_testsuite` `mq_open_7_1` (and about 50 other
`mq_*`
cases) on `rv-virt:citest64`, which does not set `DEBUG_FEATURES`.
`rv-virt:citest` (rv32) sets it, so the same tests pass there. It is not
architecture specific: rv32 `nsh` with `DEBUG_FEATURES` off fails the same
way.
## Environment
- nuttx `5128e6fe61` (upstream/master), nuttx-apps `385ae4294`
- xPack `riscv-none-elf-gcc` 14.3.0, QEMU 10.2.1, and `sim:nsh` with host
gcc 12
- configs `rv-virt:nsh64`, `rv-virt:nsh`, `sim:nsh`, each with
`CONFIG_DEBUG_FEATURES`
disabled and a small test app added
## Minimal reproduction
Mirrors LTP `mq_open/7-1.c`: create the queue `O_RDWR`, reopen it `O_RDONLY`,
then use that descriptor.
```c
mqd_t rw, ro, wo;
struct mq_attr attr = { .mq_maxmsg = 40, .mq_msgsize = 40 };
char buf[40];
unsigned pri;
mq_unlink("/rdonly");
rw = mq_open("/rdonly", O_CREAT | O_RDWR, S_IRUSR | S_IWUSR, &attr);
ro = mq_open("/rdonly", O_RDONLY, S_IRUSR | S_IWUSR, &attr);
errno = 0;
ret = mq_send(ro, "0123456789", 10, 1); /* must be -1 / EBADF */
mq_send(rw, "0123456789", 10, 1);
wo = mq_open("/rdonly", O_WRONLY, S_IRUSR | S_IWUSR, &attr);
errno = 0;
ret = mq_receive(wo, buf, sizeof(buf), &pri); /* must be -1 / EBADF */
```
Configure any board (e.g. `rv-virt:nsh64`), disable `CONFIG_DEBUG_FEATURES`,
build and run.
## Expected vs actual
Expected: `ret=-1 errno=9 (EBADF)` for both calls.
| build | `DEBUG_FEATURES` | `mq_send` on O_RDONLY | `mq_receive` on
O_WRONLY |
|---|---|---|---|
| rv-virt:nsh64 | n | `ret=0` | `ret=10` |
| rv-virt:nsh (rv32) | n | `ret=0` | `ret=10` |
| sim:nsh | n | `ret=0` | `ret=10` |
| any of the above | y | `-1`, `EBADF` | `-1`, `EBADF` |
## Which config makes it appear
Any config without `CONFIG_DEBUG_FEATURES`. In-tree, `rv-virt:citest64` is
one;
`rv-virt:citest` has it enabled.
## History
The checks were made debug-only in d09dc08eab ("sched/mqueue: do sanity
check if
DEBUG_FEATURES is enabled", chao.an, 2022). This also removed the
access-mode error
in non-debug builds, which POSIX requires (`EBADF`; the earlier unconditional
version returned `EPERM`). 1e7631e695 later moved the functions into
`mq_send.c` / `mq_receive.c` and kept the guard.
Regular `read()` / `write()` check the file's access mode unconditionally
(`fs/vfs/fs_read.c:203`, `fs/vfs/fs_write.c:157`; they return `EACCES`), so
mqueue is
the odd one out.
## Cost of the fix
Making only the POSIX-required checks unconditional (`EBADF`, `EMSGSIZE`,
`EINVAL`
for the priority) and keeping the NULL/sanity checks under `DEBUG_FEATURES`
was
measured without `DEBUG_FEATURES`, text size:
| config | before | after | delta |
|---|---|---|---|
| rv-virt:nsh | 225572 | 225628 | +56 bytes |
| rv-virt:nsh64 | 231838 | 231900 | +62 bytes |
| sim:nsh | 1179692 | 1180260 | +568 bytes |
I have a fix ready (POSIX-required checks unconditional, NULL/sanity checks
stay debug-only) and can open a PR if this approach is OK.
--
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]
