Skip to contentSkip to conversation

cpufreq: fix race on cpufreq online

PATCH New Schspa Shi 1 participant 1 message May 12, 2022 CPU frequency scaling framework

lore
Share & tools
lore

Schspa Shi

author
#0 lore raw

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.

---

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(-)
drivers/cpufreq/cpufreq.c +9 -2
index 80f535cc8a75..35dffd738580 100644
@@ -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:
Signature · 1 line
2.29.0