linux-kernel - Re: [PATCH] cpufreq: flush any pending policy update work scheduled before freeing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191021102730.GA21581@bogus>
Date:   Mon, 21 Oct 2019 11:27:30 +0100
From:   Sudeep Holla <sudeep.holla@....com>
To:     Viresh Kumar <viresh.kumar@...aro.org>
Cc:     "Rafael J. Wysocki" <rjw@...ysocki.net>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Linux PM <linux-pm@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] cpufreq: flush any pending policy update work scheduled
 before freeing

On Mon, Oct 21, 2019 at 07:45:51AM +0530, Viresh Kumar wrote:
> On 18-10-19, 12:06, Sudeep Holla wrote:
> > Callstack is:
> >
> > (cpufreq_notifier_max)
> > (notifier_call_chain)
> > (blocking_notifier_call_chain)
> > (pm_qos_update_target)
> > (freq_qos_apply)
> > (freq_qos_remove_request)
> > (cpufreq_policy_free)
> > (subsys_interface_unregister)
> > (cpufreq_unregister_driver)
>
> @sudeep: I see that the patch is merged now, but as I said earlier the
> reasoning isn't clear yet. Please don't stop working on this and lets
> clean this once and for all.
>

Sure.

> What patches were you testing this with? My buggy patches or Rafael's
> patches as well ? At least with my patches, this can happen due to the
> other bug where the notifier doesn't get removed (as I said earlier),
> but once that bug isn't there then this shouldn't happen, else we have
> another bug in pipeline somewhere and should find it.
>

I just tested now with today's linux-pm/bleeding-edge branch.
And even if I move cancel_work_sync just after freq_qos_remove_notifier,
it works fine now. It was not the case on Friday.

Is that what you wanted to check or something else ?

Regards,
Sudeep

-->8

diff --git i/drivers/cpufreq/cpufreq.c w/drivers/cpufreq/cpufreq.c
index 829a3764df1b..48a224a6b178 100644
--- i/drivers/cpufreq/cpufreq.c
+++ w/drivers/cpufreq/cpufreq.c
@@ -1268,6 +1268,9 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
        freq_qos_remove_notifier(&policy->constraints, FREQ_QOS_MIN,
                                 &policy->nb_min);

+       /* Cancel any pending policy->update work before freeing the policy. */
+       cancel_work_sync(&policy->update);
+
        if (policy->max_freq_req) {
                /*
                 * CPUFREQ_CREATE_POLICY notification is sent only after
@@ -1279,8 +1282,6 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
        }

        freq_qos_remove_request(policy->min_freq_req);
-       /* Cancel any pending policy->update work before freeing the policy. */
-       cancel_work_sync(&policy->update);
        kfree(policy->min_freq_req);

        cpufreq_policy_put_kobj(policy);