Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arm64: syscall: Ensure saved x0 is kept in-sync with tracer updates
@ 2026-07-14 14:35 Will Deacon
  2026-07-15 11:39 ` Jinjie Ruan
  2026-07-16  2:57 ` Jinjie Ruan
  0 siblings, 2 replies; 6+ messages in thread
From: Will Deacon @ 2026-07-14 14:35 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-kernel, Will Deacon, Kees Cook, Jinjie Ruan, Mark Rutland,
	Yiqi Sun, Catalin Marinas

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).
+		 */
+		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);
+
 	if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
 		trace_sys_enter(regs, regs->syscallno);
 
-- 
2.55.0.795.g602f6c329a-goog



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: syscall: Ensure saved x0 is kept in-sync with tracer updates
  2026-07-14 14:35 [PATCH] arm64: syscall: Ensure saved x0 is kept in-sync with tracer updates Will Deacon
@ 2026-07-15 11:39 ` Jinjie Ruan
  2026-07-15 13:16   ` Will Deacon
  2026-07-16  2:57 ` Jinjie Ruan
  1 sibling, 1 reply; 6+ messages in thread
From: Jinjie Ruan @ 2026-07-15 11:39 UTC (permalink / raw)
  To: Will Deacon, linux-arm-kernel
  Cc: linux-kernel, Kees Cook, Mark Rutland, Yiqi Sun, Catalin Marinas



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);
>  



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: syscall: Ensure saved x0 is kept in-sync with tracer updates
  2026-07-15 11:39 ` Jinjie Ruan
@ 2026-07-15 13:16   ` Will Deacon
  2026-07-16  2:09     ` Jinjie Ruan
  0 siblings, 1 reply; 6+ messages in thread
From: Will Deacon @ 2026-07-15 13:16 UTC (permalink / raw)
  To: Jinjie Ruan
  Cc: linux-arm-kernel, linux-kernel, Kees Cook, Mark Rutland, Yiqi Sun,
	Catalin Marinas

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: syscall: Ensure saved x0 is kept in-sync with tracer updates
  2026-07-15 13:16   ` Will Deacon
@ 2026-07-16  2:09     ` Jinjie Ruan
  0 siblings, 0 replies; 6+ messages in thread
From: Jinjie Ruan @ 2026-07-16  2:09 UTC (permalink / raw)
  To: Will Deacon
  Cc: linux-arm-kernel, linux-kernel, Kees Cook, Mark Rutland, Yiqi Sun,
	Catalin Marinas



On 7/15/2026 9:16 PM, Will Deacon wrote:
> 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:

Hi Will,

Looking forward to your update.

I do not think so, I think the trace part also matters, because the
tracepoint also reads orig_x0 through syscall_get_arguments().

 18 TRACE_EVENT_SYSCALL(sys_enter,
 19
 20 >-------TP_PROTO(struct pt_regs *regs, long id),
 21
 22 >-------TP_ARGS(regs, id),
 23
 24 >-------TP_STRUCT__entry(
 25 >------->-------__field(>-------long,>-->-------id>----->-------)
 26 >------->-------__array(>-------unsigned long,>-args,>--6>------)
 27 >-------),
 28
 29 >-------TP_fast_assign(
 30 >------->-------__entry->id>----= id;
 31 >------->-------syscall_get_arguments(current, regs, __entry->args);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

 32 >-------),
 33
 34 >-------TP_printk("NR %ld (%lx, %lx, %lx, %lx, %lx, %lx)",
 35 >------->-------  __entry->id,
 36 >------->-------  __entry->args[0], __entry->args[1], __entry->args[2],
 37 >------->-------  __entry->args[3], __entry->args[4], __entry->args[5]),
 38
 39 >-------syscall_regfunc, syscall_unregfunc
 40 );

Based on a fix patch by Kees six years ago, I understand that system
call parameters are similar to system call numbers. If ptrace or seccomp
modifies the system call parameters, then at that time, the tracing and
auditing mechanisms also need to be able to see this change.

I understand that the semantics of seccomp and trace/audit are intended
to reflect the latest relevant data of system calls that are "actually
executed" (invoke_syscall()).


Link: https://lkml.org/lkml/2020/9/11/1282

Best regards,
Jinjie

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: syscall: Ensure saved x0 is kept in-sync with tracer updates
  2026-07-14 14:35 [PATCH] arm64: syscall: Ensure saved x0 is kept in-sync with tracer updates Will Deacon
  2026-07-15 11:39 ` Jinjie Ruan
@ 2026-07-16  2:57 ` Jinjie Ruan
  2026-07-16  3:05   ` Kees Cook
  1 sibling, 1 reply; 6+ messages in thread
From: Jinjie Ruan @ 2026-07-16  2:57 UTC (permalink / raw)
  To: Will Deacon, linux-arm-kernel
  Cc: linux-kernel, Kees Cook, Mark Rutland, Yiqi Sun, Catalin Marinas



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).
> +		 */
> +		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@huawei.com>
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@arm.com>
    Cc: Will Deacon <will@kernel.org>
    Cc: Catalin Marinas <catalin.marinas@arm.com>
    Link:
https://lore.kernel.org/all/20260713025712.416366-1-ruanjinjie@huawei.com/
    Reviewed-by: Ada Couprie Diaz <ada.coupriediaz@arm.com>
    Reviewed-by: Linus Walleij <linusw@kernel.org>
    Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
    Reviewed-by: Kevin Brodsky <kevin.brodsky@arm.com>
    Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>

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);
>  



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: syscall: Ensure saved x0 is kept in-sync with tracer updates
  2026-07-16  2:57 ` Jinjie Ruan
@ 2026-07-16  3:05   ` Kees Cook
  0 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2026-07-16  3:05 UTC (permalink / raw)
  To: Jinjie Ruan
  Cc: Will Deacon, linux-arm-kernel, linux-kernel, Mark Rutland,
	Yiqi Sun, Catalin Marinas

On Thu, Jul 16, 2026 at 10:57:34AM +0800, Jinjie Ruan wrote:
> 
> 
> 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).
> > +		 */
> > +		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@huawei.com>
> 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@arm.com>
>     Cc: Will Deacon <will@kernel.org>
>     Cc: Catalin Marinas <catalin.marinas@arm.com>
>     Link:
> https://lore.kernel.org/all/20260713025712.416366-1-ruanjinjie@huawei.com/
>     Reviewed-by: Ada Couprie Diaz <ada.coupriediaz@arm.com>
>     Reviewed-by: Linus Walleij <linusw@kernel.org>
>     Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
>     Reviewed-by: Kevin Brodsky <kevin.brodsky@arm.com>
>     Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> 
> 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);

Do we have a corresponding seccomp_bpf.c selftest we can add for this? I
would really like to have a regression test that would catch this
issue...

-Kees

-- 
Kees Cook


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-07-16  3:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-14 14:35 [PATCH] arm64: syscall: Ensure saved x0 is kept in-sync with tracer updates Will Deacon
2026-07-15 11:39 ` Jinjie Ruan
2026-07-15 13:16   ` Will Deacon
2026-07-16  2:09     ` Jinjie Ruan
2026-07-16  2:57 ` Jinjie Ruan
2026-07-16  3:05   ` Kees Cook

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox