netdev - RE: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global

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 for Android: free password hash cracker in your pocket

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

Date:	Mon, 28 Sep 2015 14:50:44 +0000
From:	David Laight <David.Laight@...LAB.COM>
To:	'James Bottomley' <James.Bottomley@...senPartnership.com>
CC:	"'Rafael J. Wysocki'" <rjw@...ysocki.net>,
	Viresh Kumar <viresh.kumar@...aro.org>,
	Johannes Berg <johannes@...solutions.net>,
	"Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
	Linaro Kernel Mailman List <linaro-kernel@...ts.linaro.org>,
	QCA ath9k Development <ath9k-devel@....qualcomm.com>,
	Intel Linux Wireless <ilw@...ux.intel.com>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	"Linux Kernel Mailing List" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Linux ACPI <linux-acpi@...r.kernel.org>,
	"open list:BLUETOOTH DRIVERS" <linux-bluetooth@...r.kernel.org>,
	"open list:AMD IOMMU (AMD-VI)" <iommu@...ts.linux-foundation.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"open list:NETWORKING DRIVERS (WIRELESS)" 
	<linux-wireless@...r.kernel.org>,
	"open list:TARGET SUBSYSTEM" <linux-scsi@...r.kernel.org>,
	"open list:ULTRA-WIDEBAND (UWB) SUBSYSTEM:" 
	<linux-usb@...r.kernel.org>,
	"open list:EDAC-CORE" <linux-edac@...r.kernel.org>,
	Linux Memory Management List <linux-mm@...ck.org>,
	"moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM..." 
	<alsa-devel@...a-project.org>
Subject: RE: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of
 'global_lock'

From: James Bottomley [mailto:James.Bottomley@...senPartnership.com]
> Sent: 28 September 2015 15:27
> On Mon, 2015-09-28 at 08:58 +0000, David Laight wrote:
> > From: Rafael J. Wysocki
> > > Sent: 27 September 2015 15:09
> > ...
> > > > > Say you have three adjacent fields in a structure, x, y, z, each one byte long.
> > > > > Initially, all of them are equal to 0.
> > > > >
> > > > > CPU A writes 1 to x and CPU B writes 2 to y at the same time.
> > > > >
> > > > > What's the result?
> > > >
> > > > I think every CPU's  cache architecure guarantees adjacent store
> > > > integrity, even in the face of SMP, so it's x==1 and y==2.  If you're
> > > > thinking of old alpha SMP system where the lowest store width is 32 bits
> > > > and thus you have to do RMW to update a byte, this was usually fixed by
> > > > padding (assuming the structure is not packed).  However, it was such a
> > > > problem that even the later alpha chips had byte extensions.
> >
> > Does linux still support those old Alphas?
> >
> > The x86 cpus will also do 32bit wide rmw cycles for the 'bit' operations.
> 
> That's different: it's an atomic RMW operation.  The problem with the
> alpha was that the operation wasn't atomic (meaning that it can't be
> interrupted and no intermediate output states are visible).

It is only atomic if prefixed by the 'lock' prefix.
Normally the read and write are separate bus cycles.
 
> > You still have to ensure the compiler doesn't do wider rmw cycles.
> > I believe the recent versions of gcc won't do wider accesses for volatile data.
> 
> I don't understand this comment.  You seem to be implying gcc would do a
> 64 bit RMW for a 32 bit store ... that would be daft when a single
> instruction exists to perform the operation on all architectures.

Read the object code and weep...
It is most likely to happen for operations that are rmw (eg bit set).
For instance the arm cpu has limited offsets for 16bit accesses, for
normal structures the compiler is likely to use a 32bit rmw sequence
for a 16bit field that has a large offset.
The C language allows the compiler to do it for any access (IIRC including
volatiles).

	David