linux-kernel - Re: [PATCH v2 1/8] minmax: Put all the clamp() definitions together

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: <CAHk-=wgLsFdNert_OfCmRon7Y9+ETnjxkz_UA5mv0=1RB71kww@mail.gmail.com>
Date: Tue, 30 Jul 2024 11:02:44 -0700
From: Linus Torvalds <torvalds@...uxfoundation.org>
To: Arnd Bergmann <arnd@...nel.org>
Cc: David Laight <David.Laight@...lab.com>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Jens Axboe <axboe@...nel.dk>, 
	Matthew Wilcox <willy@...radead.org>, Christoph Hellwig <hch@...radead.org>, 
	Andrew Morton <akpm@...ux-foundation.org>, 
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, 
	Dan Carpenter <dan.carpenter@...aro.org>, "Jason A . Donenfeld" <Jason@...c4.com>, 
	"pedro.falcato@...il.com" <pedro.falcato@...il.com>, Mateusz Guzik <mjguzik@...il.com>, 
	"linux-mm@...ck.org" <linux-mm@...ck.org>, Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Subject: Re: [PATCH v2 1/8] minmax: Put all the clamp() definitions together

On Tue, 30 Jul 2024 at 07:15, Arnd Bergmann <arnd@...nel.org> wrote:
>
> There is another one that I see with gcc-8 randconfigs (arm64):

So I ended up undoing that part of my patch, so it's a non-issue, but
I wanted to figure out what went wrong.

It's actually quite funny.

> net/netfilter/ipvs/ip_vs_conn.c:1498:8: note: in expansion of macro 'clamp'
>  1498 |  max = clamp(max, min, max_avail);

So it turns out that min is seen by the compiler to be constant (8).
And max_avail() uses order_base_2(), which expands to this huge
comparison table for the constant case.

Now, in the end all of those comparisons will go away, but I think
what happens is that because they exist at the source level, the
compiler ends up expanding them, and then - for exactly the same
reason as before - the compiler can find a path in that tree of
conditionals where "max_avail" does indeed end up being a constant
smaller than 8.

The fact that that path is then never taken, and pruned away later,
doesn't help us, and the compiletime_assert happens because in one
intermediate form the compiler had folded the now trivial 'clamp()' of
two constants in with the conditionals that get thrown away, and the
warning happens even when it's not reachable.

Anyway, it does mean that yeah, the much more stupid static_assert()
that will never try to follow any chain of expressions si the way to
go.

For the type checking, this isn't as much of an issue.

The whole "is that a non-negative expression" also uses the same
machinery, but if the expression could be negative in some situation
(that goes away in the end) at worst it just means that the new rules
won't be as relaxed as they could be when the expressions get
sufficiently complicated.

                Linus