Re: [PATCH 34/82] ipc: Refactor intentional wrap-around calculation

[Linus Torvalds]

On Mon, 22 Jan 2024 at 16:46, Kees Cook <keescook@xxxxxxxxxxxx> wrote:
>
> Refactor open-coded unsigned wrap-around addition test to use
> check_add_overflow(),

NAK.

First off, none of this has anything to do with -fno-strict-overflow.
We do that, because without it gcc ends up doing various odd and
surprising things, the same way it does with strict-aliasing.

IOW, you should think of -fno-strict-overflow as a hardening thing.
Any optimization that depends on "this can overflow, so I can do
anything I want" is just a dangerous optimization for the kernel.

It matches -fno-strict-aliasing and -fno-delete-null-pointer-checks,
in other words.

And I do not understand why you mention it in the first place, since
this code USES UNSIGNED INTEGER ARITHMETIC, and thus has absolutely
nothing to do with that no-strict-overflow flag.

So the commit message is actively misleading and broken. Unsigned
arithmetic has very well-defined behavior, and the code uses that with
a very traditional and valid test.

The comment about "redundant open-coded addition" is also PURE
GARBAGE, since the compiler will trivially do the CSE - and on the
source code level your modified code is actively bigger and uglier.

So your patch improves neither code generation or source code.

And if there's some unsigned wrap-around checker that doesn't
understand this traditional way of doing overflow checking, that piece
of crap needs fixing.

I don't want to see mindless conversion patches that work around some
broken tooling.

I want to see them even less when pretty much EVERY SINGLE WORD in the
commit message seems to be actively misleading and irrelevant garbage.

Stop making the world a worse place.

                 Linus