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

[Ryan Brue <ryanbrue.dev@gmail.com>]

On 10/9/26 1:43 PM, sashiko-bot@kernel.org wrote:
> [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.
The suspend OPP is read once, from ->init, into policy->suspend_freq. 
mediatek-cpufreq adds the whole OPP table in probe before 
cpufreq_register_driver(), and never adds, removes, or disables 
individual OPPs, and has no .remove, so the table can't change or go 
away while ->init can run. cpufreq-dt makes the same call.

I also realized that I forgot my sign-off on the commits. I will send 
out a v2 that has the sign-off. My apologies.

Best regards,
Ryan Brue