diff mbox series

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

Message ID 20220924150715.247417-1-keescook@chromium.org (mailing list archive)
State Deferred
Commit 6cf1ab89c9e75ed8f3432563c212d688b6d3563b
Headers show
Series [v2] Compiler Attributes: Introduce __access_*() function attribute | expand

Commit Message

Kees Cook Sept. 24, 2022, 3:07 p.m. UTC
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(+)

Comments

Nathan Chancellor Sept. 25, 2022, 4:46 a.m. UTC | #1
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
>
Miguel Ojeda Sept. 25, 2022, 11:36 a.m. UTC | #2
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
Nathan Chancellor Sept. 25, 2022, 11:47 p.m. UTC | #3
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
diff mbox series

Patch

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
  */