[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

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

[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