linux-kernel - Re: [PATCH v4 09/10] thermal/drivers/ti-soc-thermal: Use scope-based cleanup helper

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a04f3aa8-e7e3-4d20-bc86-5496c7abe272@kernel.org>
Date: Fri, 5 Sep 2025 09:02:09 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Andreas Kemnade <andreas@...nade.info>,
 Zihuan Zhang <zhangzihuan@...inos.cn>
Cc: "Rafael J . wysocki" <rafael@...nel.org>,
 Viresh Kumar <viresh.kumar@...aro.org>,
 Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
 Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
 Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
 Michael Ellerman <mpe@...erman.id.au>, Alim Akhtar
 <alim.akhtar@...sung.com>, Thierry Reding <thierry.reding@...il.com>,
 MyungJoo Ham <myungjoo.ham@...sung.com>,
 Kyungmin Park <kyungmin.park@...sung.com>,
 Chanwoo Choi <cw00.choi@...sung.com>,
 Jani Nikula <jani.nikula@...ux.intel.com>,
 Rodrigo Vivi <rodrigo.vivi@...el.com>, Tvrtko Ursulin
 <tursulin@...ulin.net>, David Airlie <airlied@...il.com>,
 Simona Vetter <simona@...ll.ch>, Daniel Lezcano <daniel.lezcano@...nel.org>,
 Sascha Hauer <s.hauer@...gutronix.de>, Shawn Guo <shawnguo@...nel.org>,
 Eduardo Valentin <edubezval@...il.com>, Keerthy <j-keerthy@...com>,
 Ben Horgan <ben.horgan@....com>, zhenglifeng <zhenglifeng1@...wei.com>,
 Zhang Rui <rui.zhang@...el.com>, Len Brown <lenb@...nel.org>,
 Lukasz Luba <lukasz.luba@....com>,
 Pengutronix Kernel Team <kernel@...gutronix.de>,
 Beata Michalska <beata.michalska@....com>, Fabio Estevam
 <festevam@...il.com>, Pavel Machek <pavel@...nel.org>,
 Sumit Gupta <sumitg@...dia.com>,
 Prasanna Kumar T S M <ptsm@...ux.microsoft.com>,
 Sudeep Holla <sudeep.holla@....com>, Yicong Yang <yangyicong@...ilicon.com>,
 linux-pm@...r.kernel.org, linux-acpi@...r.kernel.org,
 linuxppc-dev@...ts.ozlabs.org, linux-arm-kernel@...ts.infradead.org,
 intel-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
 imx@...ts.linux.dev, linux-omap@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 09/10] thermal/drivers/ti-soc-thermal: Use scope-based
 cleanup helper

On 05/09/2025 08:57, Andreas Kemnade wrote:
> Am Wed,  3 Sep 2025 21:17:32 +0800
> schrieb Zihuan Zhang <zhangzihuan@...inos.cn>:
> 
>> Replace the manual cpufreq_cpu_put() with __free(put_cpufreq_policy)
>> annotation for policy references. This reduces the risk of reference
>> counting mistakes and aligns the code with the latest kernel style.
>>
>> No functional change intended.
>>
>> Signed-off-by: Zihuan Zhang <zhangzihuan@...inos.cn>
>> ---
>>  drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 13 ++++---------
>>  1 file changed, 4 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
>> index 0cf0826b805a..37d06468913a 100644
>> --- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
>> +++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
>> @@ -27,7 +27,6 @@
>>  
>>  /* common data structures */
>>  struct ti_thermal_data {
>> -	struct cpufreq_policy *policy;
>>  	struct thermal_zone_device *ti_thermal;
>>  	struct thermal_zone_device *pcb_tz;
>>  	struct thermal_cooling_device *cool_dev;
>> @@ -218,6 +217,7 @@ int ti_thermal_register_cpu_cooling(struct ti_bandgap *bgp, int id)
>>  {
>>  	struct ti_thermal_data *data;
>>  	struct device_node *np = bgp->dev->of_node;
>> +	struct cpufreq_policy *policy __free(put_cpufreq_policy) = cpufreq_cpu_get(0);
>>  
> this looks as it changes the lifecycle from the device lifetime to just
> this function...
> 
>>  	/*
>>  	 * We are assuming here that if one deploys the zone
>> @@ -234,19 +234,17 @@ int ti_thermal_register_cpu_cooling(struct ti_bandgap *bgp, int id)
>>  	if (!data)
>>  		return -EINVAL;
>>  
>> -	data->policy = cpufreq_cpu_get(0);
>> -	if (!data->policy) {
>> +	if (!policy) {
>>  		pr_debug("%s: CPUFreq policy not found\n", __func__);
>>  		return -EPROBE_DEFER;
>>  	}
>>  
>>  	/* Register cooling device */
>> -	data->cool_dev = cpufreq_cooling_register(data->policy);
>> +	data->cool_dev = cpufreq_cooling_register(policy);
> 
> and it is passed on to something living beyond this function. I see no
> _get(policy) in cpufreq_cooling_register().
> Am I missing something?
Yeah, " No functional change intended." is clearly incorrect.

I already commented on this series that it is very bad and author should
does not understand how cleanup.h works, and here is another example.

IMO, entire patchset should be dropped.

Best regards,
Krzysztof