From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from canpmsgout06.his.huawei.com (canpmsgout06.his.huawei.com [113.46.200.221]) (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 0E98141DE05 for ; Wed, 15 Jul 2026 11:39:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=113.46.200.221 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784115592; cv=none; b=GtVJ0HaniYaiFbjdTtP40ObyjwwWCpGPQrLqBIUrb6WzCzEGGTtoEuHfo7tOOGNZjmqBiSR8WOvraZC6l3sPyZC/99vFCgTbavgap5IG6L/0AEH8+cmAGhMCWKZpsIuYWsl64xxApHtaCyiYEkcBPZsd0jS/8lxjH5+avLYt1L0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784115592; c=relaxed/simple; bh=e/wIzZDhx8z/Z/mu6+s13/SENKHZk03+efAsL935Yro=; h=Message-ID:Date:MIME-Version:Subject:To:CC:References:From: In-Reply-To:Content-Type; b=me3M8/5q32r0xMuq3ond7rX7ufU4KWVFWZDaIubtfwYFnRiG96BlA9v+bu1V4vjr67ivZxW2vSXUdCjqBQ8aelbrE0wpCG2ZepDq7iZ6PVe91y7D1HiVilOtc7GwUGH+zZ50VZ+WssFHILgriGvzLaJd/lBsP1Na2WZkitMP/+Y= 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=lDxNM4Uj; arc=none smtp.client-ip=113.46.200.221 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="lDxNM4Uj" dkim-signature: v=1; a=rsa-sha256; d=huawei.com; s=dkim; c=relaxed/relaxed; q=dns/txt; h=From; bh=KJRMVavXKsYTe80eRZtLYQwVngIGjT5OT9X370LluXc=; b=lDxNM4Ujf/jwPWfJQZ3bZzFBxRWgJNIWTXFL//aIKOiPLGlYC2ANkmNozESfnS/SJmfmJnUvm FXSqGXkFXUgLhWBo4H/JVTw0Ujzfgp10k7jRg/ujR0peSQH1Uzu6gUvfgiXXo1Ko3JNeVZU3r/C eUO4VWFPLskw8UoYjU9h5TU= Received: from mail.maildlp.com (unknown [172.19.163.104]) by canpmsgout06.his.huawei.com (SkyGuard) with ESMTPS id 4h0YrM2BsRzRhQR; Wed, 15 Jul 2026 19:30:27 +0800 (CST) Received: from dggpemf500011.china.huawei.com (unknown [7.185.36.131]) by mail.maildlp.com (Postfix) with ESMTPS id 79F434057F; Wed, 15 Jul 2026 19:39:44 +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; Wed, 15 Jul 2026 19:39:43 +0800 Message-ID: <55b6734e-39b9-40bb-93ae-b869f17476d5@huawei.com> Date: Wed, 15 Jul 2026 19:39:43 +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] arm64: syscall: Ensure saved x0 is kept in-sync with tracer updates To: Will Deacon , CC: , Kees Cook , Mark Rutland , Yiqi Sun , Catalin Marinas References: <20260714143600.23853-1-will@kernel.org> From: Jinjie Ruan In-Reply-To: <20260714143600.23853-1-will@kernel.org> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit X-ClientProxiedBy: kwepems500001.china.huawei.com (7.221.188.70) To dggpemf500011.china.huawei.com (7.185.36.131) 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 > Cc: Jinjie Ruan > Cc: Mark Rutland > Reported-by: Yiqi Sun > Link: https://lore.kernel.org/all/20260529065444.1336608-1-sunyiqixm@gmail.com/ > Suggested-by: Catalin Marinas > Fixes: a5cd110cb836 ("arm64/ptrace: run seccomp after ptrace") > Signed-off-by: Will Deacon > --- > 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); >