casaroli opened a new pull request, #21211: URL: https://github.com/apache/nuttx/pull/21211
## Summary Three fixes for System V message queues, one commit each: - **sched/mqueue: Use is_idle_task() in msgrcv() and msgsnd().** Both check `DEBUGASSERT(NULL != rtcb->flink)` before they block. In an SMP build the running task of a CPU can be the only task in its assigned task list, so its `flink` is NULL, and with `CONFIG_DEBUG_ASSERTIONS` the first `msgrcv()` or `msgsnd()` that waits stops the system (`mqueue/msgrcv.c:129` on qemu-armv8a:nsh_smp). 18266c10129 changed the same check to `is_idle_task()` in the POSIX message queues, the semaphores and the signals; the System V code kept the old one. - **sched/mqueue: Return the POSIX error numbers from msgrcv() and msgsnd().** `msgrcv()` with `IPC_NOWAIT` and no matching message failed with `EAGAIN` (POSIX: `ENOMSG`). `msgrcv()` failed with `EMSGSIZE` whenever `msgsz` was less than `CONFIG_MQ_MAXMSGSIZE`, also when the message fits; now it compares `msgsz` with the selected message, and a longer message stays in the queue with `E2BIG` (with `MSG_NOERROR` it is truncated, as before). `msgsnd()` failed with `EMSGSIZE` for a too large `msgsz` and accepted a type less than 1 (POSIX: `EINVAL` for both). The errno lists in `include/sys/msg.h` and the two `.c` files are updated. - **sched/mqueue: Wake up the waiters of a removed System V message queue.** `msgctl(IPC_RMID)` freed the queue but did not wake up the tasks blocked in `msgrcv()` or `msgsnd()` on it. They stayed blocked forever, and their TCB `waitobj` pointed to the freed queue, so a later signal, cancellation or exit wrote to freed memory. A task that was woken but did not run before the removal also used the freed queue. `nxmsg_free()` now takes the queue out of the table first, wakes up all waiters with `EIDRM` (one context switch at the end, as `nxevent_post()` does), and then frees the messages and the queue. After a wake-up, `msgrcv()` and `msgsnd()` look up the identifier again and fail with `EIDRM` when the queue is gone. POSIX requires `EIDRM`; it is added to the errno lists. ## Impact Only builds with System V message queues (`CONFIG_DISABLE_MQUEUE_SYSV` not set); the same in flat, protected and kernel builds. No new memory, no new option. The second commit changes errno values of the flat API, so it is a possible breaking change for code that checks them: - `msgrcv(..., IPC_NOWAIT)` with no matching message: `ENOMSG` (was `EAGAIN`). - `msgrcv()` of a message longer than `msgsz`, no `MSG_NOERROR`: `E2BIG`, the message stays (was `EMSGSIZE`, also when the message fits). - `msgrcv()` with `msgsz` less than `CONFIG_MQ_MAXMSGSIZE` and a message that fits: success (was `EMSGSIZE`). - `msgsnd()` with `msgsz` above the maximum: `EINVAL` (was `EMSGSIZE`); with `mtype` less than 1: `EINVAL` (was success). - A task blocked on a removed queue: -1 with `EIDRM` (was blocked forever). No code in nuttx or nuttx-apps calls `msgrcv()`, `msgsnd()` or checks these values (grep of both repositories). ## Testing QEMU with HVF, `CONFIG_DEBUG_ASSERTIONS=y` and `CONFIG_MM_FILL_ALLOCATIONS=y`. The branch was tested on 23f5a4f06df (master 7505b874722 does not build for arm64, fixed by #21210); no file of `sched/mqueue` or `include/sys/msg.h` changed since 3122150b1f1, where the master column was taken. Test `msgrmid.c` (threads with `SCHED_FIFO` priorities; a thread that does not return in 3 s is a hang): | Case | master | this PR | |---|---|---| | `rcv`: blocked in `msgrcv()`, then `IPC_RMID` | hangs; at process exit the kernel asserts in `mm_heap/mm_free.c:167` (heap corrupted) | -1 `EIDRM` | | `snd`: blocked in `msgsnd()` of a full queue | - | -1 `EIDRM` | | `both`: 3 receivers and 3 senders | - | all -1 `EIDRM` | | `wokenrcv`: receiver woken by `msgsnd()`, `IPC_RMID` before it runs | kernel data abort, FAR `0x5555555555555555` (freed memory) | -1 `EIDRM` | | `wokensnd`: the same for a sender | kernel data abort, FAR `0x55555555555555b1` | -1 `EIDRM` | | `sigrmid`: a signal, then `IPC_RMID` | - | -1 `EINTR` | | `rmidsig`: `IPC_RMID`, then a signal | - | -1 `EIDRM`, the handler runs | | `stress`: 300 rounds, woken and signal variants | - | no hang, no assertion, bad 0 (1 CPU: `EIDRM` 1125, `EINTR` 75; 2 CPUs: `EIDRM` 681, `EINTR` 38, `EINVAL` 481 when a thread did not block yet) | Test `msgrcverr.c` (a new queue for each case, `CONFIG_MQ_MAXMSGSIZE=32`): | Case | master | this PR | |---|---|---| | `msgrcv()` of 5 bytes, `msgsz` 8 | `EMSGSIZE` | 5 bytes | | `msgrcv()` of 20 bytes, `msgsz` 8 | `EMSGSIZE` | `E2BIG`, the message stays; then `msgsz` 64: 20 bytes | | `msgrcv()` of 20 bytes, `msgsz` 8, `MSG_NOERROR` | 8 bytes | 8 bytes | | `msgrcv(IPC_NOWAIT)`, empty queue / other type | `EAGAIN` | `ENOMSG` (the message stays) | | `msgsnd()` with `mtype` 0 / `msgsz` 33 | success / `EMSGSIZE` | `EINVAL` / `EINVAL` | | `msgsnd()` + `msgrcv()` of 32 bytes | OK | OK | Results of this PR: qemu-armv8a:knsh (kernel) and qemu-armv8a:nsh (flat): msgrmid 9/9, msgrcverr 9/9, and the existing `msgsys` test (`msgget`/`msgsnd`/`IPC_STAT`/`msgrcv`/`IPC_RMID`) 6/6; ostest exits with status 0 on both. qemu-armv8a:nsh_smp (2 CPUs): msgrmid 9/9 and msgrcverr 9/9 without an assertion (master stops at the assertion in `msgrcv.c` at the first wait). The master columns: knsh and flat on 3122150b1f1; the woken cases ran first in separate boots, because the first failing case stops the program. No test in nuttx-apps covers System V message queues. Build, each OK, no warning in `sched/mqueue/msg*`: rv-virt:knsh64 (kernel, rv64), rv-virt:pnsh64 (protected, rv64), mps2-an500:knsh (protected, Cortex-M7), mps2-an500:nsh with `CONFIG_DEBUG_ASSERTIONS` (flat, Cortex-M7), qemu-armv8a:knsh with `CONFIG_DISABLE_MQUEUE=y` (System V only), qemu-armv8a:nsh with `CONFIG_DISABLE_MQUEUE_SYSV=y` (no msg code), qemu-armv8a:nsh_smp with `CONFIG_DEBUG_ASSERTIONS` (SMP). `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]
