From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from canpmsgout08.his.huawei.com (canpmsgout08.his.huawei.com [113.46.200.223]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9350E393DDB for ; Mon, 13 Jul 2026 07:49:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=113.46.200.223 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783928975; cv=none; b=Vs8Jm+ro0NxiLpMLluDFOflNagBDFFfIt6epH+97amvTF46FG6SuJjICcgawV63pP4RykH9RVFluEh7Ziqkg9x7l1G8bkjPCaG+dE29O9LJYXzv8eTtNDbp/OYLkY+xkw1IhFUFzZQUMp/yCnvwowo9qBi6NEP4wpzwmlt91arA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783928975; c=relaxed/simple; bh=qVpgv+rzBNtlqog/WQojP2uT8a3pqNZnoVox/4EDbdo=; h=Message-ID:Date:MIME-Version:Subject:To:CC:References:From: In-Reply-To:Content-Type; b=q+U+4a3FfbAli9d8iphSou+z01F1Xmrc0XG6iTQ9oKV8GmPYYJ6ORml2F84FEp+9srUb26y/KDGxMH3Y6S0Nle5uHB2hhP7gII2ysvg0aIVIafYHMFsDAJPFHdln+qbSXSF3yWLEdegok/A+m59oQ/wX0xB3HmEl7tUv/bgC5Bo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com; spf=pass smtp.mailfrom=huawei.com; dkim=pass (1024-bit key) header.d=huawei.com header.i=@huawei.com header.b=e79kpjXE; arc=none smtp.client-ip=113.46.200.223 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=huawei.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=huawei.com header.i=@huawei.com header.b="e79kpjXE" dkim-signature: v=1; a=rsa-sha256; d=huawei.com; s=dkim; c=relaxed/relaxed; q=dns/txt; h=From; bh=00dwqZHVc0FEuHqjwXW/3nir90oyie8Ek4oT/YeulX0=; b=e79kpjXEU0R1g6JsGAVYZTTubeITyeLe9BIohSGdoVoqVqDxV73JulO5e/Cn6geINygNccitZ A1odlrBqqJf5WoKXg1KstZQHGUQC4aBVQLKTTgA8K4dxULMXcXZ8+lkQVS2tmmwaaPKcQktzYyn sG9gFSoucBereibwCRRBEhI= Received: from mail.maildlp.com (unknown [172.19.163.15]) by canpmsgout08.his.huawei.com (SkyGuard) with ESMTPS id 4gzDqX5LHFzmVXs; Mon, 13 Jul 2026 15:40:08 +0800 (CST) Received: from dggpemf500011.china.huawei.com (unknown [7.185.36.131]) by mail.maildlp.com (Postfix) with ESMTPS id 044E640586; Mon, 13 Jul 2026 15:49:26 +0800 (CST) Received: from [10.67.109.254] (10.67.109.254) by dggpemf500011.china.huawei.com (7.185.36.131) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.11; Mon, 13 Jul 2026 15:49:19 +0800 Message-ID: <1610f167-1af0-4d20-877c-b362b9b49d94@huawei.com> Date: Mon, 13 Jul 2026 15:49:18 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] arm64: ptrace: use live x0 for seccomp and audit after ptrace To: Catalin Marinas , Will Deacon CC: Yiqi Sun , , , , , References: <20260529065444.1336608-1-sunyiqixm@gmail.com> <2f435bab0d61d0bf8fbaa54203525aae8e8f5371.1782384161.git.sunyiqixm@gmail.com> From: Jinjie Ruan In-Reply-To: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit X-ClientProxiedBy: kwepems200001.china.huawei.com (7.221.188.67) To dggpemf500011.china.huawei.com (7.185.36.131) On 7/1/2026 1:29 AM, Catalin Marinas wrote: > 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 >>> --- >>> 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]; Hi, Will Can we place this fix in report_syscall_entry()? The generic entry framework has already reserved the function arch_ptrace_report_syscall_permit_entry() for architecture-specific customization, so switching to it might be more convenient. > } > > /* 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 ;). >