Re: [GIT PULL] Crypto Update for 6.10

[Linus Torvalds <torvalds@linux-foundation.org>]

On Sun, 12 May 2024 at 20:50, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> Lukas Wunner (1):
>       X.509: Introduce scope-based x509_certificate allocation

I absolutely hate how this commit tries to remove one single compare
instruction by introducing a *very* dangerous hack.

The whole 'assume()' thing will generate actively wrong code if that
assumption conditional doesn't hold, to the point of being completely
impossible to debug.

Having random kernel code add random "assume()" lines is absolutely
not what we should do. Particularly not in some random code sequence
where it absolutely does not matter ONE WHIT.

Now, I've pulled this, but I killed that  "assume()" hackery in my merge.

Because there is no way we will ever encourage random code to make
these kinds of patterns, and I most definitely do not want anybody
else to try to copy that horrendous thing.

Yes, yes, we have "unreachable()" in other places, and yes, you can
make compilers generate garbage by using that incorrectly. But they
should be about obvious code warning issues, not about "let's save one
conditional instruction".

Now, if somebody really *really* cares about that one extraneous
conditional, particularly if it shows up in some more important place
than some random certificate parsing routine where is most definitely
is not in the least critical, there are better models for this
optimization.

Maybe somebody can teach the kernel build in *general* that
"kmalloc()" and friends never return an error pointer, only NULL or
success? That would not necessarily be a bad idea if the scope-based
cleanup otherwise causes issues.

But this kind of hacky "one random piece of kernel code uses a very
dangerous pattern to state that some *other* piece of kernel code has
particular return patterns" - that is not at all acceptable.

Put another way: it would probably be ok if the SLAB people added some
"this function cannot return error codes" annotation on their core
declaration and it fixed an issue in _general_.

But it is *not* ok if random kernel code starts randomly asserting the
same thing.

Quod licet Iovi, non licet bovi.

                 Linus