| Message ID | 20230810195422.2304827-1-keescook@chromium.org (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | cf007647475b5090819c5fe8da771073145c7334 |
| Headers | show |
| Series | ARM: ptrace: Restore syscall skipping and restart while tracing | expand |
On Thu, Aug 10, 2023, at 21:54, Kees Cook wrote: > Since commit 4e57a4ddf6b0 ("ARM: 9107/1: syscall: always store > thread_info->abi_syscall"), the seccomp selftests "syscall_restart" has > been broken. This was caused by the restart syscall not being stored to > "abi_syscall" during restart setup before branching to the "local_restart" > label. Tracers would see the wrong syscall, and scno would get overwritten > while returning from the TIF_WORK path. Add the missing store. > > diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S > index bcc4c9ec3aa4..5c31e9de7a60 100644 > --- a/arch/arm/kernel/entry-common.S > +++ b/arch/arm/kernel/entry-common.S > @@ -90,6 +90,7 @@ slow_work_pending: > cmp r0, #0 > beq no_work_pending > movlt scno, #(__NR_restart_syscall - __NR_SYSCALL_BASE) > + str scno, [tsk, #TI_ABI_SYSCALL] @ make sure tracers see update > ldmia sp, {r0 - r6} @ have to reload r0 - r6 > b local_restart @ ... and off we go > ENDPROC(ret_fast_syscall) Looks good to me, as in my previous reply this is safe for all cases with the current restartable syscalls, though it would be wrong if we had a restartblock version of epoll_wait on an OABI_COMPAT task, which needs to look at the high bits of the number to tell which structure layout to use. I also looked at my original patch now and see what I did wrong there. Reviewed-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S index bcc4c9ec3aa4..5c31e9de7a60 100644 --- a/arch/arm/kernel/entry-common.S +++ b/arch/arm/kernel/entry-common.S @@ -90,6 +90,7 @@ slow_work_pending: cmp r0, #0 beq no_work_pending movlt scno, #(__NR_restart_syscall - __NR_SYSCALL_BASE) + str scno, [tsk, #TI_ABI_SYSCALL] @ make sure tracers see update ldmia sp, {r0 - r6} @ have to reload r0 - r6 b local_restart @ ... and off we go ENDPROC(ret_fast_syscall)
Since commit 4e57a4ddf6b0 ("ARM: 9107/1: syscall: always store thread_info->abi_syscall"), the seccomp selftests "syscall_restart" has been broken. This was caused by the restart syscall not being stored to "abi_syscall" during restart setup before branching to the "local_restart" label. Tracers would see the wrong syscall, and scno would get overwritten while returning from the TIF_WORK path. Add the missing store. Cc: Russell King <linux@armlinux.org.uk> Cc: Arnd Bergmann <arnd@kernel.org> Cc: Lecopzer Chen <lecopzer.chen@mediatek.com> Cc: Oleg Nesterov <oleg@redhat.com> Cc: linux-arm-kernel@lists.infradead.org Fixes: 4e57a4ddf6b0 ("ARM: 9107/1: syscall: always store thread_info->abi_syscall") Signed-off-by: Kees Cook <keescook@chromium.org> --- arch/arm/kernel/entry-common.S | 1 + 1 file changed, 1 insertion(+)