linux-kernel - Re: [PATCH v4 03/10] cpufreq: intel_pstate: 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: <CAJZ5v0gN1T5woSF0tO=TbAh+2-sWzxFjWyDbB7wG2TFCOU01iQ@mail.gmail.com>
Date: Fri, 5 Sep 2025 11:48:22 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: 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>, 
	Krzysztof Kozlowski <krzk@...nel.org>, 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 03/10] cpufreq: intel_pstate: Use scope-based cleanup helper

On Wed, Sep 3, 2025 at 3:18 PM Zihuan Zhang <zhangzihuan@...inos.cn> wrote:
>
> 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/cpufreq/intel_pstate.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index f366d35c5840..925efb1e65be 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -1209,6 +1209,17 @@ static bool hybrid_clear_max_perf_cpu(void)
>         return ret;
>  }
>
> +static struct freq_qos_request *intel_pstate_cpufreq_get_req(int cpu)
> +{
> +       struct cpufreq_policy *policy __free(put_cpufreq_policy) =
> +               cpufreq_cpu_get(cpu);
> +
> +       if (!policy)
> +               return NULL;
> +
> +       return policy->driver_data;
> +}
> +
>  static void __intel_pstate_get_hwp_cap(struct cpudata *cpu)
>  {
>         u64 cap;
> @@ -1698,19 +1709,13 @@ static ssize_t store_no_turbo(struct kobject *a, struct kobj_attribute *b,
>  static void update_qos_request(enum freq_qos_req_type type)
>  {
>         struct freq_qos_request *req;
> -       struct cpufreq_policy *policy;
>         int i;
>
>         for_each_possible_cpu(i) {
>                 struct cpudata *cpu = all_cpu_data[i];
>                 unsigned int freq, perf_pct;
>
> -               policy = cpufreq_cpu_get(i);
> -               if (!policy)
> -                       continue;
> -
> -               req = policy->driver_data;
> -               cpufreq_cpu_put(policy);
> +               req = intel_pstate_cpufreq_get_req(i);
>
>                 if (!req)
>                         continue;
> --

This change would have been fine if the modified code had been
correct.  Unfortunately, this is not the case because below req is
passed to freq_qos_update_request() which accesses the policy object
indirectly through it, so the policy object should be
reference-counted until freq_qos_update_request() completes.

I will send a fix for this shortly (it is better to fix it before
rearranging the code to allow the fix to be backported more easily)
along with my version of the change in question.

Thanks!