Re: [PATCH v7 4/6] soc: samsung: exynos-pmu: add Exynos850 CPU hotplug support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] syscon: use-after-free of devm regmap on probe failure or driver unbind
- [High] soc: samsung: hardware registers incorrectly indexed by logical CPU ID
--

commit afd8000549f41385d9df745be4acc40a4d6c283b
Author: Alexey Klimov <alexey.klimov@linaro.org>

soc: samsung: exynos-pmu: add Exynos850 CPU hotplug support

This commit adds cpuhotplug support for Exynos850 platforms by introducing
SoC-specific PMU and PMU interrupt writes/updates. It also initializes a
syscon regmap with raw spinlocks in the main probe routine to handle these
states safely.

> diff --git a/drivers/soc/samsung/exynos-pmu.c b/drivers/soc/samsung/exynos-pmu.c
> --- a/drivers/soc/samsung/exynos-pmu.c
> +++ b/drivers/soc/samsung/exynos-pmu.c
[ ... ]
> @@ -480,6 +489,21 @@ static int exynos_pmu_probe(struct platform_device *pdev)
>  		ret = of_syscon_register_regmap(dev->of_node, regmap);
>  		if (ret)
>  			return ret;
> +	/*
> +	 * For SoCs that support cpuhotplug/cpuidle via PMU updates callbacks.
> +	 * Such callbacks are executed under raw_spinlock so we need a custom
> +	 * regmap too.
> +	 */
> +	} else if (pmu_context->pmu_data && pmu_context->pmu_data->pmu_cpuhp) {
> +		regmap = devm_regmap_init_mmio(dev, pmu_base_addr, &regmap_pmu);
> +		if (IS_ERR(regmap))
> +			return dev_err_probe(dev, PTR_ERR(regmap),
> +					     "hotplug regmap init failed\n");
> +
> +		ret = of_syscon_register_regmap(dev->of_node, regmap);

[Severity: High]
Does this sequence create a use-after-free if probe fails later or the driver
unbinds? The driver allocates a devm-managed regmap via
devm_regmap_init_mmio() and permanently registers it with the global syscon
list via of_syscon_register_regmap().

If probe fails later, for instance when calling devm_mfd_add_devices(),
devres will automatically free the regmap. However, syscon does not support
unregistering, so the syscon list will retain a dangling pointer to the
freed regmap. Any subsequent driver requesting this syscon node by phandle
could dereference freed memory.

> diff --git a/drivers/soc/samsung/exynos850-pmu.c b/drivers/soc/samsung/exynos850-pmu.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/soc/samsung/exynos850-pmu.c
[ ... ]
> +static int exynos850_cpu_pmu_offline(struct exynos_pmu_context *pmu_context, unsigned int cpu)
> +	__must_hold(&pmu_context->cpupm_lock)
> +{
> +	int cluster_id, core_id;
> +	u32 reg, mask;
> +
> +	cluster_id = topology_cluster_id(cpu);
> +	if (cluster_id < 0) {
> +		pr_err_ratelimited("invalid cluster ID for cpu: %u\n", cpu);
> +		return -EINVAL;
> +	}
> +
> +	core_id = topology_core_id(cpu);
> +	if (core_id < 0) {
> +		pr_err_ratelimited("invalid core ID for cpu: %u\n", cpu);
> +		return -EINVAL;
> +	}
> +
> +	/* set cpu inform hint */
> +	regmap_write(pmu_context->pmureg, EXYNOS850_CPU_INFORM(cpu), CPU_INFORM_C2);
> +
> +	mask = BIT(cpu);

[Severity: High]
Are we passing a logical Linux CPU ID directly into hardware-facing macros
and bitmask generators here? Logical CPU IDs are not guaranteed to match
physical hardware core indices, especially when booting with maxcpus, during
kdump, or with sparse topologies.

If the logical and physical IDs diverge, the bit shifts and register accesses
will target the wrong CPU's hardware registers. If cpu is 32 or higher,
BIT(cpu) assigned to a 32-bit mask could invoke undefined behavior or
truncate.

> diff --git a/include/linux/soc/samsung/exynos-regs-pmu.h b/include/linux/soc/samsung/exynos-regs-pmu.h
> --- a/include/linux/soc/samsung/exynos-regs-pmu.h
> +++ b/include/linux/soc/samsung/exynos-regs-pmu.h
[ ... ]
> @@ -1015,6 +1015,11 @@
>  #define EXYNOS_GRP2_INTR_BID_UPEND				(0x0208)
>  #define EXYNOS_GRP2_INTR_BID_CLEAR				(0x020c)
>  
> +/* Exynos850 PMU Alive */
> +#define EXYNOS850_CPU_INFORM(cpu)		(0x0860 + ((cpu) & 7) * 4)

[Severity: High]
Can this macro cause register clashing if the logical CPU ID exceeds 7? The
macro applies a hardcoded masking based on the assumption that it receives
a flat 0-7 physical index. If cpu is a logical ID, this assumption might
not hold.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930-exynos850-cpuhotplug-v7-0-203245826333@linaro.org?part=4