[PATCH] cpufreq: Allow sysfs access only for active policies

From: Viresh Kumar
Date: Tue May 10 2022 - 23:43:26 EST


It is currently possible, in a corner case, to access the sysfs files
and reach show_cpuinfo_cur_freq(), etc, for a partly initialized policy.

This can happen for example if cpufreq_online() fails after adding the
sysfs files, which are immediately accessed by another process. There
can easily be other such cases, which aren't identified yet, like while
the policy is getting freed.

Process A: Process B

cpufreq_online()
down_write(&policy->rwsem);
if (new_policy) {
ret = cpufreq_add_dev_interface(policy);
/* This fails after adding few files */
if (ret)
goto out_destroy_policy;

...
}

...

out_destroy_policy:
...
up_write(&policy->rwsem);
/*
* This will end up accessing the policy
* which isn't fully initialized.
*/
show_cpuinfo_cur_freq()

if (cpufreq_driver->offline)
cpufreq_driver->offline(policy);

if (cpufreq_driver->exit)
cpufreq_driver->exit(policy);

cpufreq_policy_free(policy);

Fix these by checking in show/store if the policy is sysfs ready or not.

Reported-by: Schspa Shi <schspa@xxxxxxxxx>
Signed-off-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
---
drivers/cpufreq/cpufreq.c | 18 ++++++++++++++----
include/linux/cpufreq.h | 3 +++
2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index c8bf6c68597c..65c2bbcf555d 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -948,13 +948,14 @@ static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf)
{
struct cpufreq_policy *policy = to_policy(kobj);
struct freq_attr *fattr = to_attr(attr);
- ssize_t ret;
+ ssize_t ret = -EBUSY;

if (!fattr->show)
return -EIO;

down_read(&policy->rwsem);
- ret = fattr->show(policy, buf);
+ if (policy->sysfs_ready)
+ ret = fattr->show(policy, buf);
up_read(&policy->rwsem);

return ret;
@@ -965,7 +966,7 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr,
{
struct cpufreq_policy *policy = to_policy(kobj);
struct freq_attr *fattr = to_attr(attr);
- ssize_t ret = -EINVAL;
+ ssize_t ret = -EBUSY;

if (!fattr->store)
return -EIO;
@@ -979,7 +980,8 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr,

if (cpu_online(policy->cpu)) {
down_write(&policy->rwsem);
- ret = fattr->store(policy, buf, count);
+ if (policy->sysfs_ready)
+ ret = fattr->store(policy, buf, count);
up_write(&policy->rwsem);
}

@@ -1280,6 +1282,11 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
unsigned long flags;
int cpu;

+ /* Disallow sysfs interactions now */
+ down_write(&policy->rwsem);
+ policy->sysfs_ready = false;
+ up_write(&policy->rwsem);
+
/* Remove policy from list */
write_lock_irqsave(&cpufreq_driver_lock, flags);
list_del(&policy->policy_list);
@@ -1516,6 +1523,9 @@ static int cpufreq_online(unsigned int cpu)
goto out_destroy_policy;
}

+ /* We can allow sysfs interactions now */
+ policy->sysfs_ready = true;
+
up_write(&policy->rwsem);

kobject_uevent(&policy->kobj, KOBJ_ADD);
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 35c7d6db4139..7e4384e535fd 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -101,6 +101,9 @@ struct cpufreq_policy {
*/
struct rw_semaphore rwsem;

+ /* Policy is ready for sysfs interactions */
+ bool sysfs_ready;
+
/*
* Fast switch flags:
* - fast_switch_possible should be set by the driver if it can
--
2.31.1.272.g89b43f80a514