mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] cpufreq: amd-pstate-ut: fix null pointer dereference
@ 2026-07-15  9:35 Qianheng Peng
  2026-07-15 14:00 ` Zhongqiu Han
  0 siblings, 1 reply; 4+ messages in thread
From: Qianheng Peng @ 2026-07-15  9:35 UTC (permalink / raw)
  To: ray.huang, mario.limonciello, perry.yuan, kprateek.nayak, rafael,
	viresh.kumar, skhan, li.meng
  Cc: linux-pm, linux-kernel, wangxb12, xiongl24, zhangar, pengqh1

The crash issue may occur when insmod amd_pstate_ut module on intel platform.

   amd_pstate_ut: 1    amd_pstate_ut_acpi_cpc_valid  success!
   amd_pstate_ut: 2    amd_pstate_ut_check_enabled   success!
   BUG: kernel NULL pointer dereference, address: 0000000000000080
   #PF: supervisor read access in kernel mode
   #PF: error_code(0x0000) - not-present page
   PGD 0 P4D 0
   Oops: 0000 [#1] SMP NOPTI
   CPU: 0 PID: 20300 Comm: modprobe
   Kdump: loaded Tainted: G O 6.6.0-0010.rc1.ctl4.x86_64 #1
   Hardware name: FiberHome R2200 V5/Xeon Boards, BIOS 3.1a 02/24/2020
   RIP: 0010:amd_pstate_ut_check_perf+0x141/0x280 [amd_pstate_ut]
   Call Trace:
    <TASK>
    amd_pstate_ut_init+0x1b/0xff0 [amd_pstate_ut]
    ? __pfx_amd_pstate_ut_init+0x10/0x10 [amd_pstate_ut]
    do_one_initcall+0x42/0x2e0
    ? kmalloc_trace+0x26/0x90
    do_init_module+0x60/0x240
    __se_sys_init_module+0x185/0x1c0
    do_syscall_64+0x62/0x190
    entry_SYSCALL_64_after_hwframe+0x76/0x7e
    </TASK>

Add invalidation check of cpudata in amd_pstate_ut_check_perf() and
amd_pstate_ut_check_freq() to avoid unpredicated null pointer dereference.
Otherwise, add driver name detection for amd_pstate_ut driver to prevent it
from testing cpufreq driver on non-amd platform.

Fixes: 14eb1c96e3a3 ("cpufreq: amd-pstate: Add test module for amd-pstate driver")
Suggested-by: Li Xiong <xiongl24@chinatelecom.cn>
Suggested-by: Xibo Wang <wangxb12@chinatelecom.cn>
Signed-off-by: Qianheng Peng <pengqh1@chinatelecom.cn>

---
v2:
  - Print policy->cpu instead of policy->kboj.name
  - Add cpufreq driver name detection to amd_pstate_ut

v1: https://lore.kernel.org/all/1784015941-26535-1-git-send-email-pengqh1@chinatelecom.cn
---
 drivers/cpufreq/amd-pstate-ut.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
index 735b29f..5d71315 100644
--- a/drivers/cpufreq/amd-pstate-ut.c
+++ b/drivers/cpufreq/amd-pstate-ut.c
@@ -157,6 +157,10 @@ static int amd_pstate_ut_check_perf(u32 index)
 		if (!policy)
 			continue;
 		cpudata = policy->driver_data;
+		if (!cpudata) {
+			pr_err("%s empty driver_data on cpu %d\n", __func__, policy->cpu);
+			return -EINVAL;
+		}
 
 		if (get_shared_mem()) {
 			ret = cppc_get_perf_caps(cpu, &cppc_perf);
@@ -229,6 +233,10 @@ static int amd_pstate_ut_check_freq(u32 index)
 		if (!policy)
 			continue;
 		cpudata = policy->driver_data;
+		if (!cpudata) {
+			pr_err("%s empty driver_data on cpu %d\n", __func__, policy->cpu);
+			return -EINVAL;
+		}
 
 		if (!((policy->cpuinfo.max_freq >= cpudata->nominal_freq) &&
 			(cpudata->nominal_freq > cpudata->lowest_nonlinear_freq) &&
@@ -560,6 +568,15 @@ static int amd_pstate_ut_check_freq_attrs(u32 index)
 static int __init amd_pstate_ut_init(void)
 {
 	u32 i = 0, arr_size = ARRAY_SIZE(amd_pstate_ut_cases);
+	const char *driver_name = NULL;
+
+	driver_name = cpufreq_get_current_driver();
+	if (!driver_name)
+		return -ENOENT;
+	/* don't test if non-amd cpufreq_driver exists */
+	else if (strcmp(driver_name, "amd-pstate") &&
+		 strcmp(driver_name, "amd-pstate-epp"))
+		return -EEXIST;
 
 	for (i = 0; i < arr_size; i++) {
 		int ret;
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] cpufreq: amd-pstate-ut: fix null pointer dereference
  2026-07-15  9:35 [PATCH v2] cpufreq: amd-pstate-ut: fix null pointer dereference Qianheng Peng
@ 2026-07-15 14:00 ` Zhongqiu Han
  2026-07-15 14:51   ` K Prateek Nayak
  0 siblings, 1 reply; 4+ messages in thread
From: Zhongqiu Han @ 2026-07-15 14:00 UTC (permalink / raw)
  To: Qianheng Peng, ray.huang, mario.limonciello, perry.yuan,
	kprateek.nayak, rafael, viresh.kumar, skhan, li.meng
  Cc: linux-pm, linux-kernel, wangxb12, xiongl24, zhangar, zhongqiu.han

Hi Qianheng,

On 7/15/2026 5:35 PM, Qianheng Peng wrote:
> The crash issue may occur when insmod amd_pstate_ut module on intel platform.
> 
>     amd_pstate_ut: 1    amd_pstate_ut_acpi_cpc_valid  success!
>     amd_pstate_ut: 2    amd_pstate_ut_check_enabled   success!
>     BUG: kernel NULL pointer dereference, address: 0000000000000080
>     #PF: supervisor read access in kernel mode
>     #PF: error_code(0x0000) - not-present page
>     PGD 0 P4D 0
>     Oops: 0000 [#1] SMP NOPTI
>     CPU: 0 PID: 20300 Comm: modprobe
>     Kdump: loaded Tainted: G O 6.6.0-0010.rc1.ctl4.x86_64 #1
>     Hardware name: FiberHome R2200 V5/Xeon Boards, BIOS 3.1a 02/24/2020
>     RIP: 0010:amd_pstate_ut_check_perf+0x141/0x280 [amd_pstate_ut]
>     Call Trace:
>      <TASK>
>      amd_pstate_ut_init+0x1b/0xff0 [amd_pstate_ut]
>      ? __pfx_amd_pstate_ut_init+0x10/0x10 [amd_pstate_ut]
>      do_one_initcall+0x42/0x2e0
>      ? kmalloc_trace+0x26/0x90
>      do_init_module+0x60/0x240
>      __se_sys_init_module+0x185/0x1c0
>      do_syscall_64+0x62/0x190
>      entry_SYSCALL_64_after_hwframe+0x76/0x7e
>      </TASK>
> 
> Add invalidation check of cpudata in amd_pstate_ut_check_perf() and
> amd_pstate_ut_check_freq() to avoid unpredicated null pointer dereference.
> Otherwise, add driver name detection for amd_pstate_ut driver to prevent it

The driver-name check in amd_pstate_ut_init() is the actual fix: it
prevents this module from running its test cases on non-AMD platforms.

The NULL check should not be necessary. On an already-registered amd
pstate driver, policy->driver_data is set only after a successful
->init() and is cleared only in ->exit(), so it is never legitimately
NULL for a live policy:

1. Hotplug offline path: a normal CPU hotplug offline does not hit
->exit(). The driver's .offline callback makes __cpufreq_offline() take
the light-weight teardown path and return early, so ->exit() (and the
kfree) is not invoked and cpudata stays valid. The full teardown only
happens on driver unregister / device unbind.

2. Driver exit path: the NULL check does not close the theoretical UAF
window here, since the test does not hold policy->rwsem: cpudata can be
kfree()'d between the check and the subsequent dereference. Worse, the
passive amd_pstate_cpu_exit() frees cpudata but does not reset policy
->driver_data to NULL, so the check would pass and then dereference a
dangling pointer. This is a pre-existing issue and out of scope for this
fix.

so it is better to drop the "NULL check of cpudata" commit msg and code.

(Others may have more context here, so please free free correct me if
I'm wrong.)



> from testing cpufreq driver on non-amd platform.
> 
> Fixes: 14eb1c96e3a3 ("cpufreq: amd-pstate: Add test module for amd-pstate driver")
> Suggested-by: Li Xiong <xiongl24@chinatelecom.cn>
> Suggested-by: Xibo Wang <wangxb12@chinatelecom.cn>
> Signed-off-by: Qianheng Peng <pengqh1@chinatelecom.cn>
> 
> ---
> v2:
>    - Print policy->cpu instead of policy->kboj.name
>    - Add cpufreq driver name detection to amd_pstate_ut
> 
> v1: https://lore.kernel.org/all/1784015941-26535-1-git-send-email-pengqh1@chinatelecom.cn
> ---
>   drivers/cpufreq/amd-pstate-ut.c | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
> index 735b29f..5d71315 100644
> --- a/drivers/cpufreq/amd-pstate-ut.c
> +++ b/drivers/cpufreq/amd-pstate-ut.c
> @@ -157,6 +157,10 @@ static int amd_pstate_ut_check_perf(u32 index)
>   		if (!policy)
>   			continue;
>   		cpudata = policy->driver_data;
> +		if (!cpudata) {
> +			pr_err("%s empty driver_data on cpu %d\n", __func__, policy->cpu);
> +			return -EINVAL;

Strictly speaking, it should be %u, right? Since policy->cpu is an
unsigned int, using %d is a format-specifier type mismatch.

> +		}
>   
>   		if (get_shared_mem()) {
>   			ret = cppc_get_perf_caps(cpu, &cppc_perf);
> @@ -229,6 +233,10 @@ static int amd_pstate_ut_check_freq(u32 index)
>   		if (!policy)
>   			continue;
>   		cpudata = policy->driver_data;
> +		if (!cpudata) {
> +			pr_err("%s empty driver_data on cpu %d\n", __func__, policy->cpu);
> +			return -EINVAL;
> +		}
>   
>   		if (!((policy->cpuinfo.max_freq >= cpudata->nominal_freq) &&
>   			(cpudata->nominal_freq > cpudata->lowest_nonlinear_freq) &&
> @@ -560,6 +568,15 @@ static int amd_pstate_ut_check_freq_attrs(u32 index)
>   static int __init amd_pstate_ut_init(void)
>   {
>   	u32 i = 0, arr_size = ARRAY_SIZE(amd_pstate_ut_cases);
> +	const char *driver_name = NULL;
> +
> +	driver_name = cpufreq_get_current_driver();
> +	if (!driver_name)
> +		return -ENOENT;
> +	/* don't test if non-amd cpufreq_driver exists */
> +	else if (strcmp(driver_name, "amd-pstate") &&

Nit: Redundant else after a return: since the previous branch already
returns, control can never reach the else if, so it is better to drop
the else and use a separate if.

> +		 strcmp(driver_name, "amd-pstate-epp"))
> +		return -EEXIST;

How about -EOPNOTSUPP or -ENODEV? And it would be helpful to log
error info here.


>   
>   	for (i = 0; i < arr_size; i++) {
>   		int ret;


-- 
Thx and BRs,
Zhongqiu Han

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] cpufreq: amd-pstate-ut: fix null pointer dereference
  2026-07-15 14:00 ` Zhongqiu Han
@ 2026-07-15 14:51   ` K Prateek Nayak
  2026-07-16  3:52     ` Qianheng Peng
  0 siblings, 1 reply; 4+ messages in thread
From: K Prateek Nayak @ 2026-07-15 14:51 UTC (permalink / raw)
  To: Zhongqiu Han, Qianheng Peng, ray.huang, mario.limonciello,
	perry.yuan, rafael, viresh.kumar, skhan, li.meng
  Cc: linux-pm, linux-kernel, wangxb12, xiongl24, zhangar

Hello Zhongqiu, Qianheng,

Thank you both for taking a look at this!

On 7/15/2026 7:30 PM, Zhongqiu Han wrote:
> Hi Qianheng,
> 
> On 7/15/2026 5:35 PM, Qianheng Peng wrote:
>> The crash issue may occur when insmod amd_pstate_ut module on intel platform.
>>
>>     amd_pstate_ut: 1    amd_pstate_ut_acpi_cpc_valid  success!
>>     amd_pstate_ut: 2    amd_pstate_ut_check_enabled   success!
>>     BUG: kernel NULL pointer dereference, address: 0000000000000080
>>     #PF: supervisor read access in kernel mode
>>     #PF: error_code(0x0000) - not-present page
>>     PGD 0 P4D 0
>>     Oops: 0000 [#1] SMP NOPTI
>>     CPU: 0 PID: 20300 Comm: modprobe
>>     Kdump: loaded Tainted: G O 6.6.0-0010.rc1.ctl4.x86_64 #1
>>     Hardware name: FiberHome R2200 V5/Xeon Boards, BIOS 3.1a 02/24/2020
>>     RIP: 0010:amd_pstate_ut_check_perf+0x141/0x280 [amd_pstate_ut]
>>     Call Trace:
>>      <TASK>
>>      amd_pstate_ut_init+0x1b/0xff0 [amd_pstate_ut]
>>      ? __pfx_amd_pstate_ut_init+0x10/0x10 [amd_pstate_ut]
>>      do_one_initcall+0x42/0x2e0
>>      ? kmalloc_trace+0x26/0x90
>>      do_init_module+0x60/0x240
>>      __se_sys_init_module+0x185/0x1c0
>>      do_syscall_64+0x62/0x190
>>      entry_SYSCALL_64_after_hwframe+0x76/0x7e
>>      </TASK>
>>
>> Add invalidation check of cpudata in amd_pstate_ut_check_perf() and
>> amd_pstate_ut_check_freq() to avoid unpredicated null pointer dereference.
>> Otherwise, add driver name detection for amd_pstate_ut driver to prevent it
> 
> The driver-name check in amd_pstate_ut_init() is the actual fix: it
> prevents this module from running its test cases on non-AMD platforms.
> 
> The NULL check should not be necessary. On an already-registered amd
> pstate driver, policy->driver_data is set only after a successful
> ->init() and is cleared only in ->exit(), so it is never legitimately
> NULL for a live policy:
> 
> 1. Hotplug offline path: a normal CPU hotplug offline does not hit
> ->exit(). The driver's .offline callback makes __cpufreq_offline() take
> the light-weight teardown path and return early, so ->exit() (and the
> kfree) is not invoked and cpudata stays valid. The full teardown only
> happens on driver unregister / device unbind.
> 
> 2. Driver exit path: the NULL check does not close the theoretical UAF
> window here, since the test does not hold policy->rwsem: cpudata can be
> kfree()'d between the check and the subsequent dereference. Worse, the
> passive amd_pstate_cpu_exit() frees cpudata but does not reset policy
> ->driver_data to NULL, so the check would pass and then dereference a
> dangling pointer. This is a pre-existing issue and out of scope for this
> fix.
> 
> so it is better to drop the "NULL check of cpudata" commit msg and code.
> 
> (Others may have more context here, so please free free correct me if
> I'm wrong.)

Ack! amd-pstate-ut was intended to be run in isolation without any
other concurrent driver operation as a sanity check for developers.
Races with hotplug / any user initiated changes during the test run
can cause issues.

> 
> 
> 
>> from testing cpufreq driver on non-amd platform.
>>
>> Fixes: 14eb1c96e3a3 ("cpufreq: amd-pstate: Add test module for amd-pstate driver")
>> Suggested-by: Li Xiong <xiongl24@chinatelecom.cn>
>> Suggested-by: Xibo Wang <wangxb12@chinatelecom.cn>
>> Signed-off-by: Qianheng Peng <pengqh1@chinatelecom.cn>
>>
>> ---
>> v2:
>>    - Print policy->cpu instead of policy->kboj.name
>>    - Add cpufreq driver name detection to amd_pstate_ut
>>
>> v1: https://lore.kernel.org/all/1784015941-26535-1-git-send-email-pengqh1@chinatelecom.cn
>> ---
>>   drivers/cpufreq/amd-pstate-ut.c | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
>> index 735b29f..5d71315 100644
>> --- a/drivers/cpufreq/amd-pstate-ut.c
>> +++ b/drivers/cpufreq/amd-pstate-ut.c
>> @@ -157,6 +157,10 @@ static int amd_pstate_ut_check_perf(u32 index)
>>           if (!policy)
>>               continue;
>>           cpudata = policy->driver_data;
>> +        if (!cpudata) {
>> +            pr_err("%s empty driver_data on cpu %d\n", __func__, policy->cpu);
>> +            return -EINVAL;
> 
> Strictly speaking, it should be %u, right? Since policy->cpu is an
> unsigned int, using %d is a format-specifier type mismatch.
> 
>> +        }
>>             if (get_shared_mem()) {
>>               ret = cppc_get_perf_caps(cpu, &cppc_perf);
>> @@ -229,6 +233,10 @@ static int amd_pstate_ut_check_freq(u32 index)
>>           if (!policy)
>>               continue;
>>           cpudata = policy->driver_data;
>> +        if (!cpudata) {
>> +            pr_err("%s empty driver_data on cpu %d\n", __func__, policy->cpu);
>> +            return -EINVAL;
>> +        }
>>             if (!((policy->cpuinfo.max_freq >= cpudata->nominal_freq) &&
>>               (cpudata->nominal_freq > cpudata->lowest_nonlinear_freq) &&
>> @@ -560,6 +568,15 @@ static int amd_pstate_ut_check_freq_attrs(u32 index)
>>   static int __init amd_pstate_ut_init(void)
>>   {
>>       u32 i = 0, arr_size = ARRAY_SIZE(amd_pstate_ut_cases);
>> +    const char *driver_name = NULL;
>> +
>> +    driver_name = cpufreq_get_current_driver();
>> +    if (!driver_name)
>> +        return -ENOENT;
>> +    /* don't test if non-amd cpufreq_driver exists */
>> +    else if (strcmp(driver_name, "amd-pstate") &&
> 
> Nit: Redundant else after a return: since the previous branch already
> returns, control can never reach the else if, so it is better to drop
> the else and use a separate if.

A simpler check would be:

  enum amd_pstate_mode mode = amd_pstate_get_status();

  if (mode == AMD_PSTATE_UNDEFINED || mode == AMD_PSTATE_DISABLE) 
           return -ENOENT;

Status always starts out as undefined and if the driver doesn't
load, it stays that way. If user has disabled the driver for any
reason, the unit test should bail out too.

> 
>> +         strcmp(driver_name, "amd-pstate-epp"))
>> +        return -EEXIST;
> 
> How about -EOPNOTSUPP or -ENODEV? And it would be helpful to log
> error info here.
> 
> 
>>         for (i = 0; i < arr_size; i++) {
>>           int ret;
> 
> 

-- 
Thanks and Regards,
Prateek


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] cpufreq: amd-pstate-ut: fix null pointer dereference
  2026-07-15 14:51   ` K Prateek Nayak
@ 2026-07-16  3:52     ` Qianheng Peng
  0 siblings, 0 replies; 4+ messages in thread
From: Qianheng Peng @ 2026-07-16  3:52 UTC (permalink / raw)
  To: kprateek.nayak
  Cc: li.meng, linux-kernel, linux-pm, mario.limonciello, pengqh1,
	perry.yuan, rafael, ray.huang, skhan, viresh.kumar, wangxb12,
	xiongl24, zhangar, zhongqiu.han

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 7029 bytes --]

Hello Prateek, Zhongqiu,
Thank you both for the awesome analyses.

On 7/15/2026 20:21:34, K Prateek Nayak wrote:
>Hello Zhongqiu, Qianheng,
>
>Thank you both for taking a look at this!
>
>On 7/15/2026 7:30 PM, Zhongqiu Han wrote:
>> Hi Qianheng,
>> 
>> On 7/15/2026 5:35 PM, Qianheng Peng wrote:
>>> The crash issue may occur when insmod amd_pstate_ut module on intel platform.
>>>
>>>     amd_pstate_ut: 1    amd_pstate_ut_acpi_cpc_valid  success!
>>>     amd_pstate_ut: 2    amd_pstate_ut_check_enabled   success!
>>>     BUG: kernel NULL pointer dereference, address: 0000000000000080
>>>     #PF: supervisor read access in kernel mode
>>>     #PF: error_code(0x0000) - not-present page
>>>     PGD 0 P4D 0
>>>     Oops: 0000 [#1] SMP NOPTI
>>>     CPU: 0 PID: 20300 Comm: modprobe
>>>     Kdump: loaded Tainted: G O 6.6.0-0010.rc1.ctl4.x86_64 #1
>>>     Hardware name: FiberHome R2200 V5/Xeon Boards, BIOS 3.1a 02/24/2020
>>>     RIP: 0010:amd_pstate_ut_check_perf+0x141/0x280 [amd_pstate_ut]
>>>     Call Trace:
>>>      <TASK>
>>>      amd_pstate_ut_init+0x1b/0xff0 [amd_pstate_ut]
>>>      ? __pfx_amd_pstate_ut_init+0x10/0x10 [amd_pstate_ut]
>>>      do_one_initcall+0x42/0x2e0
>>>      ? kmalloc_trace+0x26/0x90
>>>      do_init_module+0x60/0x240
>>>      __se_sys_init_module+0x185/0x1c0
>>>      do_syscall_64+0x62/0x190
>>>      entry_SYSCALL_64_after_hwframe+0x76/0x7e
>>>      </TASK>
>>>
>>> Add invalidation check of cpudata in amd_pstate_ut_check_perf() and
>>> amd_pstate_ut_check_freq() to avoid unpredicated null pointer dereference.
>>> Otherwise, add driver name detection for amd_pstate_ut driver to prevent it
>> 
>> The driver-name check in amd_pstate_ut_init() is the actual fix: it
>> prevents this module from running its test cases on non-AMD platforms.
>> 
>> The NULL check should not be necessary. On an already-registered amd
>> pstate driver, policy->driver_data is set only after a successful
>> ->init() and is cleared only in ->exit(), so it is never legitimately
>> NULL for a live policy:
>> 
>> 1. Hotplug offline path: a normal CPU hotplug offline does not hit
>> ->exit(). The driver's .offline callback makes __cpufreq_offline() take
>> the light-weight teardown path and return early, so ->exit() (and the
>> kfree) is not invoked and cpudata stays valid. The full teardown only
>> happens on driver unregister / device unbind.
>> 
>> 2. Driver exit path: the NULL check does not close the theoretical UAF
>> window here, since the test does not hold policy->rwsem: cpudata can be
>> kfree()'d between the check and the subsequent dereference. Worse, the
>> passive amd_pstate_cpu_exit() frees cpudata but does not reset policy
>> ->driver_data to NULL, so the check would pass and then dereference a
>> dangling pointer. This is a pre-existing issue and out of scope for this
>> fix.
>> 
>> so it is better to drop the "NULL check of cpudata" commit msg and code.
>> 
>> (Others may have more context here, so please free free correct me if
>> I'm wrong.)
>
>Ack! amd-pstate-ut was intended to be run in isolation without any
>other concurrent driver operation as a sanity check for developers.
>Races with hotplug / any user initiated changes during the test run
>can cause issues.
>
Yes, I agree that dropping NULL check of cpudata is reasonable
for the real user cases.
>> 
>> 
>> 
>>> from testing cpufreq driver on non-amd platform.
>>>
>>> Fixes: 14eb1c96e3a3 ("cpufreq: amd-pstate: Add test module for amd-pstate driver")
>>> Suggested-by: Li Xiong <xiongl24@chinatelecom.cn>
>>> Suggested-by: Xibo Wang <wangxb12@chinatelecom.cn>
>>> Signed-off-by: Qianheng Peng <pengqh1@chinatelecom.cn>
>>>
>>> ---
>>> v2:
>>>    - Print policy->cpu instead of policy->kboj.name
>>>    - Add cpufreq driver name detection to amd_pstate_ut
>>>
>>> v1: https://lore.kernel.org/all/1784015941-26535-1-git-send-email-pengqh1@chinatelecom.cn
>>> ---
>>>   drivers/cpufreq/amd-pstate-ut.c | 17 +++++++++++++++++
>>>   1 file changed, 17 insertions(+)
>>>
>>> diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
>>> index 735b29f..5d71315 100644
>>> --- a/drivers/cpufreq/amd-pstate-ut.c
>>> +++ b/drivers/cpufreq/amd-pstate-ut.c
>>> @@ -157,6 +157,10 @@ static int amd_pstate_ut_check_perf(u32 index)
>>>           if (!policy)
>>>               continue;
>>>           cpudata = policy->driver_data;
>>> +        if (!cpudata) {
>>> +            pr_err("%s empty driver_data on cpu %d\n", __func__, policy->cpu);
>>> +            return -EINVAL;
>> 
>> Strictly speaking, it should be %u, right? Since policy->cpu is an
>> unsigned int, using %d is a format-specifier type mismatch.
>> 
>>> +        }
>>>             if (get_shared_mem()) {
>>>               ret = cppc_get_perf_caps(cpu, &cppc_perf);
>>> @@ -229,6 +233,10 @@ static int amd_pstate_ut_check_freq(u32 index)
>>>           if (!policy)
>>>               continue;
>>>           cpudata = policy->driver_data;
>>> +        if (!cpudata) {
>>> +            pr_err("%s empty driver_data on cpu %d\n", __func__, policy->cpu);
>>> +            return -EINVAL;
>>> +        }
>>>             if (!((policy->cpuinfo.max_freq >= cpudata->nominal_freq) &&
>>>               (cpudata->nominal_freq > cpudata->lowest_nonlinear_freq) &&
>>> @@ -560,6 +568,15 @@ static int amd_pstate_ut_check_freq_attrs(u32 index)
>>>   static int __init amd_pstate_ut_init(void)
>>>   {
>>>       u32 i = 0, arr_size = ARRAY_SIZE(amd_pstate_ut_cases);
>>> +    const char *driver_name = NULL;
>>> +
>>> +    driver_name = cpufreq_get_current_driver();
>>> +    if (!driver_name)
>>> +        return -ENOENT;
>>> +    /* don't test if non-amd cpufreq_driver exists */
>>> +    else if (strcmp(driver_name, "amd-pstate") &&
>> 
>> Nit: Redundant else after a return: since the previous branch already
>> returns, control can never reach the else if, so it is better to drop
>> the else and use a separate if.
>
>A simpler check would be:
>
>  enum amd_pstate_mode mode = amd_pstate_get_status();
>
>  if (mode == AMD_PSTATE_UNDEFINED || mode == AMD_PSTATE_DISABLE) 
>           return -ENOENT;
>
>Status always starts out as undefined and if the driver doesn't
>load, it stays that way. If user has disabled the driver for any
>reason, the unit test should bail out too.
>
>> 
>>> +         strcmp(driver_name, "amd-pstate-epp"))
>>> +        return -EEXIST;
>> 
>> How about -EOPNOTSUPP or -ENODEV? And it would be helpful to log
>> error info here.
>> 
>> 
>>>         for (i = 0; i < arr_size; i++) {
>>>           int ret;
>> 
>> 
The simple check is honestly graceful, I will send it in patch v3.
And I also think -EOPNOTSUPP is better for telling current error.

--
Thanks and Regards,
Qianheng

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-07-16  4:00 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-15  9:35 [PATCH v2] cpufreq: amd-pstate-ut: fix null pointer dereference Qianheng Peng
2026-07-15 14:00 ` Zhongqiu Han
2026-07-15 14:51   ` K Prateek Nayak
2026-07-16  3:52     ` Qianheng Peng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome