linux-kernel - Re: linux/bitops.h

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, 4 May 2016 17:48:40 -0700
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	"H. Peter Anvin" <hpa@...or.com>
Cc:	John Denker <jsd@...n.com>, "Theodore Ts'o" <tytso@....edu>,
	noloader@...il.com,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Stephan Mueller <smueller@...onox.de>,
	Herbert Xu <herbert@...dor.apana.org.au>,
	Andi Kleen <andi@...stfloor.org>,
	Sandy Harris <sandyinchina@...il.com>,
	cryptography@...edaemon.net,
	Linux Crypto Mailing List <linux-crypto@...r.kernel.org>,
	Sasha Levin <sasha.levin@...cle.com>
Subject: Re: linux/bitops.h

On Wed, May 4, 2016 at 5:30 PM, H. Peter Anvin <hpa@...or.com> wrote:
>
> Yes. d7e35dfa is baloney IMNSHO.  All it does is produce worse code, and the
> description even says so.
>
> As I said, gcc has treated the former code as idiomatic since gcc 2, so that
> support is beyond ancient.

Well, we're *trying* to get clang supported, so the argument that gcc
has always supported it and compiles correct code for it is not
necessarily the whole story yet.

The problem with "32 - shift" is that it really could generate garbage
in situations where shift ends up being a constant zero..

That said, the fact that the other cases weren't changed
(rol64/ror64/ror32) does make that argument less interesting. Unless
there was some particular code that actively ended up using
"rol32(..0)" but not the other cases.

(And yes, rol32(x,0) is obviously nonsense, but it could easily happen
with inline functions or macros that end up generating that).

Note that there may be 8 "rol/ror" functions, but the 16-bit and 8-bit
ones don't have the undefined semantics. So there are only four that
had _that_ problem, although I agree that changing just one out of
four is wrong.

            Linus