Commit e77b89f1 authored by Dmitry Monakhov's avatar Dmitry Monakhov Committed by Dave Jones

[CPUFREQ] Fix use after free on governor restore

Currently on governer backup/restore path we storing governor's pointer.
This is wrong because one may unload governor's module after cpu goes
offline. As result use-after-free will take place on restored cpu.
It is not easy to exploit this bug, but still we have to close this
issue ASAP. Issue was introduced by following commit
084f3493

##TESTCASE##
#!/bin/sh -x
modprobe acpi_cpufreq
# Any non default governor, in may case it is "ondemand"
modprobe cpufreq_ondemand
echo ondemand > /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor
rmmod acpi_cpufreq
rmmod cpufreq_ondemand
modprobe acpi_cpufreq  # << use-after-free here.
Signed-off-by: default avatarDmitry Monakhov <dmonakhov@openvz.org>
Signed-off-by: default avatarDave Jones <davej@redhat.com>
parent 293afe44
...@@ -41,7 +41,7 @@ static struct cpufreq_driver *cpufreq_driver; ...@@ -41,7 +41,7 @@ static struct cpufreq_driver *cpufreq_driver;
static DEFINE_PER_CPU(struct cpufreq_policy *, cpufreq_cpu_data); static DEFINE_PER_CPU(struct cpufreq_policy *, cpufreq_cpu_data);
#ifdef CONFIG_HOTPLUG_CPU #ifdef CONFIG_HOTPLUG_CPU
/* This one keeps track of the previously set governor of a removed CPU */ /* This one keeps track of the previously set governor of a removed CPU */
static DEFINE_PER_CPU(struct cpufreq_governor *, cpufreq_cpu_governor); static DEFINE_PER_CPU(char[CPUFREQ_NAME_LEN], cpufreq_cpu_governor);
#endif #endif
static DEFINE_SPINLOCK(cpufreq_driver_lock); static DEFINE_SPINLOCK(cpufreq_driver_lock);
...@@ -774,10 +774,12 @@ int cpufreq_add_dev_policy(unsigned int cpu, struct cpufreq_policy *policy, ...@@ -774,10 +774,12 @@ int cpufreq_add_dev_policy(unsigned int cpu, struct cpufreq_policy *policy,
#ifdef CONFIG_SMP #ifdef CONFIG_SMP
unsigned long flags; unsigned long flags;
unsigned int j; unsigned int j;
#ifdef CONFIG_HOTPLUG_CPU #ifdef CONFIG_HOTPLUG_CPU
if (per_cpu(cpufreq_cpu_governor, cpu)) { struct cpufreq_governor *gov;
policy->governor = per_cpu(cpufreq_cpu_governor, cpu);
gov = __find_governor(per_cpu(cpufreq_cpu_governor, cpu));
if (gov) {
policy->governor = gov;
dprintk("Restoring governor %s for cpu %d\n", dprintk("Restoring governor %s for cpu %d\n",
policy->governor->name, cpu); policy->governor->name, cpu);
} }
...@@ -1111,7 +1113,8 @@ static int __cpufreq_remove_dev(struct sys_device *sys_dev) ...@@ -1111,7 +1113,8 @@ static int __cpufreq_remove_dev(struct sys_device *sys_dev)
#ifdef CONFIG_SMP #ifdef CONFIG_SMP
#ifdef CONFIG_HOTPLUG_CPU #ifdef CONFIG_HOTPLUG_CPU
per_cpu(cpufreq_cpu_governor, cpu) = data->governor; strncpy(per_cpu(cpufreq_cpu_governor, cpu), data->governor->name,
CPUFREQ_NAME_LEN);
#endif #endif
/* if we have other CPUs still registered, we need to unlink them, /* if we have other CPUs still registered, we need to unlink them,
...@@ -1135,7 +1138,8 @@ static int __cpufreq_remove_dev(struct sys_device *sys_dev) ...@@ -1135,7 +1138,8 @@ static int __cpufreq_remove_dev(struct sys_device *sys_dev)
continue; continue;
dprintk("removing link for cpu %u\n", j); dprintk("removing link for cpu %u\n", j);
#ifdef CONFIG_HOTPLUG_CPU #ifdef CONFIG_HOTPLUG_CPU
per_cpu(cpufreq_cpu_governor, j) = data->governor; strncpy(per_cpu(cpufreq_cpu_governor, j),
data->governor->name, CPUFREQ_NAME_LEN);
#endif #endif
cpu_sys_dev = get_cpu_sysdev(j); cpu_sys_dev = get_cpu_sysdev(j);
sysfs_remove_link(&cpu_sys_dev->kobj, "cpufreq"); sysfs_remove_link(&cpu_sys_dev->kobj, "cpufreq");
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment