Re: [PATCH] cpufreq: imx6q: fix devres accumulation across driver rebind

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

Pre-existing issues:
- [High] The file-scope static variable `soc_opp_count` is never reset to 0 across driver unbind/rebind or deferred probe cycles, leading to a severe out-of-bounds write array overflow.
--

--- Patch [1]: [PATCH] cpufreq: imx6q: fix devres accumulation across driver rebind ---
commit d8bcb5a2ed1157b0b23fd4c657a9fd392c54309b
Author: Karl Mehltretter <[email protected]>

cpufreq: imx6q: fix devres accumulation across driver rebind

This commit fixes a memory leak caused by devres accumulation during driver
unbind and rebind cycles. It changes the memory allocation of imx6_soc_volt
to tie its lifetime to the platform device, and clears the file-scope pointer
in the remove and probe-failure paths.

> diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
> index e93697d3edfd9..8110c95059e0e 100644
> --- a/drivers/cpufreq/imx6q-cpufreq.c
> +++ b/drivers/cpufreq/imx6q-cpufreq.c

[ ... ]

> @@ -506,6 +507,7 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev)
>  static void imx6q_cpufreq_remove(struct platform_device *pdev)
>  {
>  	cpufreq_unregister_driver(&imx6q_cpufreq_driver);
> +	imx6_soc_volt = NULL;

[Severity: High]
This is a pre-existing issue, but since this patch explicitly tests and 
fixes rebind behavior, should soc_opp_count also be reset to 0 here?

The file-scope static variable soc_opp_count is never reset to 0 across 
driver unbind/rebind or deferred probe cycles. 

In imx6q_cpufreq_probe(), the loop increments soc_opp_count:

	for (j = 0; j < num; j++) {
		val = prop->value;
		for (i = 0; i < nr / 2; i++) {
			unsigned long freq = be32_to_cpup(val++);
			unsigned long volt = be32_to_cpup(val++);
			if (freq_table[j].frequency == freq) {
				imx6_soc_volt[soc_opp_count++] = volt;
				break;
			}
		}
	}

On subsequent probes (e.g. after the unbind/rebind cycle you tested), it 
resumes counting from its previous value instead of 0. Since imx6_soc_volt 
is newly allocated with size 'num', this will write beyond the bounds of 
the array, causing memory corruption.

Does the driver need to reset soc_opp_count to 0 during removal and on the 
error paths in probe?

>  	dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table);
>  	dev_pm_opp_of_remove_table(cpu_dev);
>  	regulator_put(arm_reg);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1