Re: [PATCH v2 3/6] powerpc/xive: fix use-after-free of xive_ipis

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete fix: NULL pointer dereference on secondary CPU bring-up.
- [High] Missing error propagation from xive_setup_cpu_ipi().
--

--- Patch [3]: [PATCH v2 3/6] powerpc/xive: fix use-after-free of xive_ipis ---
commit da9c3cedc79737a7541cbdd8b690abe6862a200e
Author: Gou Hao <gouhao@uniontech.com>

powerpc/xive: fix use-after-free of xive_ipis

This commit propagates the error from xive_init_ipis() through
xive_smp_probe() to pnv_smp_probe() and pSeries_smp_probe(). It allows
the boot sequence to cleanly abort XIVE IPI setup if the initial allocation
fails.

> diff --git a/arch/powerpc/platforms/powernv/smp.c b/arch/powerpc/platforms/powernv/smp.c
> index 8f41ef364fc6f..b1201dbafcaf6 100644
> --- a/arch/powerpc/platforms/powernv/smp.c
> +++ b/arch/powerpc/platforms/powernv/smp.c
> @@ -332,10 +332,12 @@ static void pnv_cause_ipi(int cpu)
>  
>  static void __init pnv_smp_probe(void)
>  {
> -	if (xive_enabled())
> -		xive_smp_probe();
> -	else
> +	if (xive_enabled()) {
> +		if (xive_smp_probe() < 0)
> +			return;

[Severity: High]
This is a pre-existing issue, but does this early return leave XIVE
globally enabled if initialization fails?

Later during boot, SMP bring-up calls xive_smp_prepare_cpu() for secondary
CPUs because xive_enabled() remains true.

This unconditionally invokes xive_setup_cpu_ipi(), which calls
xive_ipi_cpu_to_irq() and dereferences the xive_ipis array pointer:

arch/powerpc/sysdev/xive/common.c:xive_ipi_cpu_to_irq()
    ...
    return xive_ipis[early_cpu_to_node(cpu)].irq;
    ...

Since xive_init_ipis() frees xive_ipis and sets it to NULL on failure,
could this cause a NULL pointer dereference on secondary CPU bring-up?

> +	} else {
>  		xics_smp_probe();
> +	}

[ ... ]

> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
> index f9a1229cede73..bb6ce07c1699e 100644
> --- a/arch/powerpc/sysdev/xive/common.c
> +++ b/arch/powerpc/sysdev/xive/common.c
> @@ -1256,10 +1256,14 @@ noinstr static void xive_cleanup_cpu_ipi(unsigned int cpu, struct xive_cpu *xc)
>  
>  int __init xive_smp_probe(void)
>  {
> +	int ret;
> +
>  	smp_ops->cause_ipi = xive_cause_ipi;
>  
>  	/* Register the IPI */
> -	xive_init_ipis();
> +	ret = xive_init_ipis();
> +	if (ret < 0)
> +		return ret;
>  
>  	/* Allocate and setup IPI for the boot CPU */
>  	xive_setup_cpu_ipi(smp_processor_id());

[Severity: High]
This is a pre-existing issue, but shouldn't the return value of
xive_setup_cpu_ipi() be checked and propagated here?

If hardware IPI allocation or configuration fails (e.g., returning -EIO),
the error code is ignored and xive_smp_probe() returns 0 to its caller.

Could this lead to the boot process continuing with uninitialized hardware
IPIs on the boot CPU, eventually resulting in a system hang due to missing
cross-CPU interrupts?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724022851.466017-1-gouhao@uniontech.com?part=3