linux-kernel - Re: [PATCH 1/2] cpufreq: unlock correct rwsem while updating policy->cpu

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]

Date:	Tue, 17 Sep 2013 10:16:42 +0530
From:	Viresh Kumar <viresh.kumar@...aro.org>
To:	"Jon Medhurst (Tixy)" <tixy@...aro.org>
Cc:	"Rafael J. Wysocki" <rjw@...k.pl>,
	Lists linaro-kernel <linaro-kernel@...ts.linaro.org>,
	Patch Tracking <patches@...aro.org>,
	"cpufreq@...r.kernel.org" <cpufreq@...r.kernel.org>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	"Srivatsa S. Bhat" <srivatsa.bhat@...ux.vnet.ibm.com>
Subject: Re: [PATCH 1/2] cpufreq: unlock correct rwsem while updating policy->cpu

On 17 September 2013 00:12, Jon Medhurst (Tixy) <tixy@...aro.org> wrote:
> The commit log to that patch still mentions taking both locks.

Yeah, it was sent in hurry to just give you a working solution and I forgot
to see the log if it is still valid.

> The code itself fixes the lockdep errors I had, so
>
> Tested-by: Jon Medhurst <tixy@...aro.org>

Great!!

> however, I still have concerns about the locking (below)...

:(

> But what about reads, like in cpufreq_frequency_table_update_policy_cpu
> which is called immediately after the unlocking writes? Should that not
> be covered by a reader lock?
>
> And after that call, policy is passed into blocking_notifier_call_chain,
> do those callbacks not want to look at policy fields? Or are they going
> to do there own locking?

policy->cpu is used at multiple places outside of cpufreq.c and cpufreq
core can't really put read locks for those accesses. Things will turn bad
only if cpufreq core has got these races where we are trying to access
a struct or pointer that belonged to the last policy->cpu, which is updated
now.. For example the case of lock you reported.. And so lock is required
only for those places..

> Or is update_policy_cpu itself meant to be called with a read lock held?

No.

> This is the first time I've looked at this code, so feel free just to
> say 'it's complicated, just trust me, I'm the expert, I know what I'm
> doing'...

We aren't that rude :)

Now, that you have tested this patch let me resent it...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/