Re: [PATCH] arm64: syscall: Ensure saved x0 is kept in-sync with tracer updates
From: Will Deacon
Date: Wed Jul 15 2026 - 09:22:35 EST
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