xiaoxiang781216 commented on code in PR #21140: URL: https://github.com/apache/nuttx/pull/21140#discussion_r4420431282
########## arch/arm/src/armv7-r/arm_syscall.c: ########## @@ -42,6 +43,22 @@ #include "sched/sched.h" #include "signal/signal.h" +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +/* From user mode, the only reserved system call that is allowed is the + * return from a signal handler that the kernel dispatched. Without full + * signal support the kernel dispatches no handler, so none is allowed. + */ + +#if defined(CONFIG_BUILD_PROTECTED) && defined(CONFIG_ENABLE_ALL_SIGNALS) +# define USER_SIGRETURN(tcb, cmd) \ + ((cmd) == SYS_signal_handler_return && (tcb)->xcp.sigreturn != 0) Review Comment: > @xiaoxiang781216 That would definitely be much cleaner and eliminate the `USER_SIGRETURN` macro across all architectures > > The reason we implemented it this way in this PR was to achieve 1:1 consistency with the existing ARM64 (commit [50a4f80](https://github.com/apache/nuttx/commit/50a4f8061daec54c4ed085d25ca6d0e6c746cc17) / [a9066e5](https://github.com/apache/nuttx/commit/a9066e5de2306c5fff2424fed6f0e095776552c9)) and RISC-V (commit [fd57ce7](https://github.com/apache/nuttx/commit/fd57ce769570a32a8f403e65d8aecb9f2bb75db8) / [03fd785](https://github.com/apache/nuttx/commit/03fd785795e1d1dfa2613b07d25185bb4c8bf1ad)) implementations without altering the current syscall numbering or signal trampoline ABI. > let's choice the simple method? since it make the code more maintainable. > If we remove `SYS_signal_handler_return` from the reserved region: > > 1. We can add `signal_handler_return` to `syscall/syscall.csv` (or assign it a number > ≥ > `CONFIG_SYS_RESERVED`) and let `case SYS_signal_handler_return:` intercept it. > 2. `CONFIG_SYS_RESERVED` becomes strictly kernel-only (`SYS_restore_context` through `SYS_signal_handler`), making the user-mode guard a simple: > ```c > if (is_user_mode && cmd < CONFIG_SYS_RESERVED) > { > return -ENOSYS; > } > ``` > 3. We would also align ARM64 and RISC-V to drop `USER_SIGRETURN`. > > Would you prefer we transition `SYS_signal_handler_return` to `syscall.csv` in this PR (and harmonize ARM64/RISC-V), or keep this PR aligned with the existing ARM64/RISC-V pattern and do the broader cross-arch refactoring in a follow-up? let's focus on arm in this PR and apply the same change to riscv/arm64 in the separated patch. -- 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]
