Re: [RFC] Mitigating unexpected arithmetic overflow

From: Linus Torvalds <torvalds@linux-foundation.org>
To: Kees Cook <keescook@chromium.org>
Cc: Justin Stitt <justinstitt@google.com>,
	Peter Zijlstra <peterz@infradead.org>,
	 Mark Rutland <mark.rutland@arm.com>,
	linux-hardening@vger.kernel.org,  linux-kernel@vger.kernel.org,
	llvm@lists.linux.dev
Subject: Re: [RFC] Mitigating unexpected arithmetic overflow
Date: Wed, 8 May 2024 10:52:44 -0700	[thread overview]
Message-ID: <CAHk-=wi5YPwWA8f5RAf_Hi8iL0NhGJeL6MN6UFWwRMY8L6UDvQ@mail.gmail.com> (raw)
In-Reply-To: <202404291502.612E0A10@keescook>

On Tue, 7 May 2024 at 16:27, Kees Cook <keescook@chromium.org> wrote:
>
> When I say "overflow", I mean "overflow and underflow", but more
> specifically I mean "wrap-around". This is not about "undefined
> behavior". We already demand from our compilers that all our arithmetic
> uses a well-defined overflow resolution strategy; overflow results in
> wrap-around (thanks to "-fno-strict-overflow").

I'm still entirely unconvinced.

The thing is, wrap-around is not only well-defined, it's *common*, and
*EXPECTED*.

Example:

   static inline u32 __hash_32_generic(u32 val)
   {
        return val * GOLDEN_RATIO_32;
   }

and dammit, I absolutely DO NOT THINK we should annotate this as some
kind of "special multiply".

I have no idea how many of these exist. But I am 100% convinced that
making humans annotate this and making the source code worse is
absolutely the wrong approach.

Basically, unsigned arithmetic is well-defined as wrapping around, and
it is so for a good reason.

So I'm going to have a HARD REQUIREMENT that any compiler complaints
need to be really really sane. They need to detect when people do
things like this on purpose, and they need to SHUT THE ^&% UP about
the fact that wrap-around happens.

Any tool that is so stupid as to complain about wrap-around in the
above is a BROKEN TOOL THAT NEEDS TO BE IGNORED.

Really. This is non-negotiable.

This is similar to the whole

        unsigned int a, b;

        if (a+b < a) ..

kind of pattern: a tool that complains about wrap-around in the above
is absolutely broken sh*t and needs to be ignored.

So I think you need to limit your wrap-around complaints, and really
think about how to limit them. If you go "wrap-around is wrong" as
some kind of general; rule, I'm going to ignore you, and I'm going to
tell people to ignore you, and refuse any idiotic patches that are the
result of such idiotic rules.

Put another way the absolute first and fundamental thing you should
look at is to make sure tools don't complain about sane behavior.

Until you have done that, and taken this seriously, this discussion is
not going to ever result in anything productive.

                    Linus