Re: [PATCH] cpufreq: acpi-cpufreq: Using cpufreq_for_each_entry() to iterate in extract_io()

Re: [PATCH] cpufreq: acpi-cpufreq: Using cpufreq_for_each_entry() to iterate in extract_io()

[Zhongqiu Han]

On 8/10/2026 2:10 PM, lirongqing wrote:
> From: Li RongQing <lirongqing@baidu.com>
> 
> In extract_io(), the loop iterates up to perf->state_count. However,
> when building policy->freq_table in acpi_cpufreq_cpu_init(), duplicate
> frequency entries are skipped, making freq_table smaller than
> perf->state_count.
> 
> Iterating perf->state_count times directly over policy->freq_table[i] can
> result in out-of-bounds array reads. Furthermore, policy->freq_table[i]

There is no out-of-bounds access. The array has "state_count + 1"
elements, so every index in "[0, state_count)" is inside the allocation.
right? The changelog looks like a memory-safety fix, which it is not,
and that wording alone would get the patch (mis)routed to stable and to
CVE bots.

> does not necessarily correspond to perf->states[i], as the original P-state
> index is stored in freq_table[entry].driver_data.

Yes, the real defect is the index space mismatch.

Might be good to note the side effects of it, AFAICT

freq_table[i] is not perf->states[i] once any _PSS entry has been
skipped. The function can therefore return a frequency belonging to a
different P-state, or 0 (zeroed tail entries), or CPUFREQ_TABLE_END
(~1u) when "i == valid_states", i.e. 0xfffffffe kHz reported
as a frequency. That last one is worth spelling out.

> 
> Fix this by using cpufreq_for_each_entry() to iterate over
> policy->freq_table, similar to extract_msr().
> 

Please add one Fixes tag here as well.

> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
>   drivers/cpufreq/acpi-cpufreq.c | 9 ++++-----
>   1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> index 21639d9..87e4923 100644
> --- a/drivers/cpufreq/acpi-cpufreq.c
> +++ b/drivers/cpufreq/acpi-cpufreq.c
> @@ -196,15 +196,14 @@ static int check_amd_hwpstate_cpu(unsigned int cpuid)
>   static unsigned extract_io(struct cpufreq_policy *policy, u32 value)
>   {
>   	struct acpi_cpufreq_data *data = policy->driver_data;
> +	struct cpufreq_frequency_table *pos;
>   	struct acpi_processor_performance *perf;
> -	int i;
>   
>   	perf = to_perf_data(data);
>   
> -	for (i = 0; i < perf->state_count; i++) {
> -		if (value == perf->states[i].status)
> -			return policy->freq_table[i].frequency;
> -	}
> +	cpufreq_for_each_entry(pos, policy->freq_table)
> +		if (value == perf->states[pos->driver_data].status)
> +			return pos->frequency;
>   	return 0;
>   }

One more potential same issue is in func get_cur_freq_on_cpu()

cached_freq = policy->freq_table[to_perf_data(data)->state].frequency;

It is better to fix it as well.

For v2, please use ./scripts/get_maintainer.pl to generate the CC list
so linux-kernel@vger.kernel.org doesn't get missed. Thanks


>   


-- 
Thx and BRs,
Zhongqiu Han

答复: [外部邮件] Re: [PATCH] cpufreq: acpi-cpufreq: Using cpufreq_for_each_entry() to iterate in extract_io()

[Li,Rongqing]

> > From: Li RongQing <lirongqing@baidu.com>
> >
> > In extract_io(), the loop iterates up to perf->state_count. However,
> > when building policy->freq_table in acpi_cpufreq_cpu_init(), duplicate
> > frequency entries are skipped, making freq_table smaller than
> > perf->state_count.
> >
> > Iterating perf->state_count times directly over policy->freq_table[i]
> > can result in out-of-bounds array reads. Furthermore,
> > policy->freq_table[i]
> 
> There is no out-of-bounds access. The array has "state_count + 1"
> elements, so every index in "[0, state_count)" is inside the allocation.
> right? The changelog looks like a memory-safety fix, which it is not, and that
> wording alone would get the patch (mis)routed to stable and to CVE bots.
> 
> > does not necessarily correspond to perf->states[i], as the original
> > P-state index is stored in freq_table[entry].driver_data.
> 
> Yes, the real defect is the index space mismatch.
> 
> Might be good to note the side effects of it, AFAICT
> 
> freq_table[i] is not perf->states[i] once any _PSS entry has been skipped. The
> function can therefore return a frequency belonging to a different P-state, or
> 0 (zeroed tail entries), or CPUFREQ_TABLE_END
> (~1u) when "i == valid_states", i.e. 0xfffffffe kHz reported as a frequency. That
> last one is worth spelling out.
> 
> >
> > Fix this by using cpufreq_for_each_entry() to iterate over
> > policy->freq_table, similar to extract_msr().
> >
> 
> Please add one Fixes tag here as well.
> 
> > Signed-off-by: Li RongQing <lirongqing@baidu.com>
> > ---
> >   drivers/cpufreq/acpi-cpufreq.c | 9 ++++-----
> >   1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/cpufreq/acpi-cpufreq.c
> > b/drivers/cpufreq/acpi-cpufreq.c index 21639d9..87e4923 100644
> > --- a/drivers/cpufreq/acpi-cpufreq.c
> > +++ b/drivers/cpufreq/acpi-cpufreq.c
> > @@ -196,15 +196,14 @@ static int check_amd_hwpstate_cpu(unsigned int
> cpuid)
> >   static unsigned extract_io(struct cpufreq_policy *policy, u32 value)
> >   {
> >   	struct acpi_cpufreq_data *data = policy->driver_data;
> > +	struct cpufreq_frequency_table *pos;
> >   	struct acpi_processor_performance *perf;
> > -	int i;
> >
> >   	perf = to_perf_data(data);
> >
> > -	for (i = 0; i < perf->state_count; i++) {
> > -		if (value == perf->states[i].status)
> > -			return policy->freq_table[i].frequency;
> > -	}
> > +	cpufreq_for_each_entry(pos, policy->freq_table)
> > +		if (value == perf->states[pos->driver_data].status)
> > +			return pos->frequency;
> >   	return 0;
> >   }
> 
> One more potential same issue is in func get_cur_freq_on_cpu()
> 
> cached_freq = policy->freq_table[to_perf_data(data)->state].frequency;
> 
> It is better to fix it as well.
> 
> For v2, please use ./scripts/get_maintainer.pl to generate the CC list so
> linux-kernel@vger.kernel.org doesn't get missed. Thanks
> 

I will send v2，thanks

[Li,Rongqing] 


> 
> >
> 
> 
> --
> Thx and BRs,
> Zhongqiu Han