Re: [PATCH] arm64: syscall: Ensure saved x0 is kept in-sync with tracer updates

[Jinjie Ruan <ruanjinjie@huawei.com>]

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@kernel.org>
> Cc: Jinjie Ruan <ruanjinjie@huawei.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Reported-by: Yiqi Sun <sunyiqixm@gmail.com>
> Link: https://lore.kernel.org/all/20260529065444.1336608-1-sunyiqixm@gmail.com/
> Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
> Fixes: a5cd110cb836 ("arm64/ptrace: run seccomp after ptrace")
> Signed-off-by: Will Deacon <will@kernel.org>
> ---
>  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).
> +		 */

Hi, will,

After delving into the seccomp code, I believe the comments are not
quite accurate, I think SECCOMP_RET_TRACE not return to here.

maybe,

/*
 * Ensure ptrace changes to x0 during a regular syscall-enter-stop
 * (PTRACE_SYSCALL) are visible to subsequent seccomp and audit
 * checking.
 */

> +		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. */

This comment is also not quite accurate, it implies that Seccomp itself
(such as SECCOMP_RET_ERRNO) modifies x0, but in this scenario, the audit
is not executed, because __seccomp_filter() skip the syscall.

1279 >-------switch (action) {
1280 >-------case SECCOMP_RET_ERRNO:
1281 >------->-------/* Set low-order bits as an errno, capped at
MAX_ERRNO. */
1282 >------->-------if (data > MAX_ERRNO)
1283 >------->------->-------data = MAX_ERRNO;
1284 >------->-------syscall_set_return_value(current, current_pt_regs(),
1285 >------->------->------->------->------- -data, 0);
1286 >------->-------goto skip;
                   ^^^^^^^^^^^^^<- we skip the syscall if
SECCOMP_RET_ERRNO changes x0


Here what we actually need to synchronize is the tracer's modification
of x0 in the SECCOMP_RET_TRACE path, the SECCOMP_RET_TRACE logic
notifies the tracer, the tracer modifies x0 and modifies the system call
number to a legal value and so we can continue the latter audit.

1295    case SECCOMP_RET_TRACE:
1296        /* We've been put in this state by the ptracer already. */
1297        if (recheck_after_trace)
1298            return true;
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^  <- I think we return here.

1299
1300        /* ENOSYS these calls if there is no tracer attached. */
1301        if (!ptrace_event_enabled(current, PTRACE_EVENT_SECCOMP)) {
1302            syscall_set_return_value(current,
1303                         current_pt_regs(),
1304                         -ENOSYS, 0);
1305            goto skip;
1306        }
1307
1308        /* Allow the BPF to provide the event message */
1309        ptrace_event(PTRACE_EVENT_SECCOMP, data);
1310        /*
1311         * The delivery of a fatal signal during event
1312         * notification may silently skip tracer notification,
1313         * which could leave us with a potentially unmodified
1314         * syscall that the tracer would have liked to have
1315         * changed. Since the process is about to die, we just
1316         * force the syscall to be skipped and let the signal
1317         * kill the process and correctly handle any tracer exit
1318         * notifications.
1319         */
1320        if (fatal_signal_pending(current))
1321            goto skip;
1322        /* Check if the tracer forced the syscall to be skipped. */
1323        this_syscall = syscall_get_nr(current, current_pt_regs());
1324        if (this_syscall < 0)
1325            goto skip;
1326
1327        /*
1328         * Recheck the syscall, since it may have changed. This
1329         * intentionally uses a NULL struct seccomp_data to force
1330         * a reload of all registers. This does not goto skip since
1331         * a skip would have already been reported.
1332         */
1333        return __seccomp_filter(this_syscall, true);


maybe,

/*
 * Ensure tracer changes to x0 during SECCOMP_RET_TRACE processing
 * are visible to later trace and audit.
*/


Otherwsie, LGTM

Best reagards,
Jinjie

> +	update_syscall_orig_x0_after_ptrace(regs);
> +
>  	if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
>  		trace_sys_enter(regs, regs->syscallno);
>