| 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
| ||
|
Message-ID: <c617483816b54096ba4b30bea595da49@AcuMS.aculab.com>
Date: Wed, 4 Dec 2024 18:48:38 +0000
From: David Laight <David.Laight@...LAB.COM>
To: Vincent Mailhol <mailhol.vincent@...adoo.fr>, Linus Torvalds
<torvalds@...ux-foundation.org>, Luc Van Oostenryck
<luc.vanoostenryck@...il.com>, Nathan Chancellor <nathan@...nel.org>, "Nick
Desaulniers" <ndesaulniers@...gle.com>, Bill Wendling <morbo@...gle.com>,
Justin Stitt <justinstitt@...gle.com>, Yury Norov <yury.norov@...il.com>,
Rasmus Villemoes <linux@...musvillemoes.dk>, Kees Cook <kees@...nel.org>,
"Gustavo A. R. Silva" <gustavoars@...nel.org>, 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>, Suzuki K Poulose <suzuki.poulose@....com>,
Mike Leach <mike.leach@...aro.org>, James Clark <james.clark@...aro.org>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Rikard Falkeborn
<rikard.falkeborn@...il.com>, Martin Uecker
<Martin.Uecker@....uni-goettingen.de>
CC: "linux-sparse@...r.kernel.org" <linux-sparse@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"llvm@...ts.linux.dev" <llvm@...ts.linux.dev>,
"linux-hardening@...r.kernel.org" <linux-hardening@...r.kernel.org>,
"intel-gfx@...ts.freedesktop.org" <intel-gfx@...ts.freedesktop.org>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
"coresight@...ts.linaro.org" <coresight@...ts.linaro.org>,
"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>
Subject: RE: [PATCH 03/10] compiler.h: add is_const_true() and
is_const_false()
From: Vincent Mailhol
> Sent: 02 December 2024 17:33
>
> __builtin_constant_p() is known for not always being able to produce
> constant expression [1] which led to the introduction of
> __is_constexpr() [2]. Because of its dependency on
> __builtin_constant_p(), statically_true() suffers from the same
> issues.
No, they are testing different things.
>
> For example:
>
> void foo(int a)
> {
> /* fail on GCC */
> BUILD_BUG_ON_ZERO(statically_true(a));
>
> /* fail on both clang and GCC */
> static char arr[statically_true(a) ? 1 : 2];
> }
>
> Define a new is_const_true() and is_const_false() pair of macros
> which, by making use of __is_const_zero(), always produces a constant
> expression.
>
> Note that is_const_false() can not be directly defined as an alias to
> __is_const_zero(). Otherwise, it could yield some false positives on
> huge numbers because of a lost of precision when doing the (long) cast
> in __is_const_zero(). Example:
>
> is_const_false((u128)ULONG_MAX << BITS_PER_LONG)
>
> Furthermore, using the ! operator like this:
>
> #define is_const_true(x) __is_const_zero(!(x))
> #define is_const_false(x) __is_const_zero(!!(x))
>
> would yield a -Wint-in-bool-context compiler warning if the argument
> is not a boolean. Use the == and != operators instead.
>
> It should be noted that statically_true/false() are the only ones
> capable of folding tautologic expressions in which at least one on the
> operands is not a constant expression. For example:
>
> statically_true(true || var)
> statically_true(var == var)
> statically_false(var * 0)
> statically_false(var * 8 % 4)
>
> always evaluate to true, whereas all of these would be false under
> is_const_true/false() if var is not a constant expression [3].
>
> For this reason, usage of const_true/false() should be the exception.
> Reflect in the documentation that const_true() is less powerful and
> that statically_true() is the overall preferred solution.
>
> [1] __builtin_constant_p cannot resolve to const when optimizing
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=19449
>
> [2] commit 3c8ba0d61d04 ("kernel.h: Retain constant expression output for max()/min()")
> Link: https://git.kernel.org/torvalds/c/3c8ba0d61d04
>
> [3] https://godbolt.org/z/E4r7EaxW9
>
> Signed-off-by: Vincent Mailhol <mailhol.vincent@...adoo.fr>
> ---
> include/linux/compiler.h | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index 30ce06df4153cfdc0fad9bc7bffab9097f8b0450..165aa5b9bc484376087a130a1ac1f3edb50c983d 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -357,6 +357,29 @@ static inline void *offset_to_ptr(const int *off)
> */
> #define is_const(x) __is_const_zero(0 * (x))
>
> +/*
> + * Similar to statically_true() but produces a constant expression
No.
It tests whether a value is a 'constant integer expression' and
the result is a 'constant integer expression'.
statically_true() checks for the value being a 'compile time constant'.
Most code really doesn't care, it all got added to min() so that
a very few places could do:
char foo[min(16, sizeof (type))];
without triggering the 'variable length array' warning.
But that just bloated everywhere else and (IIRC) Linus replaced
them with a MIN() that was just an expression.
> + *
> + * To be used in conjunction with macros, such as BUILD_BUG_ON_ZERO(),
> + * which require their input to be a constant expression and for which
> + * statically_true() would otherwise fail.
Use a different BUILD_BUG macro instead.
Look at the current definition of min().
David
> + *
> + * This is a trade-off: is_const_true() requires all its operands to
> + * be compile time constants. Else, it would always returns false even
> + * on the most trivial cases like:
> + *
> + * true || non_const_expr
> + *
> + * On the opposite, statically_true() is able to fold more complex
> + * tautologies and will return true on expressions such as:
> + *
> + * !(non_const_expr * 8 % 4)
> + *
> + * For the general case, statically_true() is better.
> + */
> +#define is_const_true(x) __is_const_zero((x) == 0)
> +#define is_const_false(x) __is_const_zero((x) != 0)
> +
> /*
> * This is needed in functions which generate the stack canary, see
> * arch/x86/kernel/smpboot.c::start_secondary() for an example.
>
> --
> 2.45.2
>
>
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Powered by blists - more mailing lists