Revert "cpufreq: Fix possible race in cpufreq online error path"

[PATCH] Revert "cpufreq: Fix possible race in cpufreq online error path"

Posted by Viresh Kumar 4 years ago

This reverts commit f346e96267cd76175d6c201b40f770c0116a8a04.

The commit tried to fix a possible real bug but it made it even worse.
The fix was simply buggy as now an error out to out_offline_policy or
out_exit_policy will try to release a semaphore which was never taken in
the first place. This works fine only if we failed late, i.e. via
out_destroy_policy.

Fixes: f346e96267cd ("cpufreq: Fix possible race in cpufreq online error path")
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 233e8af48848..fbaa8e6c7d23 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1534,6 +1534,8 @@ 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);
+
 out_offline_policy:
 	if (cpufreq_driver->offline)
 		cpufreq_driver->offline(policy);
@@ -1542,9 +1544,6 @@ static int cpufreq_online(unsigned int cpu)
 	if (cpufreq_driver->exit)
 		cpufreq_driver->exit(policy);
 
-	cpumask_clear(policy->cpus);
-	up_write(&policy->rwsem);
-
 out_free_policy:
 	cpufreq_policy_free(policy);
 	return ret;
-- 
2.31.1.272.g89b43f80a514

Re: [PATCH] Revert "cpufreq: Fix possible race in cpufreq online error path"

Posted by Rafael J. Wysocki 4 years ago

On Mon, May 9, 2022 at 5:57 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> This reverts commit f346e96267cd76175d6c201b40f770c0116a8a04.
>
> The commit tried to fix a possible real bug but it made it even worse.
> The fix was simply buggy as now an error out to out_offline_policy or
> out_exit_policy will try to release a semaphore which was never taken in
> the first place. This works fine only if we failed late, i.e. via
> out_destroy_policy.
>
> Fixes: f346e96267cd ("cpufreq: Fix possible race in cpufreq online error path")
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

Applied, thanks!

I should have caught the breakage in this patch, sorry about missing it.

> ---
>  drivers/cpufreq/cpufreq.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 233e8af48848..fbaa8e6c7d23 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1534,6 +1534,8 @@ 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);
> +
>  out_offline_policy:
>         if (cpufreq_driver->offline)
>                 cpufreq_driver->offline(policy);
> @@ -1542,9 +1544,6 @@ static int cpufreq_online(unsigned int cpu)
>         if (cpufreq_driver->exit)
>                 cpufreq_driver->exit(policy);
>
> -       cpumask_clear(policy->cpus);
> -       up_write(&policy->rwsem);
> -
>  out_free_policy:
>         cpufreq_policy_free(policy);
>         return ret;
> --
> 2.31.1.272.g89b43f80a514
>