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

[Nick Desaulniers <ndesaulniers@google.com>]

On Fri, Sep 10, 2021 at 1:47 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Fri, Sep 10, 2021 at 01:16:00PM -0700, Linus Torvalds wrote:
> > 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.
>
> I'm going to eventually forget this thread, so I want to get it into
> our coding style so I can find it again more easily. :) How does this
> look?
>
> diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst
> index 42969ab37b34..3c72f0232f02 100644
> --- a/Documentation/process/coding-style.rst
> +++ b/Documentation/process/coding-style.rst
> @@ -487,6 +487,29 @@ because it is a simple way to add valuable information for the reader.
>  Do not use the ``extern`` keyword with function prototypes as this makes
>  lines longer and isn't strictly necessary.
>
> +.. code-block:: c
> +
> +       static __always_inline __must_check void *action(enum magic value,
> +                                                        size_t size, u8 count,
> +                                                        char *buffer)
> +                                                       __alloc_size(2, 3)
> +       {
> +               ...
> +       }
> +
> +When writing a function prototype, keep the order of elements regular. The
> +desired order is "storage class", "return type attributes", "return
> +type", name, arguments (as described earlier), followed by "function
> +argument attributes". In the ``action`` function example above, ``static
> +__always_inline`` is the "storage class" (even though ``__always_inline``
> +is an attribute, it is treated like ``inline``). ``__must_check`` is

eh...mentioning inline as though it was a storage class doesn't seem
precise, but I think this is a good start. Thanks Kees.

Acked-by: Nick Desaulniers <ndesaulniers@google.com>

Worst case, consider "inlining related attributes like __always_inline
and noinline should follow the storage class (static, extern).

> +a "return type attribute" (describing ``void *``). ``void *`` is the
> +"return type". ``action`` is the function name, followed by the function
> +arguments. Finally ``__alloc_size(2,3)`` is an "function argument attribute",
> +describing things about the function arguments. Some attributes, like
> +``__malloc``, describe the behavior of the function more than they
> +describe the function return type, and are more appropriately included
> +in the "function argument attributes".
>
>  7) Centralized exiting of functions
>  -----------------------------------
>
> --
> Kees Cook



-- 
Thanks,
~Nick Desaulniers