Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Will Deacon <will@kernel.org>
To: Catalin Marinas <catalin.marinas@arm.com>
Cc: Yiqi Sun <sunyiqixm@gmail.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, rmk+kernel@armlinux.org.uk,
	ruanjinjie@huawei.com, kees@kernel.org, mark.rutland@arm.com
Subject: Re: [PATCH v2] arm64: ptrace: use live x0 for seccomp and audit after ptrace
Date: Fri, 10 Jul 2026 15:04:52 +0100	[thread overview]
Message-ID: <alD8BPgSKxHEeydu@willie-the-truck> (raw)
In-Reply-To: <akP8-ddTn9bhIDEW@arm.com>

Hi Catalin,

Thanks for helping with this.

On Tue, Jun 30, 2026 at 06:29:29PM +0100, Catalin Marinas wrote:
> On Mon, Jun 29, 2026 at 02:09:42PM +0100, Will Deacon wrote:
> > 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().

Ah yes, that makes sense. I got thrown by the comment in
include/asm-generic/syscall.h because it makes it sound like we need to
rewind everything to the initial state from userspace.

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

Urgh, collect_syscall() really gets in the way here. I *think* all the
other callers of syscall_get_arguments() happen on the entry path (i.e.
before we've clobbered regs[0] with the return value), which would mean
we could avoid touching orig_x0 in syscall.h, but collect_syscall() is
driven from /proc/pid/syscall so it can hit the syscall exit path too.
I wondered about setting syscallno to NO_SYSCALL earlier (i.e. before
setting the return value), but I think that breaks syscall restarting.

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

I think that's correct. I'll spin a proper patch...

Cheers,

Will


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

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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
2026-07-10 14:04       ` Will Deacon [this message]
2026-07-13  7:49       ` Jinjie Ruan
2026-07-13 14:07         ` Will Deacon
2026-07-14  3:20           ` Jinjie Ruan
2026-07-14 13:56             ` Will Deacon
2026-07-13  6:59 ` [PATCH] fix: arm64: syscall: use live x0 for syscall_get_arguments() arg0 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=alD8BPgSKxHEeydu@willie-the-truck \
    --to=will@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=kees@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=rmk+kernel@armlinux.org.uk \
    --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