Re: [PATCH 1/2] cpufreq: mediatek: Enter system sleep at the suspend OPP

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] opp: UAF and data race on opp_table->suspend_opp
--

commit e45cc103dadbac91ac1b6c64b8b61a195f1c5f95
Author: Ryan Brue <ryanbrue.dev@gmail.com>

cpufreq: mediatek: Enter system sleep at the suspend OPP

This commit updates the mediatek-cpufreq driver to use the generic
suspend-frequency support. It allows the driver to enter system sleep at the
suspend OPP, reducing power consumption during suspend-to-RAM.

> diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c
> index 052ca7cd2f4fd..f9b1eca176eb8 100644
> --- a/drivers/cpufreq/mediatek-cpufreq.c
> +++ b/drivers/cpufreq/mediatek-cpufreq.c
> @@ -603,6 +603,7 @@ static int mtk_cpufreq_init(struct cpufreq_policy *policy)
>  	policy->freq_table = freq_table;
>  	policy->driver_data = info;
>  	policy->clk = info->cpu_clk;
> +	policy->suspend_freq = dev_pm_opp_get_suspend_opp_freq(info->cpu_dev) / 1000;

[Severity: High]
Is it safe to call dev_pm_opp_get_suspend_opp_freq() here?

In drivers/opp/core.c, dev_pm_opp_get_suspend_opp_freq() locklessly
dereferences opp_table->suspend_opp:

	if (opp_table->suspend_opp && opp_table->suspend_opp->available)
		freq = dev_pm_opp_get_freq(opp_table->suspend_opp);

However, in drivers/opp/of.c, _opp_add_static_v2() modifies
opp_table->suspend_opp dynamically without holding opp_table->lock, creating a
data race:

				opp_table->suspend_opp->suspend = false;
				new_opp->suspend = true;
				opp_table->suspend_opp = new_opp;

Furthermore, when an OPP is removed, _opp_kref_release() in drivers/opp/core.c
frees the dev_pm_opp object without checking or clearing
opp_table->suspend_opp:

	_of_clear_opp(opp_table, opp);
	opp_debug_remove_one(opp);
	kfree(opp);

This leaves a permanent dangling pointer if opp_table->suspend_opp pointed to
the dying OPP. Any subsequent call to dev_pm_opp_get_suspend_opp_freq()
will locklessly dereference the now-dangling pointer, potentially causing a
use-after-free.

>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-rbrue-suez-upstreaming-mtk-cpufreq-suspend-opp-v1-0-9100f79b4080@gmail.com?part=1