Conversation
A SIGEV_SIGNAL | SIGEV_THREAD_ID timer skipped timer_create's signal check, and nxsig_notification honoured any thread ID. Check the signal where the event is delivered, keep SIGEV_THREAD_ID inside the owner's process as Linux does, and refuse both in timer_create so an expiring timer never fails its DEBUGVERIFY. Signed-off-by: Royyan Zahir <royzah@gmail.com>
|
| memcpy(&info.si_value, &event->sigev_value, sizeof(union sigval)); | ||
|
|
||
| /* SIGEV_THREAD_ID currently used only by POSIX timer. */ | ||
| if (!GOOD_SIGNO(event->sigev_signo)) |
There was a problem hiding this comment.
why need check again
There was a problem hiding this comment.
gpio, button and phy_notify store user sigevents unchecked, so this is the one spot every path goes thru. timer_create checks too, so the caller gets EINVAL, not a DEBUGVERIFY at expiry.
There was a problem hiding this comment.
but it's better to algin the check point between gpio/button/phy and timer.
There was a problem hiding this comment.
@xiaoxiang781216 ah ok got it, still learning this part so lemme check I get u right:
one small helper like nxsig_event_valid(), called at register time in timer_create + gpio + button + phy, then I drop the recheck in nxsig_notification?
but there is like ~15 more drivers that also take sigevent from user (joysticks, rtc, oneshot, aio, esp wifi ...). u want them all in this PR too, or ok to do in follow-up?
| { | ||
| FAR struct tcb_s *owner = nxsched_get_tcb(pid); | ||
| FAR struct tcb_s *target = | ||
| nxsched_get_tcb(event->sigev_notify_thread_id); |
There was a problem hiding this comment.
why check again too
There was a problem hiding this comment.
Same, those drivers take SIGEV_THREAD_ID from user space unchecked.
Note: Please adhere to Contributing Guidelines.
Summary
timer_create()checked the signal number only whensigev_notifywas exactlySIGEV_SIGNAL, soSIGEV_SIGNAL | SIGEV_THREAD_IDlet any number through to dispatch, and an expiring timer then failed itsDEBUGVERIFY.nxsig_notification()also honoured any thread ID, letting a process aim a timer, message queue or driver notification at a thread of another process.sig_notification.cSIGEV_THREAD_IDtarget must be in the owner's process, as on Linuxtimer_create.cEINVALImpact
Every build. A sigevent with a bad signal number, or aimed at another process's thread, is refused with
EINVAL.Testing
Host: Ubuntu 24.04, x86_64. Builds and QEMU runs in
ghcr.io/apache/nuttx/apache-nuttx-ci-linux(QEMU 6.2, Arm GNU GCC 13.2, xPack RISC-V GCC 14.3). Hardware: i.MX93 (Cortex-A55), PX4 kernel build.qemu-armv8a:knshostestpasses, its timer tests included;hellorv-virt:knsh64hello;osteststops afterStarted user_main at PID=6, as master doessim:ostest,qemu-armv8a:nshtimer_create()withSIGEV_THREAD_IDat another process's thread:EINVALqemu-armv8a:knsh:hello, thenostestrv-virt:knsh64:hello, thenostest; identical to master'si.MX93, PX4 kernel build, this series applied:
tests isolationtools/checkpatch.sh -c -u -m -gclean.