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

[Viresh Kumar <viresh.kumar@linaro.org>]

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 ?

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.

-- 
viresh