arm64: Fix handling of tracer updates to x0 on syscall entry
Subsystems (2)
Will Deacon
maintainerv4 cover arm64: Fix handling of tracer updates to x0 on syscall entry
Hi all,
This is version 4 of the patch previously posted here:
v1: https://lore.kernel.org/r/[email protected]
v2: https://lore.kernel.org/r/[email protected]
v3: https://lore.kernel.org/r/[email protected]
Changes since v3 include:
- Add an additional patch to pass orig_x0 as the first syscall argument for native tasks (Jinjie)
- Move WARN_ON_ONCE() after checking regs->syscallno (Sashiko)
- Add Tested-by from Jinjie.
Cheers,
Will
--->8
Will Deacon (2):
arm64: ptrace: Keep 'orig_x0' in-sync with x0 on syscall entry
arm64: syscall: Pass 'orig_x0' as first argument to native system call
arch/arm64/include/asm/syscall_wrapper.h | 13 ++++-- arch/arm64/kernel/ptrace.c | 50 ++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 4 deletions(-)
Signature · 1 line
2.55.0.508.g3f0d502094-goog
Will Deacon
maintainerPATCH v4 1/2 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(+)
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; + +/* +* 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 ; + +/* We should only be called when target is in a ptrace stop */ +if (WARN_ON_ONCE (!info)) +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.508.g3f0d502094-goog
Will Deacon
maintainerPATCH v4 2/2 arm64: syscall: Pass 'orig_x0' as first argument to native system call
In reply to Will Deaconsyscall_get_arguments() returns 'regs->orig_x0' for the first system call argument so as to avoid aliasing with the syscall return value in 'regs->regs[0]' on the return path, however the actual syscall invocation passes 'regs->regs[0]' as the first parameter.
Although the two registers should be kept in sync during syscall entry for native tasks, pass 'regs->orig_x0' as the first syscall parameter for consistency with the syscall argument APIs. Compat tasks continue to use 'regs->regs[0]' for compatibility with the behaviour of the 32-bit kernel.
--- arch/arm64/include/asm/syscall_wrapper.h | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
arch/arm64/include/asm/syscall_wrapper.h +9 -4
@@ -10,13 +10,13 @@#include <asm/ptrace.h> -#define SC_ARM64_REGS_TO_ARGS (x, ...) \ +#ifdef CONFIG_COMPAT + +#define COMPAT_SC_ARM64_REGS_TO_ARGS (x, ...) \ __MAP(x,__SC_ARGS \ ,,regs->regs[0],,regs->regs[1],,regs->regs[2] \ ,,regs->regs[3],,regs->regs[4],,regs->regs[5]) -#ifdef CONFIG_COMPAT -#define COMPAT_SYSCALL_DEFINEx (x, name, ...) \ asmlinkage long __arm64_compat_sys##name(const struct pt_regs *regs); \ ALLOW_ERROR_INJECTION(__arm64_compat_sys##name, ERRNO); \
@@ -24,7 +24,7 @@ static inline long __do_compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)); \ asmlinkage long __arm64_compat_sys##name(const struct pt_regs *regs) \ { \ - return __se_compat_sys##name(SC_ARM64_REGS_TO_ARGS(x,__VA_ARGS__)); \ + return __se_compat_sys##name(COMPAT_SC_ARM64_REGS_TO_ARGS(x,__VA_ARGS__)); \ } \ static long __se_compat_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__)) \ { \
@@ -46,6 +46,11 @@#endif /* CONFIG_COMPAT */ +#define SC_ARM64_REGS_TO_ARGS (x, ...) \ + __MAP(x,__SC_ARGS \ + ,,regs->orig_x0,,regs->regs[1],,regs->regs[2] \ + ,,regs->regs[3],,regs->regs[4],,regs->regs[5]) +#define __SYSCALL_DEFINEx (x, name, ...) \ asmlinkage long __arm64_sys##name(const struct pt_regs *regs); \ ALLOW_ERROR_INJECTION(__arm64_sys##name, ERRNO); \
Signature · 1 line
2.55.0.508.g3f0d502094-goog
Quoted 9 lines · Will Deacon
This is version 4 of the patch previously posted here:
v1: https://lore.kernel.org/r/[email protected]
v2: https://lore.kernel.org/r/[email protected]
v3: https://lore.kernel.org/r/[email protected]Changes since v3 include:
[...]
I decided against applying these for v7.2 this late in the cycle, so I've queued them up for v7.3 for some more exposure.
Applied to arm64 (for-next/ptrace), thanks!
[1/2] arm64: ptrace: Keep 'orig_x0' in-sync with x0 on syscall entry
https://git.kernel.org/arm64/c/88b839ce497c [2/2] arm64: syscall: Pass 'orig_x0' as first argument to native system call https://git.kernel.org/arm64/c/5a87e8c7f370
Cheers,
Signature · 5 lines
Will https://fixes.arm64.dev https://next.arm64.dev https://will.arm64.dev