Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Will Deacon <will@kernel.org>
To: Vladimir Murzin <vladimir.murzin@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	rmikey@meta.com,
	"Peter Zijlstra \(Intel\)" <peterz@infradead.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Jinjie Ruan <ruanjinjie@huawei.com>,
	kernel-team@meta.com, linux-kernel@vger.kernel.org,
	Breno Leitao <leitao@debian.org>,
	bpf@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH RFC] arm64: entry: PSTATE_I_SET is leaking on pseudo NMI mode
Date: Mon, 10 Aug 2026 16:05:42 +0100	[thread overview]
Message-ID: <annoxlkkq1NpujsP@willie-the-truck> (raw)
In-Reply-To: <2a97f777-d33a-4b3d-8c8c-6f6894f04c29@arm.com>

On Mon, Aug 10, 2026 at 01:42:15PM +0100, Vladimir Murzin wrote:
> On 8/10/26 12:43, Will Deacon wrote:
> > On Fri, Aug 07, 2026 at 09:29:12AM -0700, Breno Leitao wrote:
> >> On Fri, Aug 07, 2026 at 07:58:21AM -0700, Breno Leitao wrote:
> >>> Meanwhile, I will try to ftrace the writes to PMR and regs->pmr to get
> >>> a better grasp of the states machine we are in (probably on Monday).
> >> It seems LLM found a very easy to reproduce this:
> >>
> >> 	bash-5.1# dmesg
> >>
> >> 	bash-5.1#  cd /sys/kernel/tracing
> >> 	echo 'r:pmr vfs_read bad=+0($retval):u64' >> kprobe_events
> >> 	echo 1 > events/kprobes/pmr/enable
> > Nice, that triggers straightforwardly in QEMU for me. The diff below
> > (which implements my suggestion from [1]) seems to fix the issue, but
> > it would be good to hear feedback from one of the Arm folks.
> > 
> > [1] https://lore.kernel.org/all/anXgWRmcjwPKG7N5@willie-the-truck/
> > 
> > --->8
> > 
> > diff --git a/arch/arm64/include/asm/daifflags.h b/arch/arm64/include/asm/daifflags.h
> > index 795b35128467..691ee5f86dbe 100644
> > --- a/arch/arm64/include/asm/daifflags.h
> > +++ b/arch/arm64/include/asm/daifflags.h
> > @@ -132,7 +132,7 @@ static __always_inline void local_daif_inherit(struct pt_regs *regs)
> >                 trace_hardirqs_on();
> > 
> >         if (system_uses_irq_prio_masking())
> > -               gic_write_pmr(regs->pmr);
> > +               gic_write_pmr(regs->pmr & ~GIC_PRIO_PSR_I_SET);
> > 
> >         /*
> >          * We can't use local_daif_restore(regs->pstate) here as
> > 
> 
> I have no strong opinion on the change, but I struggle to see how it
> fits into the big picture.
> 
> Specifically, I have difficulty explaining this change in isolation.
> Everywhere else, we try to keep DAIF.I and GIC_PRIO_PSR_I_SET in sync,
> so it is not clear why we should allow them to go out of sync when
> inheriting the exception state from the previous context.

Yeah, I think you're right, and looking at it some more it means we
end up with the pmr in the IRQON state which will break
arch_irqs_disabled().

> At the same time, IIUC, the only reason we call local_irq_disable()
> (which also causes the states to become unsynchronized) in
> arm64_exit_to_kernel_mode() is for preemption path. So avoiding
> local_irq_disable() (and preemption) when we interrupted a
> non-preemptible context seems easier to follow.

In the past, we only preempted when returning to EL1 off the back of an
IRQ, but that was changed in ae654112eac0 ("arm64: entry: Use split
preemption logic") which I think is where this bug was introduced.

So we could probably hack something as you suggest, but maybe the best
option is to take patches 8 and 9 from your FEAT_NMI series? WDYT?

Will


  reply	other threads:[~2026-08-10 15:08 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-07 11:45 [PATCH RFC] arm64: entry: PSTATE_I_SET is leaking on pseudo NMI mode Breno Leitao
2026-08-07 13:40 ` Will Deacon
2026-08-07 13:57 ` Mark Rutland
2026-08-07 14:49   ` Vladimir Murzin
2026-08-10 12:44     ` Jinjie Ruan
2026-08-10 13:04       ` Vladimir Murzin
2026-08-11  2:47         ` Jinjie Ruan
2026-08-07 14:58   ` Breno Leitao
2026-08-07 16:29     ` Breno Leitao
2026-08-10 11:43       ` Will Deacon
2026-08-10 12:42         ` Vladimir Murzin
2026-08-10 15:05           ` Will Deacon [this message]
2026-08-10 16:39             ` Vladimir Murzin
2026-08-10 12:51         ` Jinjie Ruan
2026-08-10 12:59         ` Breno Leitao

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=annoxlkkq1NpujsP@willie-the-truck \
    --to=will@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=kernel-team@meta.com \
    --cc=leitao@debian.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=peterz@infradead.org \
    --cc=rmikey@meta.com \
    --cc=ruanjinjie@huawei.com \
    --cc=vladimir.murzin@arm.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