Re: [PATCH 12/17] arm64/nmi: Manage masking for superpriority interrupts along with DAIF

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Kprobes IRQ flag save/restore drops ALLINT state, inadvertently unmasking NMIs upon return.
- [High] `exception_exit_restore_mask` fails to restore the `allint` field, leaving NMIs unmasked during critical exception exit.
- [Medium] Returning `struct exception_mask` by value copies uninitialized memory when `system_uses_nmi()` is false.
--

commit 0abc1ecd8853e1c8db5ff01b3c4dad448c23b517
Author: Mark Brown <broonie@kernel.org>

arm64/nmi: Manage masking for superpriority interrupts along with DAIF

This patch updates DAIF management routines to also mask and unmask 
superpriority interrupts (NMIs) when asynchronous exceptions are handled. 
It modifies both inline C functions and assembly macros to track and 
restore the ALLINT state.

Link: https://lore.kernel.org/r/20221112151708.175147-11-broonie@kernel.org

> diff --git a/arch/arm64/include/asm/exception_masks.h b/arch/arm64/include/asm/exception_masks.h
> index fbbba769ca031..3dd41998f705c 100644
> --- a/arch/arm64/include/asm/exception_masks.h
> +++ b/arch/arm64/include/asm/exception_masks.h
[ ... ]
> @@ -87,7 +94,8 @@ static inline void local_exception_save_mask(struct exception_mask *mask)
>  	if (system_uses_irq_prio_masking())
>  		mask->pmr = gic_read_pmr();
>  
> -	mask->allint = 0;
> +	if (system_uses_nmi())
> +		mask->allint = read_sysreg_s(SYS_ALLINT);
>  }

[Severity: Medium]
Does this leave mask->allint uninitialized on systems where system_uses_nmi()
is false?

When local_exception_save_and_mask() allocates mask on the stack and returns 
it by value, returning the struct with an uninitialized field could copy 
uninitialized memory and trigger KMSAN warnings.

[ ... ]
> @@ -118,6 +126,9 @@ static inline void local_exception_restore(const struct exception_mask mask)
>  		pmr_sync();
>  	}
>  
> +	if (system_uses_nmi())
> +		write_sysreg_s(mask.allint, SYS_ALLINT);
> +
>  	write_sysreg(mask.daif, daif);

[Severity: High]
Does exception_exit_restore_mask() also need to be updated to restore the 
allint field?

Unlike local_exception_restore(), it appears exception_exit_restore_mask() was
not updated to restore SYS_ALLINT, which might leave NMIs unmasked during 
exception exit paths when returning to user or kernel mode.

[ ... ]
> diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> index 70d311e415ac9..8045245ebbdde 100644
> --- a/arch/arm64/kernel/probes/kprobes.c
> +++ b/arch/arm64/kernel/probes/kprobes.c
> @@ -197,12 +197,19 @@ static void __kprobes kprobes_save_local_irqflag(struct kprobe_ctlblk *kcb,
>  {
>  	kcb->saved_irqflag = regs->pstate & DAIF_MASK;
>  	regs->pstate |= DAIF_MASK;
> +
> +	if (system_uses_nmi())
> +		regs->pstate |= ALLINT_ALLINT;
>  }
>  
>  static void __kprobes kprobes_restore_local_irqflag(struct kprobe_ctlblk *kcb,
>  						struct pt_regs *regs)
>  {
>  	regs->pstate &= ~DAIF_MASK;
> +
> +	if (system_uses_nmi())
> +		regs->pstate &= ~ALLINT_ALLINT;
> +
>  	regs->pstate |= kcb->saved_irqflag;

[Severity: High]
Does this sequence inadvertently drop the ALLINT state and permanently unmask 
NMIs upon returning from the kprobe? 

In kprobes_save_local_irqflag(), DAIF_MASK does not include ALLINT_ALLINT, so 
the original NMI mask state is not saved in kcb->saved_irqflag. When 
kprobes_restore_local_irqflag() later clears ALLINT_ALLINT and restores 
saved_irqflag, any previously masked NMIs could become unexpectedly unmasked.

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703100135.2512312-1-ruanjinjie@huawei.com?part=12