Re: [PATCH 12/12] s390/percpu: Rework to simplify percpu_entry() and percpu_exit()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] s390/percpu: unconditional register fixup in percpu_exit() corrupts GPRs on NMI
--

commit 9424e4df4698f4a6dd024548cb15a93aae1331e4
Author: Heiko Carstens <hca@linux.ibm.com>

s390/percpu: Rework to simplify percpu_entry() and percpu_exit()

This commit changes s390 to implement a new approach for percpu code
sections similar to arm64, removing the need to check instruction opcodes
during interrupts. It encodes three register numbers into the
percpu_register field and uses a register pair to work around the
single-byte limit of the mviy instruction.

Link: https://lore.kernel.org/all/20260908151741.394589-1-mark.rutland@arm.com/

> diff --git a/arch/s390/include/asm/entry-percpu.h b/arch/s390/include/asm/entry-percpu.h
> index 89c9e44f6f833..7ec5d976db471 100644
> --- a/arch/s390/include/asm/entry-percpu.h
> +++ b/arch/s390/include/asm/entry-percpu.h

[ ... ]

> -static __always_inline void percpu_exit(struct pt_regs *regs, bool needs_fixup)
> +static __always_inline void percpu_exit(struct pt_regs *regs)
>  {
> +	unsigned char regval, regpcp, regoff, regptr;
>  	struct lowcore *lc = get_lowcore();
> -	unsigned char reg;
>  
> -	if (user_mode(regs))
> -		return;
> -	reg = regs->percpu_register;
> -	lc->percpu_register = reg;
> -	if (likely(!needs_fixup))
> -		return;
> -	/* Check if process has been migrated to a different CPU. */
> -	if (regs->cpu == lc->cpu_nr)
> +	if (!regs->percpu_register)
>  		return;
> -	/* Fixup percpu base register */
> -	regs->gprs[reg] -= __per_cpu_offset[regs->cpu];
> -	regs->gprs[reg] += lc->percpu_offset;
> +	regval = regs->percpu_register;
> +	lc->percpu_register = regval;
> +	regpcp = FIELD_GET(PCPU_REG_PCP, regval);
> +	regoff = FIELD_GET(PCPU_REG_OFF, regval);
> +	regptr = regoff + 1;
> +
> +	/*
> +	 * Update register 'regoff' which contains the current CPU's percpu
> +	 * offset, and recalculate and update current CPU's percpu variable
> +	 * address contained in register 'regptr'.
> +	 */
> +	regs->gprs[regoff] = lc->percpu_offset;
> +	regs->gprs[regptr] = regs->gprs[regpcp] + regs->gprs[regoff];

[Severity: High]
Does this unconditional fixup corrupt GPRs if an NMI interrupts the kernel
return path?

If a standard interrupt occurs during a percpu code section, the handler's
percpu_exit() restores the non-zero lc->percpu_register right before
returning to assembly. During the subsequent assembly return sequence (e.g.,
in .Lsysc_restore), lc->percpu_register remains non-zero.

If a non-migrating exception (such as a Machine Check NMI) hits during this
precise window, s390_do_machine_check() will save the return sequence's
current registers into pt_regs, call percpu_entry(), and then percpu_exit().

Because this patch removed the migration check (regs->cpu == lc->cpu_nr),
percpu_exit() now unconditionally overwrites the regoff and regptr GPRs.
Since the compiler's "=&d" constraint can allocate register pairs heavily
used by the return sequence (like %r10/%r11 where %r11 points to pt_regs,
or %r14/%r15 where %r15 is the stack pointer), the NMI will clobber them.

When the NMI returns, the interrupted return sequence continues with
corrupted GPRs, potentially causing fatal faults on instructions like
lmg %r0,%r15,__PT_R0(%r11).

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917192237.638324-1-hca@linux.ibm.com?part=12