Re: [patch 9/9] mm/vmalloc: add __alloc_size attributes for better bounds checking

From: Linus Torvalds <torvalds@linux-foundation.org>
To: Nick Desaulniers <ndesaulniers@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	apw@canonical.com, Christoph Lameter <cl@linux.com>,
	Daniel Micay <danielmicay@gmail.com>,
	Dennis Zhou <dennis@kernel.org>,
	dwaipayanray1@gmail.com, Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Joe Perches <joe@perches.com>, Kees Cook <keescook@chromium.org>,
	Linux-MM <linux-mm@kvack.org>,
	Lukas Bulwahn <lukas.bulwahn@gmail.com>,
	mm-commits@vger.kernel.org, Nathan Chancellor <nathan@kernel.org>,
	Miguel Ojeda <ojeda@kernel.org>,
	Pekka Enberg <penberg@kernel.org>,
	David Rientjes <rientjes@google.com>, Tejun Heo <tj@kernel.org>,
	Vlastimil Babka <vbabka@suse.cz>
Subject: Re: [patch 9/9] mm/vmalloc: add __alloc_size attributes for better bounds checking
Date: Fri, 10 Sep 2021 13:16:00 -0700	[thread overview]
Message-ID: <CAHk-=wiOCLRny5aifWNhr621kYrJwhfURsa0vFPeUEm8mF0ufg@mail.gmail.com> (raw)
In-Reply-To: <CAKwvOd=quUcE8UBcwD6Hjs40LNKodHEwAfqt=_OjNdwYM19LOA@mail.gmail.com>

On Fri, Sep 10, 2021 at 12:49 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> Huh?
> $ grep -rn __always_inline
> $ grep -rn noinline
> $ grep -rn __cold

Those are all examples of basically storage classes.

And yes, they go to the front - generally even before the return
value. Exactly like 'extern' and 'static'.

The In contrast, the "function argument descriptions" have
traditionally gone at the end, although you can certainly finds us
screwing that up too.

This, btw, tends to be how the compiler documentation also does it.

So to a close approximation

 - "storage class" goes first, so "static inline" etc.

 - return type next (including attributes directly related to the
returned value - like "__must_check")

 - then function name and argument declaration

 - and finally the "function argument type attributes" at the end.

can you do it in different orders? Yes. And the compiler won't even
generally warn about it. So we've gotten it wrong many many times.

I mean, compilers won't complain even about clear garbage that is _so_
bad that we generally get it right:

  int static myfn(void);

will build perfectly fine. That most certainly doesn't make it right.

Arguably "__malloc" could be seen about the returned type, rather than
being about the function declarations. But if it was about the
returned type, you'd call it a "restrict" pointer, so the very name
kind of implies that it's about the behaviot of the _function_ more
than the type of the return value.

And the "enumerate the arguments" of __alloc_size() makes it 100%
clear that it has absolutely NOTHING to do with the return type, and
is all about the function itself and which arguments give the size.

So the attribute goes at the end, not the front.

Are these a bit arbitrary? Sure. And because it's not checked, it's
not consistent. But I can only repeat: this is literally how the
compiler docs themselves tend to order things, pointed to in the very
patches that are under discussion.

So the rules may be arbitrary, but they are at least _somewhat_
internally consistent. Yes, you can always argue about whether some
behavior is about the returned type or whether it is about the
semantics of the function.

         Linus