linux-kernel - RE: [PATCH] hpsa: fix boot on ia64 (atomic

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:   Wed, 17 Mar 2021 19:06:10 +0000
From:   <Don.Brace@...rochip.com>
To:     <David.Laight@...LAB.COM>, <martin.petersen@...cle.com>,
        <arnd@...nel.org>
CC:     <slyfox@...too.org>, <linux-kernel@...r.kernel.org>,
        <linux-ia64@...r.kernel.org>, <storagedev@...rochip.com>,
        <linux-scsi@...r.kernel.org>, <jszczype@...hat.com>,
        <Scott.Benesh@...rochip.com>, <Scott.Teel@...rochip.com>,
        <thenzl@...hat.com>, <glaubitz@...sik.fu-berlin.de>
Subject: RE: [PATCH] hpsa: fix boot on ia64 (atomic_t alignment)

-----Original Message-----
From: David Laight [mailto:David.Laight@...LAB.COM] 
Subject: RE: [PATCH] hpsa: fix boot on ia64 (atomic_t alignment)

From: Martin K. Petersen
> Sent: 17 March 2021 02:26
>
> Arnd,
>
> > Actually that still feels wrong: the annotation of the struct is to 
> > pack every member, which causes the access to be done in byte units 
> > on architectures that do not have hardware unaligned load/store 
> > instructions, at least for things like atomic_read() that does not 
> > go through a cmpxchg() or ll/sc cycle.
>
> > This change may fix itanium, but it's still not correct. Other 
> > architectures would have already been broken before the recent 
> > change, but that's not a reason against fixing them now.
>
> I agree. I understand why there are restrictions on fields consumed by 
> the hardware. But for fields internal to the driver the packing 
> doesn't make sense to me.

Jeepers -- that global #pragma pack(1) is bollocks.

I think there are a couple of __u64 that are 32bit aligned.
Just marking those field __packed __aligned(4) should have the desired effect.
Or use a typedef for '__u64 with 32bit alignment'.
(There probably ought to be one in types.h)

Then add compile-time asserts that any non-trivial structures the hardware accesses are the right size.

        David

Don: My dilemma is that hpsa is now a maintenance driver and making more packing/alignment changes would trigger a lot of regression testing. My last patch aligns the structure with what has been in place for a long time now. All I did was rename an unused variable. So making any more changes is not high on my todo list...