Re: [PATCH] arm64: syscall: Ensure saved x0 is kept in-sync with tracer updates
From: Jinjie Ruan
Date: Wed Jul 15 2026 - 22:58:01 EST
On 7/14/2026 10:35 PM, Will Deacon wrote:
> When seccomp support was originally added to arm64 in a1ae65b21941
> ("arm64: add seccomp support"), seccomp was erroneously called _before_
> the ptrace syscall-enter-stop and therefore the tracer could trivially
> manipulate the syscall register state after the seccomp check had
> passed. This was subsequently fixed in a5cd110cb836 ("arm64/ptrace: run
> seccomp after ptrace") by moving the seccomp check after the tracer has
> run. Unfortunately, a decade later, that fix has been reported to be
> incomplete.
>
> On arm64, both the first argument to a syscall and its eventual return
> value are allocated to register x0. In order to facilitate syscall
> restarting and querying of syscall arguments on the syscall exit path,
> the original value of x0 is stashed in 'struct pt_regs::orig_x0' early
> during the syscall entry path and is returned for the first argument by
> syscall_get_arguments(). Unlike 32-bit Arm, this stashed value is not
> directly exposed via ptrace() and so changes to register x0 made by the
> tracer on a syscall-enter-stop are not reflected in 'orig_x0'. This
> means that seccomp and audit can observe a stale value for the register
> compared to the argument that will be observed by the actual syscall.
>
> Re-sync 'orig_x0' from x0 on the syscall entry path following a
> potential ptrace stop (i.e. PTRACE_EVENTMSG_SYSCALL_ENTRY or
> SECCOMP_RET_TRACE). This behaviour is limited to native tasks (because
> compat tasks expose 'orig_r0' to ptrace) where the syscall is not being
> skipped (because x0 is updated to hold the return value of -ENOSYS in
> that case).
>
> Cc: Kees Cook <kees@xxxxxxxxxx>
> Cc: Jinjie Ruan <ruanjinjie@xxxxxxxxxx>
> Cc: Mark Rutland <mark.rutland@xxxxxxx>
> Reported-by: Yiqi Sun <sunyiqixm@xxxxxxxxx>
> Link: https://lore.kernel.org/all/20260529065444.1336608-1-sunyiqixm@xxxxxxxxx/
> Suggested-by: Catalin Marinas <catalin.marinas@xxxxxxx>
> Fixes: a5cd110cb836 ("arm64/ptrace: run seccomp after ptrace")
> Signed-off-by: Will Deacon <will@xxxxxxxxxx>
> ---
> arch/arm64/kernel/ptrace.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> index 4d08598e2891..57e8c6714d44 100644
> --- a/arch/arm64/kernel/ptrace.c
> +++ b/arch/arm64/kernel/ptrace.c
> @@ -2408,6 +2408,21 @@ static void report_syscall_exit(struct pt_regs *regs)
> }
> }
>
> +static void update_syscall_orig_x0_after_ptrace(struct pt_regs *regs)
> +{
> + /*
> + * 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. Skip this 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().
> + * For compat tasks, orig_r0 is provided directly through GPR index 17.
> + */
> + if (!is_compat_task() && regs->syscallno != NO_SYSCALL)
> + regs->orig_x0 = regs->regs[0];
> +}
> +
> int syscall_trace_enter(struct pt_regs *regs)
> {
> unsigned long flags = read_thread_flags();
> @@ -2417,12 +2432,21 @@ int syscall_trace_enter(struct pt_regs *regs)
> ret = report_syscall_entry(regs);
> if (ret || (flags & _TIF_SYSCALL_EMU))
> return NO_SYSCALL;
> +
> + /*
> + * Ensure ptrace changes to x0 are visible to seccomp
> + * ptrace exits (SECCOMP_RET_TRACE).
> + */
> + update_syscall_orig_x0_after_ptrace(regs);
> }
>
> /* Do the secure computing after ptrace; failures should be fast. */
> if (secure_computing() == -1)
> return NO_SYSCALL;
>
> + /* Ensure seccomp updates to x0 are visible to audit. */
> + update_syscall_orig_x0_after_ptrace(regs);
Hi, will
I think unconditionally updating orig_x0 here is unnecessary, we could
Expand seccomp check in place as below the same as generic entry.
In this way, in most cases where seccomp is not used, the overhead of
updating orig_x0 is eliminated. Moreover, we only need to define an
architecture-specific version of the seccomp function, thus avoiding the
pain of switching from arm64 to the generic entry.
So this patch can be like below.
int syscall_trace_enter(struct pt_regs *regs)
{
unsigned long flags = read_thread_flags();
@@ -2420,12 +2435,24 @@ int syscall_trace_enter(struct pt_regs *regs)
/* ptrace might have changed work flags */
flags = read_thread_flags();
+ /*
+ * Ensure ptrace changes to x0 during a regular
syscall-enter-stop
+ * (PTRACE_SYSCALL) are visible to subsequent seccomp trace
+ * and audit checking.
+ */
+ update_syscall_orig_x0_after_ptrace(regs);
}
/* Do the secure computing after ptrace; failures should be fast. */
if (unlikely(flags & _TIF_SECCOMP)) {
if (!__seccomp_permit_syscall())
return NO_SYSCALL;
+
+ /*
+ * Ensure tracer changes to x0 during seccomp ptrace
exit processing
+ * (SECCOMP_RET_TRACE) are visible to audit.
+ */
+ update_syscall_orig_x0_after_ptrace(regs);
}
Author: Jinjie Ruan <ruanjinjie@xxxxxxxxxx>
Date: Tue Oct 29 19:08:03 2024 +0800
arm64: ptrace: Expand seccomp check in place
Refactor syscall_trace_enter() by open-coding the seccomp check
to align with the generic entry framework. While the original call to
seccomp_permit_syscall() internally re-reads the thread flags and is
therefore safe against flag changes during ptrace stops, the new
open-coded version must explicitly re-read the flags after ptrace
handling to preserve that safety.
[Background]
The generic entry implementation expands the seccomp check in-place
instead of using the seccomp_permit_syscall() wrapper. It directly
tests SYSCALL_WORK_SECCOMP and calls the underlying
__seccomp_permit_syscall() function to handle syscall filtering.
[Changes]
1. After ptrace handling, re-read thread flags:
This ensures that any _TIF_SECCOMP set during the ptrace stop is
observed before the seccomp check.
2. Open-code seccomp check:
- Instead of calling the seccomp_permit_syscall() wrapper, explicitly
check the updated 'flags' parameter for _TIF_SECCOMP.
- Call __seccomp_permit_syscall() directly if the flag is set.
[Why this matters]
- Aligns the arm64 syscall path with the generic entry implementation,
simplifying future migration to the generic entry framework.
- No functional changes are intended; seccomp behavior remains
identical.
The explicit re-read ensures the open-coded version retains the same
safety as the original wrapper, preventing the race condition
described
in the generic entry fix.
- Performance: Non-ptrace fast path avoids atomic test_bit overhead via
cached flags.
Cc: Mark Rutland <mark.rutland@xxxxxxx>
Cc: Will Deacon <will@xxxxxxxxxx>
Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
Link:
https://lore.kernel.org/all/20260713025712.416366-1-ruanjinjie@xxxxxxxxxx/
Reviewed-by: Ada Couprie Diaz <ada.coupriediaz@xxxxxxx>
Reviewed-by: Linus Walleij <linusw@xxxxxxxxxx>
Reviewed-by: Yeoreum Yun <yeoreum.yun@xxxxxxx>
Reviewed-by: Kevin Brodsky <kevin.brodsky@xxxxxxx>
Signed-off-by: Jinjie Ruan <ruanjinjie@xxxxxxxxxx>
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 5709e9d3c321..941752656ea6 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -2417,11 +2417,16 @@ int syscall_trace_enter(struct pt_regs *regs)
ret = report_syscall_entry(regs);
if (ret || (flags & _TIF_SYSCALL_EMU))
return NO_SYSCALL;
+
+ /* ptrace might have changed the flags */
+ flags = read_thread_flags();
}
/* Do the secure computing after ptrace; failures should be fast. */
- if (!seccomp_permit_syscall())
- return NO_SYSCALL;
+ if (unlikely(flags & _TIF_SECCOMP)) {
+ if (!__seccomp_permit_syscall())
+ return NO_SYSCALL;
+ }
if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
trace_sys_enter(regs, regs->syscallno);
Best regards,
Jinjie
> +
> if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
> trace_sys_enter(regs, regs->syscallno);
>