Re: [PATCH] cpufreq: cppc_cpufreq: prevent crash on reading freqdomain_cpus

[Chris Hyser <chris.hyser@oracle.com>]

On 8/22/22 1:39 AM, Viresh Kumar wrote:
> On 19-08-22, 15:03, Chris Hyser wrote:
>> From: chris hyser <chris.hyser@oracle.com>
>>
>> While running stress-ng --sysfs on an ARM system following a cpu offline,
>> we encountered the following NULL pointer dereference in the cppc_cpufreq
>> scaling driver:
>>
>> [ 1003.576816] Call trace:
>> [ 1003.579255]  _find_next_bit+0x20/0xc8
>> [ 1003.582909]  cpufreq_show_cpus+0x78/0xf4
>> [ 1003.586830]  show_freqdomain_cpus+0x20/0x30 [cppc_cpufreq]
>> [ 1003.592318]  show+0x4c/0x78
>> [ 1003.595104]  sysfs_kf_seq_show+0x9
>>
>> This is the exact issue described in commit e25303676e18 ("cpufreq:
>> acpi_cpufreq: prevent crash on reading freqdomain_cpus") with the fix
>> described there also solving this issue. I tracked the root cause to the
>> following: a scaling driver which provides a struct freq_attr sysfs
>> attributes array passed via struct cpufreq_driver during driver
>> registration, has .init() and .exit() functions and does _not_ provide
>> .online()/.offline() routines. cpufreq core creates the attributes, but
>> does not remove them even though .exit() frees the underlying memory. The
>> core functions and most drivers have corresponding NULL data pointer
>> checks.
>>
>> Signed-off-by: Chris Hyser <chris.hyser@oracle.com>
>> ---
>>   drivers/cpufreq/cppc_cpufreq.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>> index 24eaf0e..4210353 100644
>> --- a/drivers/cpufreq/cppc_cpufreq.c
>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>> @@ -876,6 +876,9 @@ static ssize_t show_freqdomain_cpus(struct cpufreq_policy *policy, char *buf)
>>   {
>>   	struct cppc_cpudata *cpu_data = policy->driver_data;
>>   
>> +	if (unlikely(!cpu_data))
>> +		return -ENODEV;
>> +
>>   	return cpufreq_show_cpus(cpu_data->shared_cpu_map, buf);
>>   }
>>   cpufreq_freq_attr_ro(freqdomain_cpus);
> 
> What kernel version are you testing this on ?

5.19

> We merged a patch sometime back:
> 
> commit d4627a287e25 ("cpufreq: Abort show()/store() for half-initialized policies")
> 
> which I believe should have fixed this issue. I will be surprise if it
> doesn't.

This patch is present and apparently does not solve the problem.

-chrish