Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] fix: arm64: syscall: use live x0 for syscall_get_arguments() arg0
@ 2026-05-29  6:54 Yiqi Sun
  2026-06-01 12:43 ` Will Deacon
  2026-06-25 10:45 ` [PATCH v2] arm64: ptrace: use live x0 for seccomp and audit after ptrace Yiqi Sun
  0 siblings, 2 replies; 10+ messages in thread
From: Yiqi Sun @ 2026-05-29  6:54 UTC (permalink / raw)
  To: catalin.marinas, linux-arm-kernel
  Cc: linux-kernel, rmk+kernel, ruanjinjie, will, Yiqi Sun

On arm64, seccomp obtains syscall arguments via syscall_get_arguments(),
where arg0 is currently read from regs->orig_x0. However, the syscall
wrapper consumes live arguments from regs->regs[0..5].

A ptracer can modify x0 on syscall-enter stop before seccomp runs,
but cannot update orig_x0 through that interface. This can
leave seccomp checking stale arg0 while the syscall executes with updated
live x0, allowing seccomp bypass when filters depend on arg0.

Make syscall_get_arguments() read arg0 from regs->regs[0], matching the
actual dispatch arguments and removing this desynchronization.

Fixes: f27bb139c387 ("arm64: Miscellaneous library functions")
Signed-off-by: Yiqi Sun <sunyiqixm@gmail.com>
---
 arch/arm64/include/asm/syscall.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/syscall.h b/arch/arm64/include/asm/syscall.h
index 5e4c7fc44f73..4bdb4d3ce2b4 100644
--- a/arch/arm64/include/asm/syscall.h
+++ b/arch/arm64/include/asm/syscall.h
@@ -81,7 +81,7 @@ static inline void syscall_get_arguments(struct task_struct *task,
 					 struct pt_regs *regs,
 					 unsigned long *args)
 {
-	args[0] = regs->orig_x0;
+	args[0] = regs->regs[0];
 	args[1] = regs->regs[1];
 	args[2] = regs->regs[2];
 	args[3] = regs->regs[3];
-- 
2.34.1



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

* Re: [PATCH] fix: arm64: syscall: use live x0 for syscall_get_arguments() arg0
  2026-05-29  6:54 [PATCH] fix: arm64: syscall: use live x0 for syscall_get_arguments() arg0 Yiqi Sun
@ 2026-06-01 12:43 ` Will Deacon
  2026-06-03  9:07   ` Yiqi Sun
  2026-06-25 10:45 ` [PATCH v2] arm64: ptrace: use live x0 for seccomp and audit after ptrace Yiqi Sun
  1 sibling, 1 reply; 10+ messages in thread
From: Will Deacon @ 2026-06-01 12:43 UTC (permalink / raw)
  To: Yiqi Sun
  Cc: catalin.marinas, linux-arm-kernel, linux-kernel, rmk+kernel,
	ruanjinjie, mark.rutland, kees, keno, luis.machado

On Fri, May 29, 2026 at 02:54:44PM +0800, Yiqi Sun wrote:
> On arm64, seccomp obtains syscall arguments via syscall_get_arguments(),
> where arg0 is currently read from regs->orig_x0. However, the syscall
> wrapper consumes live arguments from regs->regs[0..5].
> 
> A ptracer can modify x0 on syscall-enter stop before seccomp runs,
> but cannot update orig_x0 through that interface. This can
> leave seccomp checking stale arg0 while the syscall executes with updated
> live x0, allowing seccomp bypass when filters depend on arg0.
> 
> Make syscall_get_arguments() read arg0 from regs->regs[0], matching the
> actual dispatch arguments and removing this desynchronization.
> 
> Fixes: f27bb139c387 ("arm64: Miscellaneous library functions")
> Signed-off-by: Yiqi Sun <sunyiqixm@gmail.com>
> ---
>  arch/arm64/include/asm/syscall.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/include/asm/syscall.h b/arch/arm64/include/asm/syscall.h
> index 5e4c7fc44f73..4bdb4d3ce2b4 100644
> --- a/arch/arm64/include/asm/syscall.h
> +++ b/arch/arm64/include/asm/syscall.h
> @@ -81,7 +81,7 @@ static inline void syscall_get_arguments(struct task_struct *task,
>  					 struct pt_regs *regs,
>  					 unsigned long *args)
>  {
> -	args[0] = regs->orig_x0;
> +	args[0] = regs->regs[0];
>  	args[1] = regs->regs[1];
>  	args[2] = regs->regs[2];
>  	args[3] = regs->regs[3];
> -- 
> 2.34.1

Hrm, this looks like a long-standing issue and I'm pretty nervous about
changing it :/

How did you spot it?

A quick look at the code suggests we have a similar issue with
audit_syscall_entry(), so if we take your patch here then it will silently
introduce a behavioural change to this guy:

https://lore.kernel.org/all/20260320102620.1336796-5-ruanjinjie@huawei.com/

I also notice that the compat ptrace interface allows 'orig_x0' to be
set -- could that cause issues with things like syscall_rollback()?

Will


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

* Re: Re: [PATCH] fix: arm64: syscall: use live x0 for syscall_get_arguments() arg0
  2026-06-01 12:43 ` Will Deacon
@ 2026-06-03  9:07   ` Yiqi Sun
  2026-06-19 16:05     ` Will Deacon
  0 siblings, 1 reply; 10+ messages in thread
From: Yiqi Sun @ 2026-06-03  9:07 UTC (permalink / raw)
  To: will
  Cc: catalin.marinas, kees, keno, linux-arm-kernel, linux-kernel,
	luis.machado, mark.rutland, rmk+kernel, ruanjinjie, sunyiqixm

On Mon, 1 Jun 2026 13:43:42 +0100, Well wrote:
> On Fri, May 29, 2026 at 02:54:44PM +0800, Yiqi Sun wrote:
> > On arm64, seccomp obtains syscall arguments via syscall_get_arguments(),
> > where arg0 is currently read from regs->orig_x0. However, the syscall
> > wrapper consumes live arguments from regs->regs[0..5].
> > 
> > A ptracer can modify x0 on syscall-enter stop before seccomp runs,
> > but cannot update orig_x0 through that interface. This can
> > leave seccomp checking stale arg0 while the syscall executes with updated
> > live x0, allowing seccomp bypass when filters depend on arg0.
> > 
> > Make syscall_get_arguments() read arg0 from regs->regs[0], matching the
> > actual dispatch arguments and removing this desynchronization.
> > 
> > Fixes: f27bb139c387 ("arm64: Miscellaneous library functions")
> > Signed-off-by: Yiqi Sun <sunyiqixm@gmail.com>
> > ---
> >  arch/arm64/include/asm/syscall.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/include/asm/syscall.h b/arch/arm64/include/asm/syscall.h
> > index 5e4c7fc44f73..4bdb4d3ce2b4 100644
> > --- a/arch/arm64/include/asm/syscall.h
> > +++ b/arch/arm64/include/asm/syscall.h
> > @@ -81,7 +81,7 @@ static inline void syscall_get_arguments(struct task_struct *task,
> >  					 struct pt_regs *regs,
> >  					 unsigned long *args)
> >  {
> > -	args[0] = regs->orig_x0;
> > +	args[0] = regs->regs[0];
> >  	args[1] = regs->regs[1];
> >  	args[2] = regs->regs[2];
> >  	args[3] = regs->regs[3];
> > -- 
> > 2.34.1
> 
> Hrm, this looks like a long-standing issue and I'm pretty nervous about
> changing it :/
> 
> How did you spot it?

I share your concern here, and I’m trying to be very careful with any
behavior change.

I spotted this while comparing seccomp+ptrace ordering across arches.
I had previously looked at x86/x86_64 (including the seccomp/ptrace
ordering fix from more than 10 years ago), and then checked whether
other arches like arm64 had the same issue.

> A quick look at the code suggests we have a similar issue with
> audit_syscall_entry(), so if we take your patch here then it will silently
> introduce a behavioural change to this guy:
> 
> https://lore.kernel.org/all/20260320102620.1336796-5-ruanjinjie@huawei.com/
> 
> I also notice that the compat ptrace interface allows 'orig_x0' to be
> set -- could that cause issues with things like syscall_rollback()?
> Will

You are right that on arm64, audit_syscall_entry() still takes arg0
from orig_x0, so taking only this seccomp fix would diverge seccomp
and audit semantics.

I also checked syscall_rollback(): on arm64 it restores regs[0] from
orig_x0, and orig_x0 is captured at syscall entry before the ptrace
syscall-stop hook. So rollback normally returns to the syscall-entry
state (i.e. pre-ptrace argument value), which does not look like a new
security issue by itself. compat ptrace can explicitly write orig_x0,
but that is existing tracer-controlled behavior and does not, by itself,
cross a new security boundary introduced by this patch.

If you agree, I can send a follow-up later so seccomp/audit stay consistent.

Yiqi Sun


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

* Re: Re: [PATCH] fix: arm64: syscall: use live x0 for syscall_get_arguments() arg0
  2026-06-03  9:07   ` Yiqi Sun
@ 2026-06-19 16:05     ` Will Deacon
  0 siblings, 0 replies; 10+ messages in thread
From: Will Deacon @ 2026-06-19 16:05 UTC (permalink / raw)
  To: Yiqi Sun
  Cc: catalin.marinas, kees, keno, linux-arm-kernel, linux-kernel,
	luis.machado, mark.rutland, rmk+kernel, ruanjinjie

On Wed, Jun 03, 2026 at 05:07:30PM +0800, Yiqi Sun wrote:
> On Mon, 1 Jun 2026 13:43:42 +0100, Well wrote:
> > On Fri, May 29, 2026 at 02:54:44PM +0800, Yiqi Sun wrote:
> > > On arm64, seccomp obtains syscall arguments via syscall_get_arguments(),
> > > where arg0 is currently read from regs->orig_x0. However, the syscall
> > > wrapper consumes live arguments from regs->regs[0..5].
> > > 
> > > A ptracer can modify x0 on syscall-enter stop before seccomp runs,
> > > but cannot update orig_x0 through that interface. This can
> > > leave seccomp checking stale arg0 while the syscall executes with updated
> > > live x0, allowing seccomp bypass when filters depend on arg0.
> > > 
> > > Make syscall_get_arguments() read arg0 from regs->regs[0], matching the
> > > actual dispatch arguments and removing this desynchronization.
> > > 
> > > Fixes: f27bb139c387 ("arm64: Miscellaneous library functions")
> > > Signed-off-by: Yiqi Sun <sunyiqixm@gmail.com>
> > > ---
> > >  arch/arm64/include/asm/syscall.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/arm64/include/asm/syscall.h b/arch/arm64/include/asm/syscall.h
> > > index 5e4c7fc44f73..4bdb4d3ce2b4 100644
> > > --- a/arch/arm64/include/asm/syscall.h
> > > +++ b/arch/arm64/include/asm/syscall.h
> > > @@ -81,7 +81,7 @@ static inline void syscall_get_arguments(struct task_struct *task,
> > >  					 struct pt_regs *regs,
> > >  					 unsigned long *args)
> > >  {
> > > -	args[0] = regs->orig_x0;
> > > +	args[0] = regs->regs[0];
> > >  	args[1] = regs->regs[1];
> > >  	args[2] = regs->regs[2];
> > >  	args[3] = regs->regs[3];
> > > -- 
> > > 2.34.1
> > 
> > Hrm, this looks like a long-standing issue and I'm pretty nervous about
> > changing it :/
> > 
> > How did you spot it?
> 
> I share your concern here, and I’m trying to be very careful with any
> behavior change.
> 
> I spotted this while comparing seccomp+ptrace ordering across arches.
> I had previously looked at x86/x86_64 (including the seccomp/ptrace
> ordering fix from more than 10 years ago), and then checked whether
> other arches like arm64 had the same issue.
> 
> > A quick look at the code suggests we have a similar issue with
> > audit_syscall_entry(), so if we take your patch here then it will silently
> > introduce a behavioural change to this guy:
> > 
> > https://lore.kernel.org/all/20260320102620.1336796-5-ruanjinjie@huawei.com/
> > 
> > I also notice that the compat ptrace interface allows 'orig_x0' to be
> > set -- could that cause issues with things like syscall_rollback()?
> > Will
> 
> You are right that on arm64, audit_syscall_entry() still takes arg0
> from orig_x0, so taking only this seccomp fix would diverge seccomp
> and audit semantics.
> 
> I also checked syscall_rollback(): on arm64 it restores regs[0] from
> orig_x0, and orig_x0 is captured at syscall entry before the ptrace
> syscall-stop hook. So rollback normally returns to the syscall-entry
> state (i.e. pre-ptrace argument value), which does not look like a new
> security issue by itself. compat ptrace can explicitly write orig_x0,
> but that is existing tracer-controlled behavior and does not, by itself,
> cross a new security boundary introduced by this patch.
> 
> If you agree, I can send a follow-up later so seccomp/audit stay consistent.

Yes, please send a v2 that fixes audit at the same time.

Will


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

* [PATCH v2] arm64: ptrace: use live x0 for seccomp and audit after ptrace
  2026-05-29  6:54 [PATCH] fix: arm64: syscall: use live x0 for syscall_get_arguments() arg0 Yiqi Sun
  2026-06-01 12:43 ` Will Deacon
@ 2026-06-25 10:45 ` Yiqi Sun
  2026-06-25 11:11   ` Yiqi Sun
                     ` (2 more replies)
  1 sibling, 3 replies; 10+ messages in thread
From: Yiqi Sun @ 2026-06-25 10:45 UTC (permalink / raw)
  To: catalin.marinas, linux-arm-kernel
  Cc: linux-kernel, rmk+kernel, ruanjinjie, will, Yiqi Sun

On arm64, seccomp obtains syscall arguments via
syscall_get_arguments(), where arg0 is currently read from
regs->orig_x0. audit_syscall_entry() in syscall_trace_enter() also
takes arg0 from regs->orig_x0. However, the syscall wrapper consumes
live arguments from regs->regs[0..5].

A ptracer can modify x0 on syscall-enter stop before seccomp and audit
run, but cannot update orig_x0 through the native syscall-stop
interface. This can leave seccomp and audit checking stale arg0 while
the syscall executes with updated live x0.

Make both paths read arg0 from regs->regs[0], matching the actual
dispatch arguments and keeping seccomp and audit aligned after ptrace
updates.

Fixes: f27bb139c387 ("arm64: Miscellaneous library functions")
Signed-off-by: Yiqi Sun <sunyiqixm@gmail.com>
---
Changes in v2:
- Also switch the arm64 audit entry path to use live x0
- Clarify the orig_x0 synchronization comment in syscall_set_arguments()
---
 arch/arm64/include/asm/syscall.h | 7 +++----
 arch/arm64/kernel/ptrace.c       | 2 +-
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/syscall.h b/arch/arm64/include/asm/syscall.h
index 5e4c7fc44f73..0a44db425522 100644
--- a/arch/arm64/include/asm/syscall.h
+++ b/arch/arm64/include/asm/syscall.h
@@ -81,7 +81,7 @@ static inline void syscall_get_arguments(struct task_struct *task,
 					 struct pt_regs *regs,
 					 unsigned long *args)
 {
-	args[0] = regs->orig_x0;
+	args[0] = regs->regs[0];
 	args[1] = regs->regs[1];
 	args[2] = regs->regs[2];
 	args[3] = regs->regs[3];
@@ -101,9 +101,8 @@ static inline void syscall_set_arguments(struct task_struct *task,
 	regs->regs[5] = args[5];
 
 	/*
-	 * Also copy the first argument into orig_x0
-	 * so that syscall_get_arguments() would return it
-	 * instead of the previous value.
+	 * Keep orig_x0 in sync so syscall_rollback() and compat
+	 * register views see the updated first argument.
 	 */
 	regs->orig_x0 = regs->regs[0];
 }
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 4d08598e2891..35dd86f9e87a 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -2426,7 +2426,7 @@ int syscall_trace_enter(struct pt_regs *regs)
 	if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
 		trace_sys_enter(regs, regs->syscallno);
 
-	audit_syscall_entry(regs->syscallno, regs->orig_x0, regs->regs[1],
+	audit_syscall_entry(regs->syscallno, regs->regs[0], regs->regs[1],
 			    regs->regs[2], regs->regs[3]);
 
 	return regs->syscallno;
-- 
2.34.1



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

* [PATCH v2] arm64: ptrace: use live x0 for seccomp and audit after ptrace
  2026-06-25 10:45 ` [PATCH v2] arm64: ptrace: use live x0 for seccomp and audit after ptrace Yiqi Sun
@ 2026-06-25 11:11   ` Yiqi Sun
  2026-06-25 11:30   ` Yiqi Sun
  2026-06-29 13:09   ` Will Deacon
  2 siblings, 0 replies; 10+ messages in thread
From: Yiqi Sun @ 2026-06-25 11:11 UTC (permalink / raw)
  To: sunyiqixm
  Cc: catalin.marinas, linux-arm-kernel, linux-kernel, rmk+kernel,
	ruanjinjie, will

On arm64, seccomp obtains syscall arguments via
syscall_get_arguments(), where arg0 is currently read from
regs->orig_x0. audit_syscall_entry() in syscall_trace_enter() also
takes arg0 from regs->orig_x0. However, the syscall wrapper consumes
live arguments from regs->regs[0..5].

A ptracer can modify x0 on syscall-enter stop before seccomp and audit
run, but cannot update orig_x0 through the native syscall-stop
interface. This can leave seccomp and audit checking stale arg0 while
the syscall executes with updated live x0.

Make both paths read arg0 from regs->regs[0], matching the actual
dispatch arguments and keeping seccomp and audit aligned after ptrace
updates.

Fixes: f27bb139c387 ("arm64: Miscellaneous library functions")
Signed-off-by: Yiqi Sun <sunyiqixm@gmail.com>
---
Changes in v2:
- Also switch the arm64 audit entry path to use live x0
- Clarify the orig_x0 synchronization comment in syscall_set_arguments()
---
 arch/arm64/include/asm/syscall.h | 7 +++----
 arch/arm64/kernel/ptrace.c       | 2 +-
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/syscall.h b/arch/arm64/include/asm/syscall.h
index 5e4c7fc44f73..0a44db425522 100644
--- a/arch/arm64/include/asm/syscall.h
+++ b/arch/arm64/include/asm/syscall.h
@@ -81,7 +81,7 @@ static inline void syscall_get_arguments(struct task_struct *task,
 					 struct pt_regs *regs,
 					 unsigned long *args)
 {
-	args[0] = regs->orig_x0;
+	args[0] = regs->regs[0];
 	args[1] = regs->regs[1];
 	args[2] = regs->regs[2];
 	args[3] = regs->regs[3];
@@ -101,9 +101,8 @@ static inline void syscall_set_arguments(struct task_struct *task,
 	regs->regs[5] = args[5];
 
 	/*
-	 * Also copy the first argument into orig_x0
-	 * so that syscall_get_arguments() would return it
-	 * instead of the previous value.
+	 * Keep orig_x0 in sync so syscall_rollback() and compat
+	 * register views see the updated first argument.
 	 */
 	regs->orig_x0 = regs->regs[0];
 }
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 4d08598e2891..35dd86f9e87a 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -2426,7 +2426,7 @@ int syscall_trace_enter(struct pt_regs *regs)
 	if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
 		trace_sys_enter(regs, regs->syscallno);
 
-	audit_syscall_entry(regs->syscallno, regs->orig_x0, regs->regs[1],
+	audit_syscall_entry(regs->syscallno, regs->regs[0], regs->regs[1],
 			    regs->regs[2], regs->regs[3]);
 
 	return regs->syscallno;
-- 
2.34.1



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

* Re: [PATCH v2] arm64: ptrace: use live x0 for seccomp and audit after ptrace
  2026-06-25 10:45 ` [PATCH v2] arm64: ptrace: use live x0 for seccomp and audit after ptrace Yiqi Sun
  2026-06-25 11:11   ` Yiqi Sun
@ 2026-06-25 11:30   ` Yiqi Sun
  2026-06-29 13:09   ` Will Deacon
  2 siblings, 0 replies; 10+ messages in thread
From: Yiqi Sun @ 2026-06-25 11:30 UTC (permalink / raw)
  To: catalin.marinas, linux-arm-kernel
  Cc: linux-kernel, rmk+kernel, ruanjinjie, will

On Thu, Jun 25, 2026 at 07:11:39PM +0800, Yiqi Sun wrote:
> [PATCH v2] arm64: ptrace: use live x0 for seccomp and audit after ptrace

Sorry, this patch was sent twice by mistake.
Please ignore this duplicate copy.


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

* Re: [PATCH v2] arm64: ptrace: use live x0 for seccomp and audit after ptrace
  2026-06-25 10:45 ` [PATCH v2] arm64: ptrace: use live x0 for seccomp and audit after ptrace Yiqi Sun
  2026-06-25 11:11   ` Yiqi Sun
  2026-06-25 11:30   ` Yiqi Sun
@ 2026-06-29 13:09   ` Will Deacon
  2026-06-30 17:29     ` Catalin Marinas
  2 siblings, 1 reply; 10+ messages in thread
From: Will Deacon @ 2026-06-29 13:09 UTC (permalink / raw)
  To: Yiqi Sun
  Cc: catalin.marinas, linux-arm-kernel, linux-kernel, rmk+kernel,
	ruanjinjie, kees, mark.rutland

Hi Yiqi,

On Thu, Jun 25, 2026 at 06:45:02PM +0800, Yiqi Sun wrote:
> On arm64, seccomp obtains syscall arguments via
> syscall_get_arguments(), where arg0 is currently read from
> regs->orig_x0. audit_syscall_entry() in syscall_trace_enter() also
> takes arg0 from regs->orig_x0. However, the syscall wrapper consumes
> live arguments from regs->regs[0..5].
> 
> A ptracer can modify x0 on syscall-enter stop before seccomp and audit
> run, but cannot update orig_x0 through the native syscall-stop
> interface. This can leave seccomp and audit checking stale arg0 while
> the syscall executes with updated live x0.
> 
> Make both paths read arg0 from regs->regs[0], matching the actual
> dispatch arguments and keeping seccomp and audit aligned after ptrace
> updates.
> 
> Fixes: f27bb139c387 ("arm64: Miscellaneous library functions")
> Signed-off-by: Yiqi Sun <sunyiqixm@gmail.com>
> ---
> Changes in v2:
> - Also switch the arm64 audit entry path to use live x0
> - Clarify the orig_x0 synchronization comment in syscall_set_arguments()
> ---
>  arch/arm64/include/asm/syscall.h | 7 +++----
>  arch/arm64/kernel/ptrace.c       | 2 +-
>  2 files changed, 4 insertions(+), 5 deletions(-)

Sashiko has pointed out some issues with this patch that look legitimate
to me:

https://sashiko.dev/#/patchset/2f435bab0d61d0bf8fbaa54203525aae8e8f5371.1782384161.git.sunyiqixm@gmail.com

Specifically, we don't appear to handle NO_SYSCALL properly and the
syscall-exit stop is now going to see the return code instead of the
syscall number.

Looking at this more broadly, it looks like orig_x0 is used for three
different cases:


1. syscall restarting:
			We restore from orig_x0, which should hold the
			original value passed by userspace.

2. syscall_get_arguments():
			This must work correctly vs syscall_set_arguments()
			(returning the latest set x0) but also
			syscall_get_return_value() (so we need to
			distinguish the return value and the argument
			somehow).

3. syscall_rollback():
			Seccomp wants to restore the original values
			passed by userspace.


So (1) and (3) look to require the same behaviour, but (2) wants
something different because it needs to reflect changes made via
syscall_set_arguments().

The bodge we have for (2) today is that syscall_set_arguments() updates
orig_x0, but I think that breaks (1) and (2) which is the underlying
problem you're facing here.

I haven't yet figured out the right way to fix this, but I'd be interested
to hear from others. I think the starting point would be removing orig_x0
from syscall_{get,set}_arguments() altogether so that it accurately
represents the initial value passed by userspace.

Will


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

* Re: [PATCH v2] arm64: ptrace: use live x0 for seccomp and audit after ptrace
  2026-06-29 13:09   ` Will Deacon
@ 2026-06-30 17:29     ` Catalin Marinas
  2026-07-01  8:47       ` Catalin Marinas
  0 siblings, 1 reply; 10+ messages in thread
From: Catalin Marinas @ 2026-06-30 17:29 UTC (permalink / raw)
  To: Will Deacon
  Cc: Yiqi Sun, linux-arm-kernel, linux-kernel, rmk+kernel, ruanjinjie,
	kees, mark.rutland

Hi Will,

On Mon, Jun 29, 2026 at 02:09:42PM +0100, Will Deacon wrote:
> On Thu, Jun 25, 2026 at 06:45:02PM +0800, Yiqi Sun wrote:
> > On arm64, seccomp obtains syscall arguments via
> > syscall_get_arguments(), where arg0 is currently read from
> > regs->orig_x0. audit_syscall_entry() in syscall_trace_enter() also
> > takes arg0 from regs->orig_x0. However, the syscall wrapper consumes
> > live arguments from regs->regs[0..5].
> > 
> > A ptracer can modify x0 on syscall-enter stop before seccomp and audit
> > run, but cannot update orig_x0 through the native syscall-stop
> > interface. This can leave seccomp and audit checking stale arg0 while
> > the syscall executes with updated live x0.
> > 
> > Make both paths read arg0 from regs->regs[0], matching the actual
> > dispatch arguments and keeping seccomp and audit aligned after ptrace
> > updates.
> > 
> > Fixes: f27bb139c387 ("arm64: Miscellaneous library functions")
> > Signed-off-by: Yiqi Sun <sunyiqixm@gmail.com>
> > ---
> > Changes in v2:
> > - Also switch the arm64 audit entry path to use live x0
> > - Clarify the orig_x0 synchronization comment in syscall_set_arguments()
> > ---
> >  arch/arm64/include/asm/syscall.h | 7 +++----
> >  arch/arm64/kernel/ptrace.c       | 2 +-
> >  2 files changed, 4 insertions(+), 5 deletions(-)
> 
> Sashiko has pointed out some issues with this patch that look legitimate
> to me:
> 
> https://sashiko.dev/#/patchset/2f435bab0d61d0bf8fbaa54203525aae8e8f5371.1782384161.git.sunyiqixm@gmail.com
> 
> Specifically, we don't appear to handle NO_SYSCALL properly and the
> syscall-exit stop is now going to see the return code instead of the
> syscall number.

Yes, good points from Sashiko.

> Looking at this more broadly, it looks like orig_x0 is used for three
> different cases:

At least the reported problem is real, the seccomp/audit code needs to
see the values the tracer modified and, IIUC, that's the behaviour x86
implements (it doesn't even clobber the arguments with the return
value). Unlike arm64, powerpc, arm32 expose orig_* to the ptrace
interface. We can't extend the user_pt_regs structure but we could
expose a new structure via ptrace.

> 1. syscall restarting:
> 			We restore from orig_x0, which should hold the
> 			original value passed by userspace.

Yes, we definitely need the orig_x0 since regs[0] was clobbered by the
return value.

> 2. syscall_get_arguments():
> 			This must work correctly vs syscall_set_arguments()
> 			(returning the latest set x0) but also
> 			syscall_get_return_value() (so we need to
> 			distinguish the return value and the argument
> 			somehow).

syscall_set_arguments() also updates orig_x0. W.r.t.
syscall_set_return_value(), it sets regs[0] which also matches what
syscall_get_return_value() reads. But yes, mismatch with the above.

> 3. syscall_rollback():
> 			Seccomp wants to restore the original values
> 			passed by userspace.

The "original values" comment is slightly misleading and just restoring
orig_x0 won't help with the other args anyway. x86 doesn't roll back any
arguments, it just uses the tracer's new values if they've been set via
syscall_trace_enter(). We do the same if the arguments are set via
syscall_set_arguments() since it updates orig_x0 but not if the tracer
did a gpr_set(). I don't think we can safely update orig_x0 via
gpr_set() since it has no idea whether it's in a syscall or not, may
mess up syscall restarting. Interestingly, riscv's SC_RISCV_REGS_TO_ARGS
uses orig_a0, a0 is always the return value even for gpr_get/set(). If
they want to change the syscall arguments, it's only possible via
PTRACE_SET_SYSCALL_INFO.

> So (1) and (3) look to require the same behaviour, but (2) wants
> something different because it needs to reflect changes made via
> syscall_set_arguments().
> 
> The bodge we have for (2) today is that syscall_set_arguments() updates
> orig_x0, but I think that breaks (1) and (2) which is the underlying
> problem you're facing here.

I think the reason (1) needs orig_x0 is because regs[0] was clobbered by
the return value. For (3), orig_x0 and regs[0] are mostly in sync on
this path other than the NO_SYSCALL case where el0_svc_common() sets
regs[0] to -ENOSYS early, before we even reach a tracer.

> I haven't yet figured out the right way to fix this, but I'd be interested
> to hear from others. I think the starting point would be removing orig_x0
> from syscall_{get,set}_arguments() altogether so that it accurately
> represents the initial value passed by userspace.

I thought this might be a cleaner way forward but it's pretty messed up.
Depending on when syscall_get_arguments() is called, it needs different
things: we have seccomp before syscall and regs[0] would do but also
collect_syscall() at the end of a syscall and regs[0] has been clobbered
with the return value.

I also looked at replacing orig_x0 (or its meaning) with a ret_x0 and
only update it on the ERET to user but it breaks the ABI since a tracer
may expect to see the syscall return value in regs[0] on the exit path.

I think we need to keep orig_x0 as our original arg0 throughout the
kernel and just fix the tracer path to sync it on the syscall entry. It
doesn't unclutter the code but it shouldn't break the ABI either (unless
someone relied on the ptrace change x0 and not being noticed by
seccomp). Something like below:

----------------8<-----------------------------
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 4d08598e2891..cd21b301e154 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -2417,6 +2417,18 @@ int syscall_trace_enter(struct pt_regs *regs)
 		ret = report_syscall_entry(regs);
 		if (ret || (flags & _TIF_SYSCALL_EMU))
 			return NO_SYSCALL;
+		/*
+		 * 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()). For compat, orig_r0 is provided directly
+		 * through GPR index 17.
+		 */
+		if (!is_compat_task() && regs->syscallno != NO_SYSCALL)
+			regs->orig_x0 = regs->regs[0];
 	}
 
 	/* Do the secure computing after ptrace; failures should be fast. */
----------------8<-----------------------------

If we want to change the ABI, we could do like riscv and only set the
arguments via PTRACE_SET_SYSCALL_INFO while the GPR ptrace accesses
whatever is in regs[0] - either the original arg or the return value. I
think they changed this inadvertently in 2023 when they moved to the
generic syscall.

We could also introduce NT_ARM_ORIG_X0 but on its own it feels a bit
weird for a tracer to know when the kernel may use orig_x0 or regs[0].

So quick hack above if it works, otherwise we need to look into change
the ABI and hoping no-one notices ;).

-- 
Catalin


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

* Re: [PATCH v2] arm64: ptrace: use live x0 for seccomp and audit after ptrace
  2026-06-30 17:29     ` Catalin Marinas
@ 2026-07-01  8:47       ` Catalin Marinas
  0 siblings, 0 replies; 10+ messages in thread
From: Catalin Marinas @ 2026-07-01  8:47 UTC (permalink / raw)
  To: Will Deacon
  Cc: Yiqi Sun, linux-arm-kernel, linux-kernel, rmk+kernel, ruanjinjie,
	kees, mark.rutland

On Tue, Jun 30, 2026 at 06:29:29PM +0100, Catalin Marinas wrote:
> I think we need to keep orig_x0 as our original arg0 throughout the
> kernel and just fix the tracer path to sync it on the syscall entry. It
> doesn't unclutter the code but it shouldn't break the ABI either (unless
> someone relied on the ptrace change x0 and not being noticed by
> seccomp). Something like below:
> 
> ----------------8<-----------------------------
> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> index 4d08598e2891..cd21b301e154 100644
> --- a/arch/arm64/kernel/ptrace.c
> +++ b/arch/arm64/kernel/ptrace.c
> @@ -2417,6 +2417,18 @@ int syscall_trace_enter(struct pt_regs *regs)
>  		ret = report_syscall_entry(regs);
>  		if (ret || (flags & _TIF_SYSCALL_EMU))
>  			return NO_SYSCALL;
> +		/*
> +		 * 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()). For compat, orig_r0 is provided directly
> +		 * through GPR index 17.
> +		 */
> +		if (!is_compat_task() && regs->syscallno != NO_SYSCALL)
> +			regs->orig_x0 = regs->regs[0];
>  	}
>  
>  	/* Do the secure computing after ptrace; failures should be fast. */
> ----------------8<-----------------------------
> 
> If we want to change the ABI, we could do like riscv and only set the
> arguments via PTRACE_SET_SYSCALL_INFO while the GPR ptrace accesses
> whatever is in regs[0] - either the original arg or the return value. I
> think they changed this inadvertently in 2023 when they moved to the
> generic syscall.

Looking at some of the history, the ABI break on riscv was noticed, so
definitely not an option for us. I think the change would have looked
something like below. We could keep regs[0] match orig_x0 for entry but
it gets out of sync later, so still confusing for gdb/lldb/strace.

---------------8<----------------------
diff --git a/arch/arm64/include/asm/syscall.h b/arch/arm64/include/asm/syscall.h
index 5e4c7fc44f73..c58ac8d25692 100644
--- a/arch/arm64/include/asm/syscall.h
+++ b/arch/arm64/include/asm/syscall.h
@@ -93,19 +93,12 @@ static inline void syscall_set_arguments(struct task_struct *task,
 					 struct pt_regs *regs,
 					 const unsigned long *args)
 {
-	regs->regs[0] = args[0];
+	regs->orig_x0 = args[0];
 	regs->regs[1] = args[1];
 	regs->regs[2] = args[2];
 	regs->regs[3] = args[3];
 	regs->regs[4] = args[4];
 	regs->regs[5] = args[5];
-
-	/*
-	 * Also copy the first argument into orig_x0
-	 * so that syscall_get_arguments() would return it
-	 * instead of the previous value.
-	 */
-	regs->orig_x0 = regs->regs[0];
 }
 
 /*
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])
 
 #ifdef CONFIG_COMPAT
diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
index 358ddfbf1401..a80596531a5c 100644
--- a/arch/arm64/kernel/syscall.c
+++ b/arch/arm64/kernel/syscall.c
@@ -66,6 +66,7 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
 
 	regs->orig_x0 = regs->regs[0];
 	regs->syscallno = scno;
+	syscall_set_return_value(current, regs, -ENOSYS, 0);
 
 	/*
 	 * BTI note:
@@ -111,8 +112,6 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
 		 * setting the return value is unlikely to do anything sensible
 		 * anyway.
 		 */
-		if (scno == NO_SYSCALL)
-			syscall_set_return_value(current, regs, -ENOSYS, 0);
 		scno = syscall_trace_enter(regs);
 		if (scno == NO_SYSCALL)
 			goto trace_exit;

-- 
Catalin


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

end of thread, other threads:[~2026-07-01  8:48 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-29  6:54 [PATCH] fix: arm64: syscall: use live x0 for syscall_get_arguments() arg0 Yiqi Sun
2026-06-01 12:43 ` Will Deacon
2026-06-03  9:07   ` Yiqi Sun
2026-06-19 16:05     ` Will Deacon
2026-06-25 10:45 ` [PATCH v2] arm64: ptrace: use live x0 for seccomp and audit after ptrace Yiqi Sun
2026-06-25 11:11   ` Yiqi Sun
2026-06-25 11:30   ` Yiqi Sun
2026-06-29 13:09   ` Will Deacon
2026-06-30 17:29     ` Catalin Marinas
2026-07-01  8:47       ` Catalin Marinas

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