[PATCH v2] cpufreq: fix race on cpufreq online
From: Schspa Shi
Date: Tue May 10 2022 - 11:30:35 EST
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 <schspa@xxxxxxxxx>
---
Changelog:
v1 -> v2:
- Fix bad critical region enlarge which causes uninitialized
unlock.
---
drivers/cpufreq/cpufreq.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 80f535cc8a75..8edfa840dd74 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1337,12 +1337,12 @@ static int cpufreq_online(unsigned int cpu)
down_write(&policy->rwsem);
policy->cpu = cpu;
policy->governor = NULL;
- up_write(&policy->rwsem);
} else {
new_policy = true;
policy = cpufreq_policy_alloc(cpu);
if (!policy)
return -ENOMEM;
+ down_write(&policy->rwsem);
}
if (!new_policy && cpufreq_driver->online) {
@@ -1533,7 +1533,7 @@ static int cpufreq_online(unsigned int cpu)
for_each_cpu(j, policy->real_cpus)
remove_cpu_dev_symlink(policy, get_cpu_device(j));
- up_write(&policy->rwsem);
+ cpumask_clear(policy->cpus);
out_offline_policy:
if (cpufreq_driver->offline)
@@ -1542,6 +1542,7 @@ static int cpufreq_online(unsigned int cpu)
out_exit_policy:
if (cpufreq_driver->exit)
cpufreq_driver->exit(policy);
+ up_write(&policy->rwsem);
out_free_policy:
cpufreq_policy_free(policy);
--
2.24.3 (Apple Git-128)