Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Will Deacon <will@kernel.org>
To: Jinjie Ruan <ruanjinjie@huawei.com>
Cc: linux-arm-kernel@lists.infradead.org, catalin.marinas@arm.com,
	kernel-team@android.com, linux-kernel@vger.kernel.org,
	Kees Cook <kees@kernel.org>, Mark Rutland <mark.rutland@arm.com>,
	Yiqi Sun <sunyiqixm@gmail.com>
Subject: Re: [PATCH v2] arm64: syscall: Ensure saved x0 is kept in-sync with tracer updates
Date: Tue, 21 Jul 2026 15:05:01 +0100	[thread overview]
Message-ID: <al98jdcleu-6tMDL@willie-the-truck> (raw)
In-Reply-To: <8d3d3ea7-b250-4022-8ed7-6d438b43a518@huawei.com>

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@kernel.org
> > 
> 
> 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.
Having said that, I notice we're already different in how we invoke
audit_syscall_entry() :(

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

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

>  #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


  reply	other threads:[~2026-07-21 14:05 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-16 12:06 [PATCH v2] arm64: syscall: Ensure saved x0 is kept in-sync with tracer updates Will Deacon
2026-07-16 12:28 ` Jinjie Ruan
2026-07-16 16:48 ` Will Deacon
2026-07-17 17:54   ` Will Deacon
2026-07-20  3:48     ` Jinjie Ruan
2026-07-21 14:05       ` Will Deacon [this message]
2026-07-23  4:12         ` Jinjie Ruan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=al98jdcleu-6tMDL@willie-the-truck \
    --to=will@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=kees@kernel.org \
    --cc=kernel-team@android.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=ruanjinjie@huawei.com \
    --cc=sunyiqixm@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox