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

[Will Deacon <will@kernel.org>]

Hi Jinjie,

Thank you for having a look at this. We're nearly there!

On Wed, Jul 15, 2026 at 07:39:43PM +0800, Jinjie Ruan wrote:
> On 7/14/2026 10:35 PM, Will Deacon wrote:
> > 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
> > @@ -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).
> > +		 */
> 
> 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.
>  */

Yes, that's better. Seccomp BPF filters will get passed whatever comes
back from syscall_get_arguments(), so mentioning SECCOMP_RET_TRACE is
confusing here.

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

Right, the 'SECCOMP_RET_TRACE' example should be in _this_ comment, not
the previous one.

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

I'll tweak that, as I don't think the tracing part matters (it doesn't
see orig_x0 afaict) and I'd like to be very clear that this is down
to the secure_computing() call. So it becomes:

  /*
   * Ensure tracer changes to x0 during seccomp ptrace exit processing
   * (SECCOMP_RET_TRACE) are visible to audit.
   */

Will