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  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:	Mon, 10 Nov 2014 07:26:42 -0500
From:	Prarit Bhargava <prarit@...hat.com>
To:	Viresh Kumar <viresh.kumar@...aro.org>
CC:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Robert Schöne <robert.schoene@...dresden.de>,
	Stephen Boyd <sboyd@...eaurora.org>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>
Subject: Re: [PATCH 2/5] cpufreq, fix locking around CPUFREQ_GOV_POLICY_EXIT
 calls



On 11/10/2014 05:44 AM, Viresh Kumar wrote:
> On 5 November 2014 20:23, Prarit Bhargava <prarit@...hat.com> wrote:
>> commit 955ef4833574636819cd269cfbae12f79cbde63a (" cpufreq: Drop rwsem
>> lock around CPUFREQ_GOV_POLICY_EXIT") opens up a hole in the locking
>> scheme for cpufreq.
>>
>> Simple tests such as rapidly switching the governor between ondemand and
>> performance or attempting to read policy values while a governor switch occurs
>> now fail with very NULL pointer warnings, sysfs namespace collisions, and
>> system hangs.  In short, the locking that policy->rwsem is supposed to provide
>> is currently broken.
>>
>> The identified commit attempts to resolve a lockdep warning by removing
>> a lock around a section of code which does a shutdown of the
>> existing policy.  The problem is that this is part of the _critical_ section of
>> code that switches the governors and must be protected by the lock; without
>> locking readers may access now NULL or stale data, and writes may collide with
>> each other.
>>
>> With the previous patch, which now returns -EBUSY during times of
>> contention the deadlock reported in
>> 955ef4833574636819cd269cfbae12f79cbde63a (" cpufreq: Drop rwsem lock
>> around CPUFREQ_GOV_POLICY_EXIT") cannot occur, so adding the locks back
>> into this section of code is possible.
> 
> I still fail to understand why ? What will the _trylock() change ?

viresh, afaict read_trylock will return 0 when busy with write:

static inline int queue_read_trylock(struct qrwlock *lock)
{
        u32 cnts;

        cnts = atomic_read(&lock->cnts);
        if (likely(!(cnts & _QW_WMASK))) {

so the deadlock will not occur.  the show() is opened, write lock is taken, and
if the thread is rescheduled and takes read lock the trylock will return 0, and
the thread will return -EBUSY to userspace avoiding the deadlock.

P.
> 
--
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/

Powered by blists - more mailing lists