linux-kernel - Re: [PATCH v3 RESEND] uapi: Revert "bitops: avoid integer overflow in GENMASK(

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]

Message-ID: <20250302223811.1a54deec@pumpkin>
Date: Sun, 2 Mar 2025 22:38:11 +0000
From: David Laight <david.laight.linux@...il.com>
To: I Hsin Cheng <richard120310@...il.com>
Cc: yury.norov@...il.com, anshuman.khandual@....com, arnd@...db.de,
 linux-kernel@...r.kernel.org, jserv@...s.ncku.edu.tw,
 skhan@...uxfoundation.org, mka@...omium.org, akpm@...ux-foundation.org
Subject: Re: [PATCH v3 RESEND] uapi: Revert "bitops: avoid integer overflow
 in GENMASK(_ULL)"

On Wed, 26 Feb 2025 14:56:23 +0800
I Hsin Cheng <richard120310@...il.com> wrote:

> This patch reverts 'commit c32ee3d9abd2("bitops: avoid integer overflow in
>  GENMASK(_ULL)")'.
> 
> The code generation can be shrink by over 1KB by reverting this commit.
> Originally the commit claimed that clang would emit warnings using the
> implementation at that time.
...
> The results of code size reduction are summarized in the following table.
> The code size changes for clang are all zero across different versions,
> so they're not listed in the table.

I've been looking at the object changes.

I think all the big differences are due to the compiler changing the 'inline'
decision for some functions.
This is most obvious when the functions bloat-o-meter generates differ.
But I've seen odd things like seeing an inlined (IIRC) pud_val() immediately
followed by call to the wrapper (which is just an indirect call).
(The kernel I'm building is for the machine I'm building it on and has pretty
much all the mitigations compiled out for speed.)

I suspect the +1/-1 and extra negates all depend on subtleties in the compiler
and whether it is doing |= GENMASK() or &= ~GENMASK().
Some of the differences are also in for_each_set_bit() which used GENMASK()
with variables - plausibly it needs a specific implementation.

Sometimes you win, sometimes you lose.
But simplicity is a good win, and the current version is anything but.
(It is also broken for ASM because _UL(x) and _ULL(x) are both just (x)
so GENMASK() and GENMASK_ULL() can't both be right.)

	David