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

[Jinjie Ruan]

On 7/21/2026 10:05 PM, Will Deacon wrote:
> Hi Jinjie,
> 
> On Mon, Jul 20, 2026 at 11:48:40AM +0800, Jinjie Ruan wrote:
>> On 7/18/2026 1:54 AM, Will Deacon wrote:
>>> On Thu, Jul 16, 2026 at 05:48:01PM +0100, Will Deacon wrote:
>>>> On Thu, 16 Jul 2026 13:06:39 +0100, 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.
>>>>>
>>>>> [...]
>>>>
>>>> Applied to arm64 (for-next/fixes), thanks!
>>>>
>>>> [1/1] arm64: syscall: Ensure saved x0 is kept in-sync with tracer updates
>>>>       https://git.kernel.org/arm64/c/e057b9477232
>>>
>>> Bah, I've had to revert this. I think Sashiko makes a good point here
>>> that the seccomp interaction is still broken when the filter is
>>> re-evaluated after the tracer stop, because that all happens inside
>>> secure_computing() so we don't get a chance to update 'orig_x0':
>>>
>>> https://sashiko.dev/#/patchset/20260716120640.6590-1-will@xxxxxxxxxx
>>>
>>
>> Yes, I also think the point raised by Sashiko is meaningful.
>>
>> After reviewing the relevant code, I believe that the issue Sashiko
>> pointed out regarding the compat task also exists.
>>
>> My confusion is that on arm64 compat mode, both audit and trace use
>> orig_x0, but the first parameter used for executing system calls is
>> regs->reg[0]. If x0 is modified at the system call entry point in ptrace
>> but orig_x0 is not modified, or if orig_x0 is modified but x0 is not,
>> then the first parameter for audit and the actual system call being
>> executed will be out of sync. Could there be any issues here? Is it the
>> responsibility of ptrace to ensure that orig_x0 and x0 are synchronized
>> at the system call entry point on arm64 compat mode?
> 
> Even with orig_r0, I think compat suffers from some similar issues here.
> However, I really don't want to diverge from the arch/arm/ behaviour, so
> the 32-bit code should be fixed first if we want to change anything there.

Hi Will,

I will try to clarify whether 32-bit code has similar issues.

> Having said that, I notice we're already different in how we invoke
> audit_syscall_entry() :(

Yes, arm32 use regs->ARM_r0 instead of orig_r0 for audit, which is
different from using orig_x0 for arm64 compat mode.

871 >-------audit_syscall_entry(scno, regs->ARM_r0, regs->ARM_r1,
regs->ARM_r2,
872 >------->------->-------    regs->ARM_r3);

> 
> I also don't know what the correct behaviour should be when both r0 and
> orig_r0 are exposed to the tracer. I have a horrible feeling it probably

Yes, more discussion and historical background research are needed here.
I will try to look into what the correct behavior for ptrace should be
in this context.

> all worked until the API in syscall.h came along, at which point orig_r0
> was suddenly used for more than restarting.
> 
>> Whether it is ptrace or the kernel, ensuring that orig_x0 and x0 are
>> synchronized at the entry point of a system call, I believe it is
>> reasonable to use orig_x0 as the first parameter of the system call
>> execution and pre-set an error code in advance, as this way orig_x0
>> always retains the latest value of x0.
>>
>> diff --git a/arch/arm64/include/asm/syscall_wrapper.h
>> b/arch/arm64/include/asm/syscall_wrapper.h
>> index abb57bc54305..6b13d7c8ad95 100644
>> --- a/arch/arm64/include/asm/syscall_wrapper.h
>> +++ b/arch/arm64/include/asm/syscall_wrapper.h
>> @@ -12,7 +12,7 @@
>>
>>  #define SC_ARM64_REGS_TO_ARGS(x, ...)                          \
>>         __MAP(x,__SC_ARGS                                       \
>> -             ,,regs->regs[0],,regs->regs[1],,regs->regs[2]     \
>> +             ,,regs->orig_x0,,regs->regs[1],,regs->regs[2]     \
>>               ,,regs->regs[3],,regs->regs[4],,regs->regs[5])
> 
> I quite like this idea, but I think we'll have to limit it to native
> (64-bit) tasks. The 32-bit code in arch/arm/ looks like it passes
> regs[0] for the first argument and so tracers could easily be relying on

Yes, compat mode uses regs[0], which is consistent with 32-bit code
behavior, so it is reasonable to only modify 64-bit code.

> that.
> 
> I'll do that as a follow-up patch.

Thank you for doing this.

Best regards,
Jinjie

> 
>>  #ifdef CONFIG_COMPAT
>> diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
>> index 2535cae9413d..a978bab59924 100644
>> --- a/arch/arm64/kernel/syscall.c
>> +++ b/arch/arm64/kernel/syscall.c
>> @@ -62,6 +62,7 @@ static __always_inline void el0_svc_common(struct
>> pt_regs *regs, int scno, int s
>>         unsigned long work;
>>
>>         regs->orig_x0 = regs->regs[0];
>> +       syscall_set_return_value(current, regs, -ENOSYS, 0);
> 
> I don't think it's safe to do this unconditionally as tracers could be
> relying on reading X0 with ptrace to retrieve the first syscall argument.
> 
> Will