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

[Qianheng Peng]

On 14-07-26, 01:13:57 (PDT), Viresh Kumar wrote:
>On 14-07-26, 15:59, Qianheng Peng wrote:
>> The crash issue may occur when insmod amd_pstate_ut modules.
>> 
>>    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.
>> 
>> Fixes: 14eb1c96e3a3 ("cpufreq: amd-pstate: Add test module for amd-pstate driver")
>> Suggested-by: Li Xiong <xiongl24@xxxxxxxxxxxxxxx>
>> Suggested-by: Xibo Wang <wangxb12@xxxxxxxxxxxxxxx>
>> Signed-off-by: Qianheng Peng <pengqh1@xxxxxxxxxxxxxxx>
>> ---
>>  drivers/cpufreq/amd-pstate-ut.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>> 
>> diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
>> index 735b29f..3b24260 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 driver_data of %s is empty!\n", __func__, policy->kobj.name);
>
>You can use kobject_name(), but I wonder why print kobj name here ? Other
>drivers just print policy->cpu normally and that is sufficient.
Thanks for review.I will change kobj name to policy->cpu in patch v2.

--
Qianheng