linux-kernel - Re: [PATCH] x86, nospec: Fix uninitialized mask in array_index_mask

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

Message-ID: <CA+55aFxe2Q0vJ5nz-nRvf8n61ZWG2ak2jt=JK_3VFsFbiFS_XQ@mail.gmail.com>
Date:   Wed, 14 Feb 2018 13:04:11 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Dan Williams <dan.j.williams@...el.com>
Cc:     Ingo Molnar <mingo@...nel.org>, Andi Kleen <ak@...ux.intel.com>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH] x86, nospec: Fix uninitialized mask in array_index_mask_nospec()

On Wed, Feb 14, 2018 at 12:50 PM, Dan Williams <dan.j.williams@...el.com> wrote:
> Break the false dependency on the uninitialized value of 'mask' in
> array_index_mask_nospec(). This potentially allows the CPU to rename the
> register, avoids reports from tools that can detect uninitialized inline
> asm arguments, and makes it explicit that this sequence wants the result
> of (0 - 0 - CARRY).

NAK.

This is stupid.

Any asm tools that think this is "uninitialized inline asm arguments"
are complete garbage. It's an output-only register. Fix the tool, or
ignore it as shit.

And that false dependency argument is pure garbage too. Making this an
input that has been initialized to zero doesn't just generate a
pointless zeroing operation, it also adds to register pressure.

So NAK NAK NAK. The arguments for this patch are pure and utter shite.

If somebody can actually show a performance reason (with NUMBERS!) for
it or some _real_ reason to do something this stupid, that's one
thing. But these arguments are just bad, and the patch is bad.

              Linus