linux-kernel - Re: [tip:x86/fpu] bitops: Add clear/set

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] [day] [month] [year] [list]

Date:   Sun, 5 Nov 2017 10:01:29 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Heiko Carstens <heiko.carstens@...ibm.com>
Cc:     Peter Anvin <hpa@...or.com>, Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Andi Kleen <ak@...ux.intel.com>,
        "linux-tip-commits@...r.kernel.org" 
        <linux-tip-commits@...r.kernel.org>
Subject: Re: [tip:x86/fpu] bitops: Add clear/set_bit32() to linux/bitops.h

On Sun, Nov 5, 2017 at 7:53 AM, Heiko Carstens
<heiko.carstens@...ibm.com> wrote:
> On Tue, Oct 17, 2017 at 09:21:46AM -0700, tip-bot for Andi Kleen wrote:
>>
>> bitops: Add clear/set_bit32() to linux/bitops.h
>
> This does not work at all on 64 bit big endian machines. If e.g. the array
> would contain only one 32 bit member set_bit() would write to whatever is
> behind the array.

Oh, it's much worse than that.

It's not that it would write beyond the array on big-endian, even if
it were to be _in_ the array it simply writes the wrong bit entirely.

And even on little-endian, it is complete and utter garbage.

On little-endian, it writes the right bit - except when it doesn't
work at all! The alignment may be wrong, and even on architectures
that handle unaligned loads and stores perfectly well, _atomic_
unaligned may not work at all.

x86 is actually unique in handling even unaligned atomics, and even on
x86 it's generally something you absolutely need to avoid for
performance reasons (most of the time it's fine, but atomics that
cross cachelines or pages can be very very expensive).

So that commit was beyond broken. It's dangerous and pure shit.

There's a very good reason our bitops are defined for "unsigned long
*", and why they do *not* take a "void *" or anything like that.

                        Linus