2022-05-14 01:19:19

by Schspa Shi

[permalink] [raw]
Subject: [PATCH 1/2] cpufreq: fix race on cpufreq online

When cpufreq online failed, policy->cpus are not empty while
cpufreq sysfs file available, we may access some data freed.

Take policy->clk as an example:

static int cpufreq_online(unsigned int cpu)
{
...
// policy->cpus != 0 at this time
down_write(&policy->rwsem);
ret = cpufreq_add_dev_interface(policy);
up_write(&policy->rwsem);

down_write(&policy->rwsem);
...
/* cpufreq nitialization fails in some cases */
if (cpufreq_driver->get && has_target()) {
policy->cur = cpufreq_driver->get(policy->cpu);
if (!policy->cur) {
ret = -EIO;
pr_err("%s: ->get() failed\n", __func__);
goto out_destroy_policy;
}
}
...
up_write(&policy->rwsem);
...

return 0;

out_destroy_policy:
for_each_cpu(j, policy->real_cpus)
remove_cpu_dev_symlink(policy, get_cpu_device(j));
up_write(&policy->rwsem);
...
out_exit_policy:
if (cpufreq_driver->exit)
cpufreq_driver->exit(policy);
clk_put(policy->clk);
// policy->clk is a wild pointer
...
^
|
Another process access
__cpufreq_get
cpufreq_verify_current_freq
cpufreq_generic_get
// acces wild pointer of policy->clk;
|
|
out_offline_policy: |
cpufreq_policy_free(policy); |
// deleted here, and will wait for no body reference
cpufreq_policy_put_kobj(policy);
}

We can fix it by clear the policy->cpus mask.
Both show_scaling_cur_freq and show_cpuinfo_cur_freq will return an
error by checking this mask, thus avoiding UAF.

Signed-off-by: Schspa Shi <[email protected]>

---

Changelog:
v1 -> v2:
- Fix bad critical region enlarge which causes uninitialized
unlock.
- Move cpumask_clear(policy->cpus); before out_offline_policy
v2 -> v3:
- Remove the missed down_write() before
cpumask_and(policy->cpus, policy->cpus, cpu_online_mask);
v3 -> v4:
- Seprate to two patchs.
- Add policy_is_inactive check before sysfs access
---
drivers/cpufreq/cpufreq.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 80f535cc8a75..35dffd738580 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -953,7 +953,10 @@ static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf)
return -EIO;

down_read(&policy->rwsem);
- ret = fattr->show(policy, buf);
+ if (unlikely(policy_is_inactive(policy)))
+ ret = -EBUSY;
+ else
+ ret = fattr->show(policy, buf);
up_read(&policy->rwsem);

return ret;
@@ -978,7 +981,10 @@ 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 (unlikely(policy_is_inactive(policy)))
+ ret = -EBUSY;
+ else
+ ret = fattr->store(policy, buf, count);
up_write(&policy->rwsem);
}

@@ -1533,6 +1539,7 @@ static int cpufreq_online(unsigned int cpu)
for_each_cpu(j, policy->real_cpus)
remove_cpu_dev_symlink(policy, get_cpu_device(j));

+ cpumask_clear(policy->cpus);
up_write(&policy->rwsem);

out_offline_policy:
--
2.29.0