linux-kernel - Re: [PATCH v5 1/7] bits: split the definition of the asm and non-asm 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250309015853.01412484@pumpkin>
Date: Sun, 9 Mar 2025 01:58:53 +0000
From: David Laight <david.laight.linux@...il.com>
To: Vincent Mailhol <mailhol.vincent@...adoo.fr>
Cc: Yury Norov <yury.norov@...il.com>, Lucas De Marchi
 <lucas.demarchi@...el.com>, Rasmus Villemoes <linux@...musvillemoes.dk>,
 Jani Nikula <jani.nikula@...ux.intel.com>, Joonas Lahtinen
 <joonas.lahtinen@...ux.intel.com>, Rodrigo Vivi <rodrigo.vivi@...el.com>,
 Tvrtko Ursulin <tursulin@...ulin.net>, David Airlie <airlied@...il.com>,
 Simona Vetter <simona@...ll.ch>, Andrew Morton <akpm@...ux-foundation.org>,
 linux-kernel@...r.kernel.org, intel-gfx@...ts.freedesktop.org,
 dri-devel@...ts.freedesktop.org, Andi Shyti <andi.shyti@...ux.intel.com>,
 David Laight <David.Laight@...LAB.COM>, Dmitry Baryshkov
 <dmitry.baryshkov@...aro.org>, Andy Shevchenko
 <andriy.shevchenko@...ux.intel.com>
Subject: Re: [PATCH v5 1/7] bits: split the definition of the asm and
 non-asm GENMASK()

On Fri, 7 Mar 2025 18:58:08 +0900
Vincent Mailhol <mailhol.vincent@...adoo.fr> wrote:

> On 07/03/2025 at 04:23, David Laight wrote:
> > On Thu, 06 Mar 2025 20:29:52 +0900
> > Vincent Mailhol via B4 Relay <devnull+mailhol.vincent.wanadoo.fr@...nel.org> wrote:
> >   
> >> From: Vincent Mailhol <mailhol.vincent@...adoo.fr>
> >>
> >> In an upcoming change, GENMASK() and its friends will indirectly
> >> depend on sizeof() which is not available in asm.
> >>
> >> Instead of adding further complexity to __GENMASK() to make it work
> >> for both asm and non asm, just split the definition of the two
> >> variants.  
> > ...  
> >> +#else /* defined(__ASSEMBLY__) */
> >> +
> >> +#define GENMASK(h, l)		__GENMASK(h, l)
> >> +#define GENMASK_ULL(h, l)	__GENMASK_ULL(h, l)  
> > 
> > What do those actually expand to now?
> > As I've said a few times both UL(0) and ULL(0) are just (0) for __ASSEMBLY__
> > so the expansions of __GENMASK() and __GENMASK_ULL() contained the
> > same numeric constants.  
> 
> Indeed, in asm, the UL(0) and ULL(0) expands to the same thing: 0.
> 
> But the two macros still expand to something different on 32 bits
> architectures:
> 
>   * __GENMASK:
> 
>       (((~(0)) << (l)) & (~(0) >> (32 - 1 - (h))))
> 
>   * __GENMASK_ULL:
> 
>       (((~(0)) << (l)) & (~(0) >> (64 - 1 - (h))))
> 
> On 64 bits architecture these are the same.

I've just fed those into godbolt (https://www.godbolt.org/z/Ter6WE9qE) as:
int fi(void)
{
    int v;
    asm("mov $(((~(0)) << (8)) & (~(0) >> (32 - 1 - (15)))),%0": "=r" (v));
    return v -(((~(0u)) << (8)) & (~(0u) >> (32 - 1 - (15))));
}

gas warns:
<source>:4: Warning: 0xffffffffff00 shortened to 0xffffff00

unsigned long long fll(void)
{
    unsigned long long v;
    asm("mov $(((~(0)) << (8)) & (~(0) >> (64 - 1 - (15)))),%0": "=r" (v));
    return v -(((~(0ull)) << (8)) & (~(0ull) >> (64 - 1 - (15))));
}

(for other architectures you'll need to change the opcode)

For x86 and x86-32 the assembler seems to be doing 64bit maths with unsigned
right shifts - so the second function (with the 64 in it) generates 0xff00.
I doubt a 32bit only assembler does 64bit maths, but the '>> 48' above
might get masked to a '>> 16' by the cpu and generate the correct result.

So __GENMASK() is likely to be broken for any assembler that supports 64bits
when generating 32bit code.
__GENMASK_ULL() works (assuming all have unsigned >>) on 64bit assemblers
(even when generating 32bit code). It may work on some 32bit assemblers.

Since most uses in the header files will be GENMASK() I doubt (hope) no
asm code actually uses the values!
The headers assemble - but that is about all that can be said.

Bags of worms :-)

	David