Re: [PATCH v2] cpufreq: amd-pstate-ut: fix null pointer dereference

[Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>]

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