ARM: ptrace: Restore syscall skipping and restart while tracing
Subsystems (3)
v2 cover ARM: ptrace: Restore syscall skipping and restart while tracing
Hi,
Fix tracing on arm since commit 4e57a4ddf6b0 ("ARM: 9107/1: syscall:
always store thread_info->abi_syscall"). This was seen with the broken seccomp tests "syscall_errno", "syscall_faked", and "syscall_restart".
Thanks!
-Kees
v2:
- split fixes
- move scno store into the actual restart path
v1: https://lore.kernel.org/lkml/[email protected]
Kees Cook (2):
ARM: ptrace: Restore syscall restart tracing
ARM: ptrace: Restore syscall skipping for tracers
arch/arm/include/asm/syscall.h | 3 +++ arch/arm/kernel/entry-common.S | 1 + arch/arm/kernel/ptrace.c | 5 +++-- 3 files changed, 7 insertions(+), 2 deletions(-)
Signature · 1 line
2.34.1
PATCH v2 1/2 ARM: ptrace: Restore syscall restart tracing
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.
--- arch/arm/kernel/entry-common.S | 1 + 1 file changed, 1 insertion(+)
arch/arm/kernel/entry-common.S +1 -0
@@ -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)
Signature · 1 line
2.34.1
Re: [PATCH v2 1/2] ARM: ptrace: Restore syscall restart tracing
Quoted 13 lines · Kees Cook
Since commit
4e57a4ddf6b0("ARM: 9107/1: syscall: always storethread_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 theTIF_WORKpath. Add the missing store.arch/arm/kernel/entry-common.S +0
-0
arch/arm/kernel/entry-common.S slow_work_pending: · line 96
Quoted 7 lines
arch/arm/kernel/entry-common.S +1
-0@@ -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.
PATCH v2 2/2 ARM: ptrace: Restore syscall skipping for tracers
In reply to Kees CookSince commit 4e57a4ddf6b0 ("ARM: 9107/1: syscall: always store thread_info->abi_syscall"), the seccomp selftests "syscall_errno" and "syscall_faked" have been broken. Both seccomp and PTRACE depend on using the special value of "-1" for skipping syscalls. This value wasn't working because it was getting masked by __NR_SYSCALL_MASK in both PTRACE_SET_SYSCALL and get_syscall_nr().
Explicitly test for -1 in PTRACE_SET_SYSCALL and get_syscall_nr(), leaving it exposed when present, allowing tracers to skip syscalls again.
--- arch/arm/include/asm/syscall.h | 3 +++ arch/arm/kernel/ptrace.c | 5 +++-- 2 files changed, 6 insertions(+), 2 deletions(-)
arch/arm/include/asm/syscall.h +3 -0
@@ -25,6 +25,9 @@ static inline int syscall_get_nr(struct task_struct *task,if (IS_ENABLED (CONFIG_AEABI ) && !IS_ENABLED (CONFIG_OABI_COMPAT ))return task_thread_info (task)->abi_syscall; +if (task_thread_info (task)->abi_syscall ==-1 ) +return -1 ; +return task_thread_info (task)->abi_syscall & __NR_SYSCALL_MASK; }
arch/arm/kernel/ptrace.c +3 -2
@@ -783,8 +783,9 @@ long arch_ptrace(struct task_struct *child, long request,break ;case PTRACE_SET_SYSCALL : -task_thread_info (child)->abi_syscall = data & - __NR_SYSCALL_MASK; +if (data !=-1 ) + data &= __NR_SYSCALL_MASK; +task_thread_info (child)->abi_syscall = data; ret =0 ;break ;
Signature · 1 line
2.34.1
Re: [PATCH v2 2/2] ARM: ptrace: Restore syscall skipping for tracers
Quoted 11 lines · Kees Cook
Since commit
4e57a4ddf6b0("ARM: 9107/1: syscall: always storethread_info->abi_syscall"), the seccomp selftests "syscall_errno" and "syscall_faked" have been broken. Both seccomp and PTRACE depend on using the special value of "-1" for skipping syscalls. This value wasn't working because it was getting masked by__NR_SYSCALL_MASKin bothPTRACE_SET_SYSCALLandget_syscall_nr().Explicitly test for -1 in
PTRACE_SET_SYSCALLandget_syscall_nr(), leaving it exposed when present, allowing tracers to skip syscalls again.
Quoted 8 lines
Cc Russell King <[email protected]>Cc Arnd Bergmann <[email protected]>Cc Lecopzer Chen <[email protected]>Cc Oleg Nesterov <[email protected]>Fixes 4e57a4ddf6b0 ("ARM: 9107/1: syscall: always store
thread_info->abi_syscall")Signed-off-by Kees Cook <[email protected]>
Quoted 3 lines · Kees Cook
Fix tracing on arm since commit
4e57a4ddf6b0("ARM: 9107/1: syscall:
always storethread_info->abi_syscall"). This was seen with the broken seccomp tests "syscall_errno", "syscall_faked", and "syscall_restart".
I'd like to get this fixed in v6.5. Russell, do you have a fixes tree that you'll be sending Linus's way before the next -rc? I can send it via my seccomp-fixes tree if that'd be easier.
Thanks!
-Kees
Signature · 1 line
Kees Cook
Kees Cook
AppliedQuoted 3 lines · Kees Cook
Fix tracing on arm since commit
4e57a4ddf6b0("ARM: 9107/1: syscall:
always storethread_info->abi_syscall"). This was seen with the broken seccomp tests "syscall_errno", "syscall_faked", and "syscall_restart".
Applied to for-next/seccomp, thanks!
[1/2] ARM: ptrace: Restore syscall restart tracing
https://git.kernel.org/kees/c/cf007647475b [2/2] ARM: ptrace: Restore syscall skipping for tracers https://git.kernel.org/kees/c/4697b5848bd9
Take care,
Signature · 1 line
Kees Cook