Re: [PATCH RFC] arm64: entry: PSTATE_I_SET is leaking on pseudo NMI mode

[Breno Leitao <leitao@debian.org>]

On Fri, Aug 07, 2026 at 02:57:15PM +0100, Mark Rutland wrote:
> Hi Breno,
> 
> On Fri, Aug 07, 2026 at 04:45:31AM -0700, Breno Leitao wrote:
> > 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.

local_daif_restore() keeps GIC_PRIO_PSR_I_SET on aborts, which is not
the case here.

so, as you said, GIC_PRIO_PSR_I_SET is set in PMR and then cleared in
local_daif_restore().

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

Right, that was my reading as well.

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

I think I can find a reproducer for us, give me a few hours.

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

Sorry, I meant PSTATE_I_SET. 

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

For the live PMR register I follow it now: arch_local_save_flags()
returns the PMR and nothing there looks at PSTATE, so a context masked
by PSTATE.I has to advertise that in the PMR or irqs_disabled() lies.

What I don't follow is the saved copy. regs_irqs_disabled() already
consults both halves:

      return (regs->pstate & PSR_I_BIT) || !irqs_priority_unmasked(regs);

so GIC_PRIO_PSR_I_SET in regs->pmr looks redundant for anything querying
the frame.

That matters for the warning, because local_daif_inherit() reloading the
entry-time 'GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET' is what puts 0xf0 in
the live PMR before the handler body runs.

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

Thanks for the answers so far,
--breno