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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <bdce7d99-7f02-4667-acda-9ffc62c92af2@wanadoo.fr>
Date: Fri, 7 Mar 2025 18:58:08 +0900
From: Vincent Mailhol <mailhol.vincent@...adoo.fr>
To: David Laight <david.laight.linux@...il.com>
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 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.

> This means they should be generating the same values.
> I don't know the correct 'sizeof (int_type)' for the shift right of ~0.
> My suspicion is that a 32bit assembler used 32bit signed integers and a
> 64bit one 64bit signed integers (but a 32bit asm on a 64bit host might
> be 64bit).
> So the asm versions need to avoid the right shift and only do left shifts.
> 
> Which probably means they need to be enirely separate from the C versions.
> And then the C ones can have all the ULL() removed.

In this v5, I already have the ULL() removed from the non-uapi C
version. And we are left with two distinct variants:

  - the uapi C & asm
  - the non-uapi C (including fix width)

For the uapi ones, I do not think we can modify it without a risk of
breaking some random userland. At least, this is not a risk I will take.
And if we have to keep the __GENMASK() and __GENMASK_ULL(), then I would
rather just reuse these for the asm variant instead of splitting further
more and finding ourselves with three variants:

  - the uapi C
  - the asm
  - the non-uapi C (including fix width)

If __GENMASK() and __GENMASK_ULL() were not in the uapi, I would have
agreed with you.

If you believe that the risk of modifying the uapi GENMASK*() is low
enough, then you can submit a patch. But I will definitely not touch
these myself.


Yours sincerely,
Vincent Mailhol