The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH RFC] arm64: entry: PSTATE_I_SET is leaking on pseudo NMI mode
@ 2026-08-07 11:45 Breno Leitao
  2026-08-07 13:40 ` Will Deacon
  2026-08-07 13:57 ` Mark Rutland
  0 siblings, 2 replies; 6+ messages in thread
From: Breno Leitao @ 2026-08-07 11:45 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Peter Zijlstra (Intel),
	Mark Rutland, Jinjie Ruan
  Cc: linux-arm-kernel, linux-kernel, bpf, rmikey, kernel-team,
	Breno Leitao

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

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

C) A nested synchronous exception happens (Not sure why -- BPF related)
	* Now both the live PMR and regs->pmr have PSR_I_SET.

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.

How to fix it? I don't know very well.

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?

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 <leitao@debian.org>
---
 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);
 	local_daif_mask();
 	mte_check_tfsr_exit();

---
base-commit: ea2bff00da89d7767d677bb68470130ba96f4928
change-id: 20260807-arm64_fix-47cad8fb6323

Best regards,
--  
Breno Leitao <leitao@debian.org>


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH RFC] arm64: entry: PSTATE_I_SET is leaking on pseudo NMI mode
  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
  1 sibling, 0 replies; 6+ messages in thread
From: Will Deacon @ 2026-08-07 13:40 UTC (permalink / raw)
  To: Breno Leitao
  Cc: Catalin Marinas, Peter Zijlstra (Intel), Mark Rutland,
	Jinjie Ruan, linux-arm-kernel, linux-kernel, bpf, rmikey,
	kernel-team, maz, ada.coupriediaz, vladimir.murzin

[+Marc, Ada and Vladimir]

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

That's warning because we're trying to disable interrupts in the PMR but
the existing PMR value is not IRQON or IRQOFF. The implication later is
that GIC_PRIO_PSR_I_SET is set.

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

Just a couple of nits that might help others:

  - The description refers to PSTATE_I_SET, which doesn't exist
  - The steps refer to 'regs->PMR' as though it's always the same memory
    location

> 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

Hmm. My (possibly incorrect) reading of the code here is that the syscall
entry path will clear GIC_PRIO_PSR_I_SET in the live PMR register when
unmasking interrupts via local_daif_restore(DAIF_PROCCTX) ...

> 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

... so the EL1 debug handler for the BRK shouldn't save a PMR value with
GIC_PRIO_PSR_I_SET into the regs. We leave interrupts disabled for debug
exceptions, so I agree that the PMR register retains 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.

That looks like it's the case and local_daif_inherit() won't change
anything. I wonder if we should drop GIC_PRIO_PSR_I_SET when writing
the PMR there?

Will

(retaining the rest of the mail for the folks I've added)

> 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.
>
> How to fix it? I don't know very well.
> 
> 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?
> 
> 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 <leitao@debian.org>
> ---
>  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);
>  	local_daif_mask();
>  	mte_check_tfsr_exit();
> 
> ---
> base-commit: ea2bff00da89d7767d677bb68470130ba96f4928
> change-id: 20260807-arm64_fix-47cad8fb6323
> 
> Best regards,
> --  
> Breno Leitao <leitao@debian.org>
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RFC] arm64: entry: PSTATE_I_SET is leaking on pseudo NMI mode
  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-07 14:58   ` Breno Leitao
  1 sibling, 2 replies; 6+ messages in thread
From: Mark Rutland @ 2026-08-07 13:57 UTC (permalink / raw)
  To: Breno Leitao
  Cc: Catalin Marinas, Will Deacon, Peter Zijlstra (Intel), Jinjie Ruan,
	linux-arm-kernel, linux-kernel, bpf, rmikey, kernel-team,
	Vladimir Murzin, Ada Couprie Diaz

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 <leitao@debian.org>
> ---
>  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 <leitao@debian.org>
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RFC] arm64: entry: PSTATE_I_SET is leaking on pseudo NMI mode
  2026-08-07 13:57 ` Mark Rutland
@ 2026-08-07 14:49   ` Vladimir Murzin
  2026-08-07 14:58   ` Breno Leitao
  1 sibling, 0 replies; 6+ messages in thread
From: Vladimir Murzin @ 2026-08-07 14:49 UTC (permalink / raw)
  To: Mark Rutland, Breno Leitao
  Cc: Catalin Marinas, Will Deacon, Peter Zijlstra (Intel), Jinjie Ruan,
	linux-arm-kernel, linux-kernel, bpf, rmikey, kernel-team,
	Ada Couprie Diaz

On 8/7/26 14:57, Mark Rutland wrote:
> 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.
> 

That matches my understanding of call trace

Call stack                    |live DAIF:PMR  | regs DAIF:PMR
--------------------------------------------------------------
<exception>
el0_svc                       | DAIF:IRQ_ON+I | daif:IRQ_ON
 - local_daif_restore         | daif:IRQ_ON   |
<exception>            
el1_brk                       | DAIF:IRQ_ON+I | daif:IRQ_ON
<exception>            
el1_abort                     | DAIF:IRQ_ON+I | DAIF:IRQ_ON+I
 - local_daif_inherit         | DAIF_IRQ_ON+I |
 - arm64_exit_to_kernel_mode  |               |
   - local_irq_disable        |               |

where:
- DAIF is DAIF_MASK
- daif is DAIF_PROCCTX
- IRQ_ON is GIC_PRIO_IRQON
- I is GIC_PRIO_PSR_I_SET


> 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 <leitao@debian.org>
>> ---
>>  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.
> 

Agreed on the need for a clearer explanation.

Perhaps local_irq_disable() isn't the right API here, and we should
use local_daif_restore(DAIF_PROCCTX_NOIRQ) instead?

Looking ahead, making an early decision on whether to preempt based on
regs_irqs_disabled() seems like a reasonable approach. That's what
I've done in [1] (though keep in mind that I forgot to update
el0_irq).

[1] https://lore.kernel.org/linux-arm-kernel/20260727163453.7969-10-vladimir.murzin@arm.com/

Cheers
Vladimir

> Mark.
> 
>>  	local_daif_mask();
>>  	mte_check_tfsr_exit();
>>
>> ---
>> base-commit: ea2bff00da89d7767d677bb68470130ba96f4928
>> change-id: 20260807-arm64_fix-47cad8fb6323
>>
>> Best regards,
>> --  
>> Breno Leitao <leitao@debian.org>
>>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RFC] arm64: entry: PSTATE_I_SET is leaking on pseudo NMI mode
  2026-08-07 13:57 ` Mark Rutland
  2026-08-07 14:49   ` Vladimir Murzin
@ 2026-08-07 14:58   ` Breno Leitao
  2026-08-07 16:29     ` Breno Leitao
  1 sibling, 1 reply; 6+ messages in thread
From: Breno Leitao @ 2026-08-07 14:58 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Catalin Marinas, Will Deacon, Peter Zijlstra (Intel), Jinjie Ruan,
	linux-arm-kernel, linux-kernel, bpf, rmikey, kernel-team,
	Vladimir Murzin, Ada Couprie Diaz

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RFC] arm64: entry: PSTATE_I_SET is leaking on pseudo NMI mode
  2026-08-07 14:58   ` Breno Leitao
@ 2026-08-07 16:29     ` Breno Leitao
  0 siblings, 0 replies; 6+ messages in thread
From: Breno Leitao @ 2026-08-07 16:29 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Catalin Marinas, Will Deacon, Peter Zijlstra (Intel), Jinjie Ruan,
	linux-arm-kernel, linux-kernel, bpf, rmikey, kernel-team,
	Vladimir Murzin, Ada Couprie Diaz

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

	bash-5.1# dmesg
	[   54.997498] ------------[ cut here ]------------
	[   54.997506] WARNING: ./arch/arm64/include/asm/irqflags.h:63 at arm64_exit_to_kernel_mode+0xc0/0xc8, CPU#1: bash/185
	[   54.997530] Modules linked in:
	[   54.997535] CPU: 1 UID: 0 PID: 185 Comm: bash Not tainted 7.2.0-rc6-next-20260806upstream #15 PREEMPT(full)
	[   54.997538] Hardware name: linux,dummy-virt (DT)
	[   54.997540] pstate: 234003c5 (nzCv DAIF +PAN -UAO +TCO +DIT -SSBS BTYPE=--)
	[   54.997542] pc : arm64_exit_to_kernel_mode+0xc0/0xc8
	[   54.997544] lr : el1_abort+0x5c/0x80
	[   54.997547] sp : ffff800088ba35f0
	[   54.997548] pmr: 000000f0
	[   54.997550] x29: ffff800088ba35f0 x28: ffff0000c3cd0000 x27: 0000000000000000
	[   54.997554] x26: ffff0000c3f277d0 x25: 0000000000000030 x24: 0000000000000001
	[   54.997557] x23: 00000000834003c5 x22: 0000000000000000 x21: 0000000000000001
	[   54.997566] x20: ffff800088ba3650 x19: 0000000000000000 x18: 0000000000000000
	[   54.997569] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000001
	[   54.997572] x14: 0000000040000000 x13: ffff800086b171f8 x12: fff080007fe00000
	[   54.997576] x11: 0000000000000001 x10: aaaaaaaaaaaaaaab x9 : 000000000000001f
	[   54.997579] x8 : 00000000000000d0 x7 : 0000000000000000 x6 : ffff0000c0adf02c
	[   54.997582] x5 : ffff0000c0adf034 x4 : ffff800081408b08 x3 : 000000000000000c
	[   54.997585] x2 : 00000000000002fe x1 : 0000000000000000 x0 : ffff800088ba3650
	[   54.997589] Call trace:
	[   54.997590]  arm64_exit_to_kernel_mode+0xc0/0xc8 (P)
	[   54.997593]  el1_abort+0x5c/0x80
	[   54.997595]  el1h_64_sync_handler+0x50/0x100
	[   54.997597]  el1h_64_sync+0x80/0x88
	[   54.997601]  __arch_copy_from_user+0x220/0x240 (P)
	[   54.997604]  process_fetch_insn+0x450/0x8b0
	[   54.997608]  kretprobe_trace_func+0x1b0/0x298
	[   54.997611]  kretprobe_dispatcher+0x5c/0x88
	[   54.997613]  __kretprobe_trampoline_handler+0xc4/0x168
	[   54.997617]  kretprobe_brk_handler+0x40/0x68
	[   54.997620]  call_el1_break_hook+0x74/0xa0
	[   54.997623]  do_el1_brk64+0x30/0x60
	[   54.997624]  el1_brk64+0x2c/0x48
	[   54.997626]  el1h_64_sync_handler+0x80/0x100
	[   54.997628]  el1h_64_sync+0x80/0x88
	[   54.997630]  ksys_read+0x80/0x100 (P)
	[   54.997634]  __arm64_sys_read+0x28/0x40
	[   54.997636]  invoke_syscall+0x54/0xf0
	[   54.997640]  do_el0_svc+0x7c/0xb8
	[   54.997642]  el0_svc+0x60/0x188
	[   54.997644]  el0t_64_sync_handler+0x84/0x130
	[   54.997645]  el0t_64_sync+0x1ac/0x1b0
	[   54.997647] ---[ end trace 0000000000000000 ]---


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-08-07 16:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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-07 14:58   ` Breno Leitao
2026-08-07 16:29     ` Breno Leitao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox