From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 87553C2A09B for ; Fri, 7 Aug 2026 13:57:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Cc:List-Subscribe: List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To: Content-Transfer-Encoding:Content-Type:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=04YxBDSfUvXnl+ezO5VcnvMVrZM6FctX2K5h0cF0mms=; b=UUj/iFdZBtqfCjLMfFduCAOosy ClfApFFwnLGfwuU5ZfSccCtTYETKLrWj6DDxv/S5bt65rFBFKXQbrs5DwG0YrdnlrkT8DAe/g4GVE L0DeY6o6OzR+y/BWv/x6rsgJl5sAIGIHm2c9bGaOFGpiNdSu6bEJ3R0MqNCSZjfXONQhWm9nZXlSY JanDx34xLJePRNanvVakht5hT/44dGe4PmPFiUCZbpAkU0ixqOzok5CVtFmBFyLdUEY9sx1U+mdS8 psd+whXLXDqsNuLpp6FrXjO4bZVbu/MkoZMAX7531HCfZRjNjCv/rm13NRckda1oYViVOSsPJwmkh ZXPvOKig==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.99.1 #2 (Red Hat Linux)) id 1wsL4h-00000008CiY-3l4w; Fri, 07 Aug 2026 13:57:27 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.99.1 #2 (Red Hat Linux)) id 1wsL4f-00000008Cgd-0EC9 for linux-arm-kernel@lists.infradead.org; Fri, 07 Aug 2026 13:57:27 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 7133B1BCA; Fri, 7 Aug 2026 06:57:18 -0700 (PDT) Received: from J2N7QTR9R3 (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 9F14F3F86F; Fri, 7 Aug 2026 06:57:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=arm.com; s=foss; t=1786111042; bh=Z72ieCKGBrnJ1R0wWPsVqo7s6jCtOFoq1qgV6zt/2gI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=dU5zKa0EcEAsEnfu2Qy+ka5i4OThCAavFl2jizuDYKelhfqyW1HHNUFulz8XCapzy HwfqsNnquDrJYICvLpWU626Hy7j6DOx/cgspynRPglFNEsKzZHz2opDvjB1eJJPm+4 Ia35TUaRtaKop6hDUgt4+4qfmzBVlHCuVcfqfPlA= Date: Fri, 7 Aug 2026 14:57:15 +0100 From: Mark Rutland To: Breno Leitao Subject: Re: [PATCH RFC] arm64: entry: PSTATE_I_SET is leaking on pseudo NMI mode Message-ID: References: <20260807-arm64_fix-v1-1-d069ccf9d71b@debian.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20260807-arm64_fix-v1-1-d069ccf9d71b@debian.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.9.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20260807_065725_271602_473B0479 X-CRM114-Status: GOOD ( 42.20 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Vladimir Murzin , rmikey@meta.com, "Peter Zijlstra \(Intel\)" , Catalin Marinas , Jinjie Ruan , kernel-team@meta.com, linux-kernel@vger.kernel.org, bpf@vger.kernel.org, Will Deacon , linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Breno, On Fri, Aug 07, 2026 at 04:45:31AM -0700, Breno Leitao wrote: > Running retsnoop on a kernel with GIC priority masking and > CONFIG_ARM64_DEBUG_PRIORITY_MASKING=y trips the ICC_PMR_EL1 sanity check > in __pmr_local_irq_disable(): > > WARNING: ./arch/arm64/include/asm/irqflags.h:63 at arm64_exit_to_kernel_mode+0xb8/0xc0, CPU#40: retsnoop/31805 > CPU: 40 UID: 0 PID: 31805 Comm: retsnoop Not tainted 7.2.0-rc6-next-20260805 #7 PREEMPTLAZY > pstate: 234013c9 (nzCv DAIF +PAN -UAO +TCO +DIT +SSBS BTYPE=--) > pc : arm64_exit_to_kernel_mode (arch/arm64/kernel/entry-common.c:63) > lr : el1_abort (arch/arm64/kernel/entry-common.c:323) > pmr: 000000f0 > Call trace: > D) arm64_exit_to_kernel_mode (arch/arm64/kernel/entry-common.c:63) (P) > el1_abort (arch/arm64/kernel/entry-common.c:323) > el1h_64_sync_handler (arch/arm64/kernel/entry-common.c:449) > C) el1h_64_sync (arch/arm64/kernel/entry.S:589) > copy_from_kernel_nofault (mm/maccess.c:52) (P) > bpf_probe_read_kernel (kernel/trace/bpf_trace.c:268) > bpf_prog_db21a1730c2407e5_calib_exit+0xf0/0x160 > trace_call_bpf (kernel/trace/bpf_trace.c:147) > kretprobe_perf_func (kernel/trace/trace_kprobe.c:1750) > kretprobe_dispatcher (kernel/trace/trace_kprobe.c:1875) > __kretprobe_trampoline_handler (kernel/kprobes.c:2116) > kretprobe_brk_handler (arch/arm64/kernel/probes/kprobes.c:422) > call_el1_break_hook (arch/arm64/kernel/debug-monitors.c:244) > do_el1_brk64 (arch/arm64/kernel/debug-monitors.c:266) > B) el1_brk64 (arch/arm64/kernel/entry-common.c:427) > el1h_64_sync_handler (arch/arm64/kernel/entry-common.c:481) > el1h_64_sync (arch/arm64/kernel/entry.S:589) > invoke_syscall (arch/arm64/kernel/syscall.c:49) (P) > do_el0_svc (arch/arm64/kernel/syscall.c:140) > el0_svc (arch/arm64/kernel/entry-common.c:736) > el0t_64_sync_handler (arch/arm64/kernel/entry-common.c:755) > A) el0t_64_sync (arch/arm64/kernel/entry.S:594) > > This is my understand of the current situation: > > A) A task enters the kernel via a syscall. > * PSTATE_I_SET becomes set on the live PMR > * regs->PMR doesn't have PSR_I_SET set At this point, regs->pmr will be the value of PMR when we were executing in userspace, which should be the value of regs->pmr the last time we returned to userspace. That should be GIC_PRIO_IRQON, as configured by start_thread_common(). The entry asm will set PMR to 'GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET', but that should be reset to GIC_PRIO_IRQON when el0_svc() calls local_daif_restore(DAIF_PROCCTX). Within invoke_syscall(), we should have DAIF==0 and PMR==GIC_PRIO_IRQON. > B) A BRK fires at EL1 and that is what leaves the live PMR > with PSR_I_SET. > * At this stage PSTATE_I_SET is set on both on PMR and regs->PMR The value in the regs->pmr should be GIC_PRIO_IRQON, without GIC_PRIO_PSR_I_SET. As we don't unmask anything during BRK handling, the live PMR should contain 'GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET'. > C) A nested synchronous exception happens (Not sure why -- BPF related) > * Now both the live PMR and regs->pmr have PSR_I_SET. The presence of 'el1_abort()' in the trace suggests that BPF tried to access memory which faulted. That looks to be a result of bpf_probe_read_kernel() and copy_from_kernel_nofault(). Regardless of BPF, we can probably trigger a synchronous exception with a WARN() or similar, so we will need to handle synchronous exceptions from the same contexts. In general, probing kernel VAs is fraught with danger even with a fault handler, and IMO copy_from_kernel_nofault() is a dangerous and poorly conceived interface. For example, if you're probing an arbitrary kernel VA, you have no idea whether you're about to poke MMIO and crash the system. It would be interesting to know what the BPF program is doing, in case it could crash the kernel in other ways we can't prevent here. > D) On the way out, local_irq_disable() → __pmr_local_irq_disable() warns. > It detects that PMR is different than GIC_PRIO_IRQON and GIC_PRIO_IRQOFF, > given live PMR and regs->PMR have PSTATE_I_SET ORed. Only the live value of PMR is important here, and the value of regs->pmr is immaterial. The warning is here to capture the fact that if __pmr_local_irq_disable() were to set PMR to GIC_PRIO_IRQOFF, it would effectively discard GIC_PRIO_PSR_I_SET and change the masked priority, which has a bunch of secondary impacts for things like idle. We'll need to work through those impacts; we might be able to relax the check. > How to fix it? I don't know very well. The complete fix (which Ada and Vladimir have both been working on) is to get rid of GIC_PRIO_PSR_I_SET entirely, which requires structural changes in a few places/ I don't think we'd realised there was an extant bug of this shape, so we'll need to consider what we can do to fix this in a backportable way. > I am not certain that we want to have PSTATE_I_SET ever be sent to > regs->pstate. Do we ever need PSTATE_I_SET in regs->pstate? I'm not sure what you mean here. PSTATE_I_SET doesn't exist, and I'm not sure whether you're asking about the PSTATE.DAIF bits saved in regs->pstate, or the PMR value in regs->pmr. Regardless of pseudo-NMI, is is essential that regs->pstate holds a complete snapshot of SPSR, including the I bit. With pseudo-NMI, it is essential that the full PMR value (including GIC_PRIO_PSR_I_SET) is saved into regs->pmr. However, as above, AFAICT only the live value matters here. > In the current patch, I found that disabling IRQ in case it is disabled, > would solve the warning, but, this seems more a hack than a proper fix, > perhaps. > > Fixes: ae654112eac0 ("arm64: entry: Use split preemption logic") > Signed-off-by: Breno Leitao > --- > arch/arm64/kernel/entry-common.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c > index ceb4eb11232a6..fcce9ccd37108 100644 > --- a/arch/arm64/kernel/entry-common.c > +++ b/arch/arm64/kernel/entry-common.c > @@ -55,7 +55,13 @@ static noinstr irqentry_state_t arm64_enter_from_kernel_mode(struct pt_regs *reg > static void noinstr arm64_exit_to_kernel_mode(struct pt_regs *regs, > irqentry_state_t state) > { > - local_irq_disable(); > + /* > + * Only irqentry_exit_to_kernel_mode_preempt() needs interrupts masked, > + * and it returns early when regs had them disabled. Skipping the > + * disable avoids clobbering a PMR the irqflags API does not expect. > + */ > + if (!regs_irqs_disabled(regs)) > + local_irq_disable(); > irqentry_exit_to_kernel_mode_preempt(regs, state); I think that might happen to work as a bodge, but I don't think this is a proper fix, and we'll need a clearer explanation of what's going on. Mark. > local_daif_mask(); > mte_check_tfsr_exit(); > > --- > base-commit: ea2bff00da89d7767d677bb68470130ba96f4928 > change-id: 20260807-arm64_fix-47cad8fb6323 > > Best regards, > -- > Breno Leitao >