linux-kernel - Re: [PATCH treewide v2 1/3] bitfield: Add non-constant field

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: <45920591-e1d6-4337-a906-35bb5319836c@wanadoo.fr>
Date: Mon, 3 Feb 2025 22:36:43 +0900
From: Vincent Mailhol <mailhol.vincent@...adoo.fr>
To: Johannes Berg <johannes@...solutions.net>,
 Yury Norov <yury.norov@...il.com>
Cc: Geert Uytterhoeven <geert+renesas@...der.be>, linux-clk@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-renesas-soc@...r.kernel.org,
 linux-crypto@...r.kernel.org, qat-linux@...el.com,
 linux-gpio@...r.kernel.org, linux-aspeed@...ts.ozlabs.org,
 linux-iio@...r.kernel.org, linux-sound@...r.kernel.org,
 linux-kernel@...r.kernel.org, Michael Turquette <mturquette@...libre.com>,
 Stephen Boyd <sboyd@...nel.org>, Nicolas Ferre
 <nicolas.ferre@...rochip.com>,
 Alexandre Belloni <alexandre.belloni@...tlin.com>,
 Claudiu Beznea <claudiu.beznea@...on.dev>,
 Giovanni Cabiddu <giovanni.cabiddu@...el.com>,
 Herbert Xu <herbert@...dor.apana.org.au>,
 "David S . Miller" <davem@...emloft.net>,
 Linus Walleij <linus.walleij@...aro.org>, Bartosz Golaszewski
 <brgl@...ev.pl>, Joel Stanley <joel@....id.au>,
 Andrew Jeffery <andrew@...econstruct.com.au>, Crt Mori <cmo@...exis.com>,
 Jonathan Cameron <jic23@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>,
 Jacky Huang <ychuang3@...oton.com>, Shan-Chun Hung <schung@...oton.com>,
 Rasmus Villemoes <linux@...musvillemoes.dk>, Jaroslav Kysela
 <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
 Jakub Kicinski <kuba@...nel.org>, Alex Elder <elder@...e.org>
Subject: Re: [PATCH treewide v2 1/3] bitfield: Add non-constant
 field_{prep,get}() helpers

On 03/02/2025 at 16:44, Johannes Berg wrote:
> On Sun, 2025-02-02 at 12:53 -0500, Yury Norov wrote:
>>
>>> Instead of creating another variant for
>>> non-constant bitfields, wouldn't it be better to make the existing macro
>>> accept both?
>>
>> Yes, it would definitely be better IMO.
> 
> On the flip side, there have been discussions in the past (though I
> think not all, if any, on the list(s)) about the argument order. Since
> the value is typically not a constant, requiring the mask to be a
> constant has ensured that the argument order isn't as easily mixed up as
> otherwise.

If this is a concern, then it can be checked with:

  BUILD_BUG_ON_MSG(!__builtin_constant_p(_mask) &&
                   __builtin_constant_p(_val),
                   _pfx "mask is not constant");

It means that we forbid FIELD_PREP(non_const_mask, const_val) but allow
any other combination.

> With a non-constant mask there can also be no validation that the mask
> is contiguous etc.
> 
> Now that doesn't imply a strong objection - personally I've come to
> prefer the lower-case typed versions anyway - but something to keep in
> mind when doing this.
> 
> However, the suggested change to BUILD_BUG_ON_NOT_POWER_OF_2 almost
> certainly shouldn't be done for the same reason - not compiling for non-
> constant values is [IMHO] part of the API contract for that macro. This
> can be important for the same reasons.

Your point is fair enough. But I do not see this as a killer argument.
We can instead just add below helper:

  BUILD_BUG_ON_STATICALLY_NOT_POWER_OF_2()

But, for the same reason why I would rather not have both the
FIELD_{PREP,GET}() and the field_{prep,get}(), I would also rather not
have a BUILD_BUG_ON_NOT_POWER_OF_2() and a
BUILD_BUG_ON_STATICALLY_NOT_POWER_OF_2().

If your concern is the wording of the contract, the description can just
be updated.

Yours sincerely,
Vincent Mailhol