[PATCH] cpufreq: acpi-cpufreq: add NULL check for acpi_perf_data before freeing

[PATCH] cpufreq: acpi-cpufreq: add NULL check for acpi_perf_data before freeing

[lirongqing]

From: Li RongQing <lirongqing@baidu.com>

If alloc_percpu() fails in acpi_cpufreq_early_init(), acpi_perf_data is
NULL.  Calling free_acpi_perf_data() will execute per_cpu_ptr(NULL, i),
which does not return NULL but an offset pointer.
Dereferencing ->shared_cpu_map on this invalid pointer leads to a kernel
crash.

Fix these issues by adding a NULL check at the start of
free_acpi_perf_data() and setting acpi_perf_data to NULL after freeing.

Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 drivers/cpufreq/acpi-cpufreq.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index 87e4923..cd2ca87 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -517,11 +517,14 @@ static void free_acpi_perf_data(void)
 {
 	unsigned int i;
 
-	/* Freeing a NULL pointer is OK, and alloc_percpu zeroes. */
+	if (!acpi_perf_data)
+		return;
+
 	for_each_possible_cpu(i)
 		free_cpumask_var(per_cpu_ptr(acpi_perf_data, i)
 				 ->shared_cpu_map);
 	free_percpu(acpi_perf_data);
+	acpi_perf_data = NULL;
 }
 
 static int cpufreq_boost_down_prep(unsigned int cpu)
-- 
2.9.4

[PATCH] cpufreq: acpi-cpufreq: Initialize cmd.val in drv_read()

[lirongqing]

From: Li RongQing <lirongqing@baidu.com>

In drv_read(), struct drv_cmd cmd is allocated on the stack with .val left
uninitialized. If smp_call_function_any() fails, do_drv_read() will not
run to populate cmd.val. As a result, drv_read() would return uninitialized
stack data.

Fix this by explicitly initializing .val to 0 when declaring cmd.

Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 drivers/cpufreq/acpi-cpufreq.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index cd2ca87..6c7ece9 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -308,6 +308,7 @@ static u32 drv_read(struct acpi_cpufreq_data *data, const struct cpumask *mask)
 	struct acpi_processor_performance *perf = to_perf_data(data);
 	struct drv_cmd cmd = {
 		.reg = &perf->control_register,
+		.val = 0,
 		.func.read = data->cpu_freq_read,
 	};
 	int err;
-- 
2.9.4

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

[lirongqing]

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

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

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;
 }
 
-- 
2.9.4

Re: [PATCH] cpufreq: acpi-cpufreq: add NULL check for acpi_perf_data before freeing

[Zhongqiu Han]

On 8/10/2026 2:10 PM, lirongqing wrote:
> From: Li RongQing <lirongqing@baidu.com>
> 
> If alloc_percpu() fails in acpi_cpufreq_early_init(), acpi_perf_data is
> NULL.  Calling free_acpi_perf_data() will execute per_cpu_ptr(NULL, i),
> which does not return NULL but an offset pointer.

Hi RongQing,

Could you please confirm if acpi_perf_data() can ever be executed when 
alloc_percpu() fails?


> Dereferencing ->shared_cpu_map on this invalid pointer leads to a kernel
> crash.
> 
> Fix these issues by adding a NULL check at the start of
> free_acpi_perf_data() and setting acpi_perf_data to NULL after freeing.
> 
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
>   drivers/cpufreq/acpi-cpufreq.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> index 87e4923..cd2ca87 100644
> --- a/drivers/cpufreq/acpi-cpufreq.c
> +++ b/drivers/cpufreq/acpi-cpufreq.c
> @@ -517,11 +517,14 @@ static void free_acpi_perf_data(void)
>   {
>   	unsigned int i;
>   
> -	/* Freeing a NULL pointer is OK, and alloc_percpu zeroes. */
> +	if (!acpi_perf_data)
> +		return;
> +
>   	for_each_possible_cpu(i)
>   		free_cpumask_var(per_cpu_ptr(acpi_perf_data, i)
>   				 ->shared_cpu_map);
>   	free_percpu(acpi_perf_data);
> +	acpi_perf_data = NULL;
>   }
>   
>   static int cpufreq_boost_down_prep(unsigned int cpu)


-- 
Thx and BRs,
Zhongqiu Han

Re: [PATCH] cpufreq: acpi-cpufreq: add NULL check for acpi_perf_data before freeing

[Zhongqiu Han]

On 8/10/2026 7:21 PM, Zhongqiu Han wrote:
> On 8/10/2026 2:10 PM, lirongqing wrote:
>> From: Li RongQing <lirongqing@baidu.com>
>>
>> If alloc_percpu() fails in acpi_cpufreq_early_init(), acpi_perf_data is
>> NULL.  Calling free_acpi_perf_data() will execute per_cpu_ptr(NULL, i),
>> which does not return NULL but an offset pointer.
> 
> Hi RongQing,
> 
> Could you please confirm if acpi_perf_data() can ever be executed when 
> alloc_percpu() fails?

Sorry, typo: acpi_perf_data() --> free_acpi_perf_data()

> 
> 
>> Dereferencing ->shared_cpu_map on this invalid pointer leads to a kernel
>> crash.
>>
>> Fix these issues by adding a NULL check at the start of
>> free_acpi_perf_data() and setting acpi_perf_data to NULL after freeing.
>>
>> Signed-off-by: Li RongQing <lirongqing@baidu.com>
>> ---
>>   drivers/cpufreq/acpi-cpufreq.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi- 
>> cpufreq.c
>> index 87e4923..cd2ca87 100644
>> --- a/drivers/cpufreq/acpi-cpufreq.c
>> +++ b/drivers/cpufreq/acpi-cpufreq.c
>> @@ -517,11 +517,14 @@ static void free_acpi_perf_data(void)
>>   {
>>       unsigned int i;
>> -    /* Freeing a NULL pointer is OK, and alloc_percpu zeroes. */
>> +    if (!acpi_perf_data)
>> +        return;
>> +
>>       for_each_possible_cpu(i)
>>           free_cpumask_var(per_cpu_ptr(acpi_perf_data, i)
>>                    ->shared_cpu_map);
>>       free_percpu(acpi_perf_data);
>> +    acpi_perf_data = NULL;
>>   }
>>   static int cpufreq_boost_down_prep(unsigned int cpu)
> 
> 


-- 
Thx and BRs,
Zhongqiu Han

Re: [PATCH] cpufreq: acpi-cpufreq: Initialize cmd.val in drv_read()

[Zhongqiu Han]

On 8/10/2026 2:10 PM, lirongqing wrote:
> From: Li RongQing <lirongqing@baidu.com>
> 
> In drv_read(), struct drv_cmd cmd is allocated on the stack with .val left
> uninitialized. If smp_call_function_any() fails, do_drv_read() will not
> run to populate cmd.val. As a result, drv_read() would return uninitialized
> stack data.
> 
> Fix this by explicitly initializing .val to 0 when declaring cmd.
> 
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
>   drivers/cpufreq/acpi-cpufreq.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> index cd2ca87..6c7ece9 100644
> --- a/drivers/cpufreq/acpi-cpufreq.c
> +++ b/drivers/cpufreq/acpi-cpufreq.c
> @@ -308,6 +308,7 @@ static u32 drv_read(struct acpi_cpufreq_data *data, const struct cpumask *mask)
>   	struct acpi_processor_performance *perf = to_perf_data(data);
>   	struct drv_cmd cmd = {
>   		.reg = &perf->control_register,
> +		.val = 0,

Will the compiler handle this automatically?

https://en.cppreference.com/c/language/initialization


>   		.func.read = data->cpu_freq_read,
>   	};
>   	int err;


-- 
Thx and BRs,
Zhongqiu Han

答复: [外部邮件] Re: [PATCH] cpufreq: acpi-cpufreq: add NULL check for acpi_perf_data before freeing

[Li,Rongqing]

> >
> > If alloc_percpu() fails in acpi_cpufreq_early_init(), acpi_perf_data
> > is NULL.  Calling free_acpi_perf_data() will execute per_cpu_ptr(NULL,
> > i), which does not return NULL but an offset pointer.
> 
> Hi RongQing,
> 
> Could you please confirm if acpi_perf_data() can ever be executed when
> alloc_percpu() fails?

You are right, acpi_perf_data() can not be called when alloc_percpu() fails

Please drop this patch

thanks 

[Li,Rongqing] 



> 
> 
> > Dereferencing ->shared_cpu_map on this invalid pointer leads to a kernel
> > crash.
> >
> > Fix these issues by adding a NULL check at the start of
> > free_acpi_perf_data() and setting acpi_perf_data to NULL after freeing.
> >
> > Signed-off-by: Li RongQing <lirongqing@baidu.com>
> > ---
> >   drivers/cpufreq/acpi-cpufreq.c | 5 ++++-
> >   1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> > index 87e4923..cd2ca87 100644
> > --- a/drivers/cpufreq/acpi-cpufreq.c
> > +++ b/drivers/cpufreq/acpi-cpufreq.c
> > @@ -517,11 +517,14 @@ static void free_acpi_perf_data(void)
> >   {
> >   	unsigned int i;
> >
> > -	/* Freeing a NULL pointer is OK, and alloc_percpu zeroes. */
> > +	if (!acpi_perf_data)
> > +		return;
> > +
> >   	for_each_possible_cpu(i)
> >   		free_cpumask_var(per_cpu_ptr(acpi_perf_data, i)
> >   				 ->shared_cpu_map);
> >   	free_percpu(acpi_perf_data);
> > +	acpi_perf_data = NULL;
> >   }
> >
> >   static int cpufreq_boost_down_prep(unsigned int cpu)
> 
> 
> --
> Thx and BRs,
> Zhongqiu Han

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