arm64: ptrace: Keep 'orig_x0' in-sync with x0 on syscall entry
Subsystems (2)
Will Deacon
maintainerPATCH v3 1/1 arm64: ptrace: Keep 'orig_x0' in-sync with x0 on syscall entry
Commit e057b9477232 ("arm64: syscall: Ensure saved x0 is kept in-sync with tracer updates") attempted to resolve a long-standing issue with syscall entry tracing, where a tracer is able to manipulate the first syscall argument without being subjected to seccomp or audit checking.
Unfortunately, that fix was incomplete [1], as it failed to update 'orig_x0' between a tracer updating x0 during a seccomp ptrace exit (SECCOMP_RET_TRACE) and the seccomp filter being re-evaluated.
Rather than add hooks to the core seccomp code, instead move the synchronisation code into the ptrace GPR and syscall setting code so that 'orig_x0' is kept up to date with x0 whenever we're stopped on the syscall entry path.
--- arch/arm64/kernel/ptrace.c | 50 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+)
v2: https://lore.kernel.org/r/[email protected]
arch/arm64/kernel/ptrace.c +50 -0
@@ -560,6 +560,42 @@ static int gpr_get(struct task_struct *target,return membuf_write (&to, uregs,sizeof (*uregs)); } +static void update_syscall_orig_x0_after_ptrace (struct task_struct *target) +{ +struct pt_regs *regs =task_pt_regs (target); +struct kernel_siginfo *info = target->last_siginfo; + +/* We should only be called when target is in a ptrace stop */ +if (WARN_ON_ONCE (!info)) +return ; + +/* +* Skip the update for NO_SYSCALL (set either by the user or the +* tracer), as regs[0] holds the return value (see the comment in +* el0_svc_common()) and can be unwound using syscall_rollback(). +*/ +if (regs->syscallno ==NO_SYSCALL ) +return ; + +/* +* For compat tasks, orig_r0 is provided directly through GPR index +* 17. +*/ +if (is_compat_thread (task_thread_info (target))) +return ; + +/* +* Don't update orig_x0 for a syscall-exit-stop, as x0 now contains the +* return value of the system call. +*/ +if ((info->si_code & ~0x80 ) ==SIGTRAP && + target->ptrace_message ==PTRACE_EVENTMSG_SYSCALL_EXIT ) { +return ; + } + + regs->orig_x0 = regs->regs[0 ]; +} +static int gpr_set (struct task_struct *target,const struct user_regset *regset,unsigned int pos,unsigned int count,const void *kbuf,const void __user *ubuf)
@@ -575,6 +611,14 @@ static int gpr_set(struct task_struct *target, const struct user_regset *regset,return -EINVAL ;task_pt_regs (target)->user_regs = newregs; + +/* +* Keep orig_x0 authoritative so that seccomp (via +* syscall_get_arguments()), audit and the restart path all see the same +* first argument the syscall is dispatched with, even if it has been +* updated by a tracer. +*/ +update_syscall_orig_x0_after_ptrace (target);return 0 ; }
@@ -753,6 +797,12 @@ static int system_call_set(struct task_struct *target,return ret;task_pt_regs (target)->syscallno = syscallno; + +/* +* Re-sync orig_x0 in case the syscall number has been changed +* from NO_SYSCALL. +*/ +update_syscall_orig_x0_after_ptrace (target);return ret; }
Signature · 1 line
2.55.0.229.g6434b31f56-goog
Quoted 78 lines · Will Deacon
Commit
e057b9477232("arm64: syscall: Ensure saved x0 is kept in-sync with tracer updates") attempted to resolve a long-standing issue with syscall entry tracing, where a tracer is able to manipulate the first syscall argument without being subjected to seccomp or audit checking.Unfortunately, that fix was incomplete [1], as it failed to update '
orig_x0' between a tracer updating x0 during a seccomp ptrace exit (SECCOMP_RET_TRACE) and the seccomp filter being re-evaluated.Rather than add hooks to the core seccomp code, instead move the synchronisation code into the ptrace GPR and syscall setting code so that '
orig_x0' is kept up to date with x0 whenever we're stopped on the syscall entry path.Cc Kees Cook <[email protected]>Cc Jinjie Ruan <[email protected]>Cc Mark Rutland <[email protected]>Reported-by Yiqi Sun <[email protected]>Fixes e057b9477232 ("arm64: syscall: Ensure saved x0 is kept in-sync with tracer updates")Fixes a5cd110cb836 ("arm64/ptrace: run seccomp after ptrace")Signed-off-by Will Deacon <[email protected]>--- arch/arm64/kernel/ptrace.c | 50 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+)v2: https://lore.kernel.org/r/[email protected]
arch/arm64/kernel/ptrace.c +36
-0@@ -560,6 +560,42 @@ static int gpr_get(struct task_struct *target,return membuf_write (&to, uregs,sizeof (*uregs)); } +static void update_syscall_orig_x0_after_ptrace (struct task_struct *target) +{ +struct pt_regs *regs =task_pt_regs (target); +struct kernel_siginfo *info = target->last_siginfo; + +/* We should only be called when target is in a ptrace stop */ +if (WARN_ON_ONCE (!info)) +return ; + +/* +* Skip the update for NO_SYSCALL (set either by the user or the +* tracer), as regs[0] holds the return value (see the comment in +* el0_svc_common()) and can be unwound using syscall_rollback(). +*/ +if (regs->syscallno ==NO_SYSCALL ) +return ; + +/* +* For compat tasks, orig_r0 is provided directly through GPR index +* 17. +*/ +if (is_compat_thread (task_thread_info (target))) +return ; + +/* +* Don't update orig_x0 for a syscall-exit-stop, as x0 now contains the +* return value of the system call. +*/ +if ((info->si_code & ~0x80 ) ==SIGTRAP && + target->ptrace_message ==PTRACE_EVENTMSG_SYSCALL_EXIT ) { +return ; + } + + regs->orig_x0 = regs->regs[0 ]; +} +static int gpr_set (struct task_struct *target,const struct user_regset *regset,unsigned int pos,unsigned int count,const void *kbuf,const void __user *ubuf)@@ -575,6 +611,14 @@ static int gpr_set(struct task_struct *target, const struct user_regset *regset,return -EINVAL ;
arch/arm64/kernel/ptrace.c static int gpr_set(struct task_struct *target, const struct user_regset *regset, · line 621
Quoted 10 lines
arch/arm64/kernel/ptrace.c +8
-0task_pt_regs (target)->user_regs = newregs; + +/* +* Keep orig_x0 authoritative so that seccomp (via +* syscall_get_arguments()), audit and the restart path all see the same +* first argument the syscall is dispatched with, even if it has been +* updated by a tracer. +*/ +update_syscall_orig_x0_after_ptrace (target);
Quoted 16 lines
arch/arm64/kernel/ptrace.c +6
-0return 0 ; }@@ -753,6 +797,12 @@ static int system_call_set(struct task_struct *target,return ret;task_pt_regs (target)->syscallno = syscallno; + +/* +* Re-sync orig_x0 in case the syscall number has been changed +* from NO_SYSCALL. +*/ +update_syscall_orig_x0_after_ptrace (target);return ret; }