[PATCH v2] Compiler Attributes: Introduce __access_*() function attribute

[PATCH v2] Compiler Attributes: Introduce __access_*() function attribute

[Kees Cook]

Added in GCC 10.1, the "access" function attribute is used to mark pointer
arguments for how they are expected to be accessed in a given function.
Both their access type (read/write, read-only, or write-only) and bounds
are specified.

These can improve GCC's compile-time diagnostics including -Warray-bounds,
-Wstringop-overflow, etc, and can affect __builtin_dynamic_object_size()
results.

Cc: Miguel Ojeda <ojeda@kernel.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Tom Rix <trix@redhat.com>
Cc: llvm@lists.linux.dev
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/compiler_attributes.h | 30 +++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
index 9a9907fad6fd..465be5f072ff 100644
--- a/include/linux/compiler_attributes.h
+++ b/include/linux/compiler_attributes.h
@@ -20,6 +20,36 @@
  * Provide links to the documentation of each supported compiler, if it exists.
  */
 
+/*
+ * Optional: only supported since gcc >= 10
+ * Optional: not supported by Clang
+ *
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-access-function-attribute
+ *
+ * While it is legal to provide only the pointer argument position and
+ * access type, the kernel macros are designed to require also the bounds
+ * (element count) argument position; if a function has no bounds argument,
+ * refactor the code to include one.
+ *
+ * These can be used multiple times. For example:
+ *
+ * __access_wo(2, 3) __access_ro(4, 5)
+ * int copy_something(struct context *ctx, u32 *dst, size_t dst_count,
+ *		      const u8 *src, int src_len);
+ *
+ * If "dst" will also be read from, it could use __access_rw(2, 3) instead.
+ *
+ */
+#if __has_attribute(__access__)
+# define __access_rw(ptr, count)	__attribute__((__access__(read_write, ptr, count)))
+# define __access_ro(ptr, count)	__attribute__((__access__(read_only,  ptr, count)))
+# define __access_wo(ptr, count)	__attribute__((__access__(write_only, ptr, count)))
+#else
+# define __access_rw(ptr, count)
+# define __access_ro(ptr, count)
+# define __access_wo(ptr, count)
+#endif
+
 /*
  *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-alias-function-attribute
  */
-- 
2.34.1

Re: [PATCH v2] Compiler Attributes: Introduce __access_*() function attribute

[Nathan Chancellor]

On Sat, Sep 24, 2022 at 08:07:15AM -0700, Kees Cook wrote:
> Added in GCC 10.1, the "access" function attribute is used to mark pointer
> arguments for how they are expected to be accessed in a given function.
> Both their access type (read/write, read-only, or write-only) and bounds
> are specified.
> 
> These can improve GCC's compile-time diagnostics including -Warray-bounds,
> -Wstringop-overflow, etc, and can affect __builtin_dynamic_object_size()
> results.
> 
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Tom Rix <trix@redhat.com>
> Cc: llvm@lists.linux.dev
> Signed-off-by: Kees Cook <keescook@chromium.org>

The GCC docs say it is 'access', instead of '__access__'. I assume it is
probably okay so:

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  include/linux/compiler_attributes.h | 30 +++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> index 9a9907fad6fd..465be5f072ff 100644
> --- a/include/linux/compiler_attributes.h
> +++ b/include/linux/compiler_attributes.h
> @@ -20,6 +20,36 @@
>   * Provide links to the documentation of each supported compiler, if it exists.
>   */
>  
> +/*
> + * Optional: only supported since gcc >= 10
> + * Optional: not supported by Clang
> + *
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-access-function-attribute
> + *
> + * While it is legal to provide only the pointer argument position and
> + * access type, the kernel macros are designed to require also the bounds
> + * (element count) argument position; if a function has no bounds argument,
> + * refactor the code to include one.
> + *
> + * These can be used multiple times. For example:
> + *
> + * __access_wo(2, 3) __access_ro(4, 5)
> + * int copy_something(struct context *ctx, u32 *dst, size_t dst_count,
> + *		      const u8 *src, int src_len);
> + *
> + * If "dst" will also be read from, it could use __access_rw(2, 3) instead.
> + *
> + */
> +#if __has_attribute(__access__)
> +# define __access_rw(ptr, count)	__attribute__((__access__(read_write, ptr, count)))
> +# define __access_ro(ptr, count)	__attribute__((__access__(read_only,  ptr, count)))
> +# define __access_wo(ptr, count)	__attribute__((__access__(write_only, ptr, count)))
> +#else
> +# define __access_rw(ptr, count)
> +# define __access_ro(ptr, count)
> +# define __access_wo(ptr, count)
> +#endif
> +
>  /*
>   *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-alias-function-attribute
>   */
> -- 
> 2.34.1
> 

Re: [PATCH v2] Compiler Attributes: Introduce __access_*() function attribute

[Miguel Ojeda]

On Sun, Sep 25, 2022 at 6:46 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> The GCC docs say it is 'access', instead of '__access__'. I assume it is
> probably okay so:

Yeah, attributes can be used either with or without double
underscores, see
https://gcc.gnu.org/onlinedocs/gcc/Attribute-Syntax.html#Attribute-Syntax.

In this file, we always use the underscore ones to minimize the chance
of collisions, since the idea is that the rest of the kernel uses the
shorthands provided here.

Cheers,
Miguel

Re: [PATCH v2] Compiler Attributes: Introduce __access_*() function attribute

[Nathan Chancellor]

On Sun, Sep 25, 2022 at 01:36:22PM +0200, Miguel Ojeda wrote:
> On Sun, Sep 25, 2022 at 6:46 AM Nathan Chancellor <nathan@kernel.org> wrote:
> >
> > The GCC docs say it is 'access', instead of '__access__'. I assume it is
> > probably okay so:
> 
> Yeah, attributes can be used either with or without double
> underscores, see
> https://gcc.gnu.org/onlinedocs/gcc/Attribute-Syntax.html#Attribute-Syntax.

Aha, I figured I was missing something. That is what I get for doing a
mobile review :)

> In this file, we always use the underscore ones to minimize the chance
> of collisions, since the idea is that the rest of the kernel uses the
> shorthands provided here.

That certainly makes sense, I think it is a little easier to read too,
as it gives the attribute name a little separating from '__attribute__'
and any arguments to it.

Cheers,
Nathan