linux-kernel - Re: [PATCH 1/3] PM / Wakeup: Add missing memory barriers

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: <Pine.LNX.4.44L0.1101261514280.1879-100000@iolanthe.rowland.org>
Date:	Wed, 26 Jan 2011 15:21:39 -0500 (EST)
From:	Alan Stern <stern@...land.harvard.edu>
To:	"Rafael J. Wysocki" <rjw@...k.pl>
cc:	Linux-pm mailing list <linux-pm@...ts.linux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/3] PM / Wakeup: Add missing memory barriers

On Tue, 25 Jan 2011, Rafael J. Wysocki wrote:

> From: Rafael J. Wysocki <rjw@...k.pl>
> 
> The memory barrier in wakeup_source_deactivate() is supposed to
> prevent the callers of pm_wakeup_pending() and pm_get_wakeup_count()
> from seeing the new value of events_in_progress (0, in particular)
> and the old value of event_count at the same time.  However, if
> wakeup_source_deactivate() is executed by CPU0 and, for instance,
> pm_wakeup_pending() is executed by CPU1, where both processors can
> reorder operations, the memory barrier in wakeup_source_deactivate()
> doesn't affect CPU1 which can reorder reads.  In that case CPU1 may
> very well decide to fetch event_count before it's modified and
> events_in_progress after it's been updated, so pm_wakeup_pending()
> may fail to detect a wakeup event.  This issue can be addressed by
> adding a read memory barrier in pm_wakeup_pending() that will enforce
> events_in_progress to be read before event_count.
> 
> For similar reason, a read memory barrier should be added to
> pm_get_wakeup_count().

How come this is implemented using memory barriers rather than a lock?  
Is it because this is potentially a fairly hot path?

New memory barriers are supposed to have comments present in the code, 
explaining why they are needed.

Ideally you could do away with the need for synchronization entirely.  
For example, events_in_progress and event_count could be stored as two 
16-bit values stuffed into a single atomic variable.  Then they could 
both be read or updated simultaneously.

Alan Stern

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