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
| ||
|
Date: Thu, 12 May 2022 16:44:30 +0200 From: "Rafael J. Wysocki" <rafael@...nel.org> To: Schspa Shi <schspa@...il.com> Cc: "Rafael J. Wysocki" <rafael@...nel.org>, Viresh Kumar <viresh.kumar@...aro.org>, Linux PM <linux-pm@...r.kernel.org>, Linux Kernel Mailing List <linux-kernel@...r.kernel.org> Subject: Re: [PATCH v4 2/2] cpufreq: make interface functions and lock holding state clear On Thu, May 12, 2022 at 3:52 PM Schspa Shi <schspa@...il.com> wrote: > > cpufreq_offline() calls offline() and exit() under the policy rwsem > But they are called outside the rwsem in cpufreq_online(). > > This patch move the offline(), exit(), online(), init() to be inside > of policy rwsem to achieve a clear lock relationship. > > All the init() online() implement only initialize policy object without > holding this lock and won't call cpufreq APIs need to hold this lock. > > Signed-off-by: Schspa Shi <schspa@...il.com> IMV this still addresses 2 different issues and so it should be split into 2 different patches. > --- > 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 35dffd738580..f242d5488364 100644 > --- a/drivers/cpufreq/cpufreq.c > +++ b/drivers/cpufreq/cpufreq.c Patch 1: > @@ -1343,12 +1343,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) { > @@ -1388,7 +1388,6 @@ static int cpufreq_online(unsigned int cpu) > cpumask_copy(policy->related_cpus, policy->cpus); > } > > - down_write(&policy->rwsem); > /* > * affected cpus must always be the one, which are online. We aren't > * managing offline cpus here. which addresses the problem that cpufreq_online() updates the policy->cpus and related_cpus masks without holding the policy rwsem (since the policy kobject has been registered already at this point, this is generally unsafe). A side-effect of it is that ->online() and ->init() will be called under the policy rwsem now, but that should be fine and is more consistent than the current code too. Patch 2: > @@ -1540,7 +1539,6 @@ static int cpufreq_online(unsigned int cpu) > remove_cpu_dev_symlink(policy, get_cpu_device(j)); > > cpumask_clear(policy->cpus); > - up_write(&policy->rwsem); > > out_offline_policy: > if (cpufreq_driver->offline) > @@ -1549,6 +1547,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); > -- which addressed the issue of calling ->offline() and ->exit() without holding the policy rwsem that is at best inconsistent with cpufreq_offline().
Powered by blists - more mailing lists