* Re: [PATCH] Compiler Attributes: counted_by: Adjust name and identifier expansion
2023-08-17 20:06 [PATCH] Compiler Attributes: counted_by: Adjust name and identifier expansion Kees Cook
@ 2023-08-17 20:13 ` Nathan Chancellor
2023-08-17 21:00 ` Kees Cook
2023-08-17 22:06 ` Miguel Ojeda
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Nathan Chancellor @ 2023-08-17 20:13 UTC (permalink / raw)
To: Kees Cook
Cc: Miguel Ojeda, Nick Desaulniers, Tom Rix, Gustavo A. R. Silva,
linux-kernel, llvm, linux-hardening
On Thu, Aug 17, 2023 at 01:06:03PM -0700, Kees Cook wrote:
> GCC and Clang's current RFCs name this attribute "counted_by", and have
> moved away from using a string for the member name. Update the kernel's
> macros to match. Additionally provide a UAPI no-op macro for UAPI structs
> that will gain annotations.
>
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Fixes: dd06e72e68bc ("Compiler Attributes: Add __counted_by macro")
> Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
One comment below.
> ---
> include/linux/compiler_attributes.h | 26 +++++++++++++-------------
> include/uapi/linux/stddef.h | 4 ++++
> 2 files changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> index 00efa35c350f..74716a407aac 100644
> --- a/include/linux/compiler_attributes.h
> +++ b/include/linux/compiler_attributes.h
> @@ -94,6 +94,19 @@
> # define __copy(symbol)
> #endif
>
> +/*
> + * Optional: only supported since gcc >= 14
> + * Optional: only supported since clang >= 17
Since you are in the area, should the supported clang version be
optimistically updated to '18'? 17.x has already branched and I suspect
it would be too late in the release cycle to cherry-pick that change
from main if it were landed today (which seems pretty unlikely, given
there do not appear to be many reviews from high profile clang folks
yet).
> + *
> + * gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896
> + * clang: https://reviews.llvm.org/D148381
> + */
> +#if __has_attribute(__counted_by__)
> +# define __counted_by(member) __attribute__((__counted_by__(member)))
> +#else
> +# define __counted_by(member)
> +#endif
> +
> /*
> * Optional: not supported by gcc
> * Optional: only supported since clang >= 14.0
> @@ -129,19 +142,6 @@
> # define __designated_init
> #endif
>
> -/*
> - * Optional: only supported since gcc >= 14
> - * Optional: only supported since clang >= 17
> - *
> - * gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896
> - * clang: https://reviews.llvm.org/D148381
> - */
> -#if __has_attribute(__element_count__)
> -# define __counted_by(member) __attribute__((__element_count__(#member)))
> -#else
> -# define __counted_by(member)
> -#endif
> -
> /*
> * Optional: only supported since clang >= 14.0
> *
> diff --git a/include/uapi/linux/stddef.h b/include/uapi/linux/stddef.h
> index 7837ba4fe728..7c3fc3980881 100644
> --- a/include/uapi/linux/stddef.h
> +++ b/include/uapi/linux/stddef.h
> @@ -45,3 +45,7 @@
> TYPE NAME[]; \
> }
> #endif
> +
> +#ifndef __counted_by
> +#define __counted_by(m)
> +#endif
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] Compiler Attributes: counted_by: Adjust name and identifier expansion
2023-08-17 20:13 ` Nathan Chancellor
@ 2023-08-17 21:00 ` Kees Cook
0 siblings, 0 replies; 7+ messages in thread
From: Kees Cook @ 2023-08-17 21:00 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Miguel Ojeda, Nick Desaulniers, Tom Rix, Gustavo A. R. Silva,
linux-kernel, llvm, linux-hardening
On Thu, Aug 17, 2023 at 01:13:04PM -0700, Nathan Chancellor wrote:
> On Thu, Aug 17, 2023 at 01:06:03PM -0700, Kees Cook wrote:
> > GCC and Clang's current RFCs name this attribute "counted_by", and have
> > moved away from using a string for the member name. Update the kernel's
> > macros to match. Additionally provide a UAPI no-op macro for UAPI structs
> > that will gain annotations.
> >
> > Cc: Miguel Ojeda <ojeda@kernel.org>
> > Cc: Nick Desaulniers <ndesaulniers@google.com>
> > Fixes: dd06e72e68bc ("Compiler Attributes: Add __counted_by macro")
> > Signed-off-by: Kees Cook <keescook@chromium.org>
>
> Reviewed-by: Nathan Chancellor <nathan@kernel.org>
>
> One comment below.
>
> > ---
> > include/linux/compiler_attributes.h | 26 +++++++++++++-------------
> > include/uapi/linux/stddef.h | 4 ++++
> > 2 files changed, 17 insertions(+), 13 deletions(-)
> >
> > diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> > index 00efa35c350f..74716a407aac 100644
> > --- a/include/linux/compiler_attributes.h
> > +++ b/include/linux/compiler_attributes.h
> > @@ -94,6 +94,19 @@
> > # define __copy(symbol)
> > #endif
> >
> > +/*
> > + * Optional: only supported since gcc >= 14
> > + * Optional: only supported since clang >= 17
>
> Since you are in the area, should the supported clang version be
> optimistically updated to '18'? 17.x has already branched and I suspect
> it would be too late in the release cycle to cherry-pick that change
> from main if it were landed today (which seems pretty unlikely, given
> there do not appear to be many reviews from high profile clang folks
> yet).
Oops, yes. Excellent point. I'll bump this to 18.
-Kees
>
> > + *
> > + * gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896
> > + * clang: https://reviews.llvm.org/D148381
> > + */
> > +#if __has_attribute(__counted_by__)
> > +# define __counted_by(member) __attribute__((__counted_by__(member)))
> > +#else
> > +# define __counted_by(member)
> > +#endif
> > +
> > /*
> > * Optional: not supported by gcc
> > * Optional: only supported since clang >= 14.0
> > @@ -129,19 +142,6 @@
> > # define __designated_init
> > #endif
> >
> > -/*
> > - * Optional: only supported since gcc >= 14
> > - * Optional: only supported since clang >= 17
> > - *
> > - * gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896
> > - * clang: https://reviews.llvm.org/D148381
> > - */
> > -#if __has_attribute(__element_count__)
> > -# define __counted_by(member) __attribute__((__element_count__(#member)))
> > -#else
> > -# define __counted_by(member)
> > -#endif
> > -
> > /*
> > * Optional: only supported since clang >= 14.0
> > *
> > diff --git a/include/uapi/linux/stddef.h b/include/uapi/linux/stddef.h
> > index 7837ba4fe728..7c3fc3980881 100644
> > --- a/include/uapi/linux/stddef.h
> > +++ b/include/uapi/linux/stddef.h
> > @@ -45,3 +45,7 @@
> > TYPE NAME[]; \
> > }
> > #endif
> > +
> > +#ifndef __counted_by
> > +#define __counted_by(m)
> > +#endif
> > --
> > 2.34.1
> >
--
Kees Cook
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Compiler Attributes: counted_by: Adjust name and identifier expansion
2023-08-17 20:06 [PATCH] Compiler Attributes: counted_by: Adjust name and identifier expansion Kees Cook
2023-08-17 20:13 ` Nathan Chancellor
@ 2023-08-17 22:06 ` Miguel Ojeda
2023-08-17 23:45 ` Kees Cook
2023-08-18 3:42 ` Gustavo A. R. Silva
2023-08-18 17:58 ` Kees Cook
3 siblings, 1 reply; 7+ messages in thread
From: Miguel Ojeda @ 2023-08-17 22:06 UTC (permalink / raw)
To: Kees Cook
Cc: Miguel Ojeda, Nick Desaulniers, Nathan Chancellor, Tom Rix,
Gustavo A. R. Silva, linux-kernel, llvm, linux-hardening
On Thu, Aug 17, 2023 at 10:06 PM Kees Cook <keescook@chromium.org> wrote:
>
> GCC and Clang's current RFCs name this attribute "counted_by", and have
> moved away from using a string for the member name. Update the kernel's
> macros to match. Additionally provide a UAPI no-op macro for UAPI structs
> that will gain annotations.
>
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Fixes: dd06e72e68bc ("Compiler Attributes: Add __counted_by macro")
> Signed-off-by: Kees Cook <keescook@chromium.org>
Looks good to me, thanks. Perhaps we should split the UAPI bit into a
second patch.
I can take it into my tree; otherwise if you need this elsewhere:
Acked-by: Miguel Ojeda <ojeda@kernel.org>
Cheers,
Miguel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] Compiler Attributes: counted_by: Adjust name and identifier expansion
2023-08-17 22:06 ` Miguel Ojeda
@ 2023-08-17 23:45 ` Kees Cook
0 siblings, 0 replies; 7+ messages in thread
From: Kees Cook @ 2023-08-17 23:45 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Miguel Ojeda, Nick Desaulniers, Nathan Chancellor, Tom Rix,
Gustavo A. R. Silva, linux-kernel, llvm, linux-hardening
On Fri, Aug 18, 2023 at 12:06:15AM +0200, Miguel Ojeda wrote:
> On Thu, Aug 17, 2023 at 10:06 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > GCC and Clang's current RFCs name this attribute "counted_by", and have
> > moved away from using a string for the member name. Update the kernel's
> > macros to match. Additionally provide a UAPI no-op macro for UAPI structs
> > that will gain annotations.
> >
> > Cc: Miguel Ojeda <ojeda@kernel.org>
> > Cc: Nick Desaulniers <ndesaulniers@google.com>
> > Fixes: dd06e72e68bc ("Compiler Attributes: Add __counted_by macro")
> > Signed-off-by: Kees Cook <keescook@chromium.org>
>
> Looks good to me, thanks. Perhaps we should split the UAPI bit into a
> second patch.
I'd like to keep the UAPI part -- that was an oversight when I first
added it.
> I can take it into my tree; otherwise if you need this elsewhere:
>
> Acked-by: Miguel Ojeda <ojeda@kernel.org>
Thanks! It'd be slightly easier for me if it was is my tree, so I'll
grab your Ack. :) Let me know if you'd prefer it otherwise.
-Kees
--
Kees Cook
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Compiler Attributes: counted_by: Adjust name and identifier expansion
2023-08-17 20:06 [PATCH] Compiler Attributes: counted_by: Adjust name and identifier expansion Kees Cook
2023-08-17 20:13 ` Nathan Chancellor
2023-08-17 22:06 ` Miguel Ojeda
@ 2023-08-18 3:42 ` Gustavo A. R. Silva
2023-08-18 17:58 ` Kees Cook
3 siblings, 0 replies; 7+ messages in thread
From: Gustavo A. R. Silva @ 2023-08-18 3:42 UTC (permalink / raw)
To: Kees Cook, Miguel Ojeda
Cc: Nick Desaulniers, Nathan Chancellor, Tom Rix, Gustavo A. R. Silva,
linux-kernel, llvm, linux-hardening
On 8/17/23 14:06, Kees Cook wrote:
> GCC and Clang's current RFCs name this attribute "counted_by", and have
> moved away from using a string for the member name. Update the kernel's
> macros to match. Additionally provide a UAPI no-op macro for UAPI structs
> that will gain annotations.
>
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Fixes: dd06e72e68bc ("Compiler Attributes: Add __counted_by macro")
> Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Thanks
--
Gustavo
> ---
> include/linux/compiler_attributes.h | 26 +++++++++++++-------------
> include/uapi/linux/stddef.h | 4 ++++
> 2 files changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> index 00efa35c350f..74716a407aac 100644
> --- a/include/linux/compiler_attributes.h
> +++ b/include/linux/compiler_attributes.h
> @@ -94,6 +94,19 @@
> # define __copy(symbol)
> #endif
>
> +/*
> + * Optional: only supported since gcc >= 14
> + * Optional: only supported since clang >= 17
> + *
> + * gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896
> + * clang: https://reviews.llvm.org/D148381
> + */
> +#if __has_attribute(__counted_by__)
> +# define __counted_by(member) __attribute__((__counted_by__(member)))
> +#else
> +# define __counted_by(member)
> +#endif
> +
> /*
> * Optional: not supported by gcc
> * Optional: only supported since clang >= 14.0
> @@ -129,19 +142,6 @@
> # define __designated_init
> #endif
>
> -/*
> - * Optional: only supported since gcc >= 14
> - * Optional: only supported since clang >= 17
> - *
> - * gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896
> - * clang: https://reviews.llvm.org/D148381
> - */
> -#if __has_attribute(__element_count__)
> -# define __counted_by(member) __attribute__((__element_count__(#member)))
> -#else
> -# define __counted_by(member)
> -#endif
> -
> /*
> * Optional: only supported since clang >= 14.0
> *
> diff --git a/include/uapi/linux/stddef.h b/include/uapi/linux/stddef.h
> index 7837ba4fe728..7c3fc3980881 100644
> --- a/include/uapi/linux/stddef.h
> +++ b/include/uapi/linux/stddef.h
> @@ -45,3 +45,7 @@
> TYPE NAME[]; \
> }
> #endif
> +
> +#ifndef __counted_by
> +#define __counted_by(m)
> +#endif
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] Compiler Attributes: counted_by: Adjust name and identifier expansion
2023-08-17 20:06 [PATCH] Compiler Attributes: counted_by: Adjust name and identifier expansion Kees Cook
` (2 preceding siblings ...)
2023-08-18 3:42 ` Gustavo A. R. Silva
@ 2023-08-18 17:58 ` Kees Cook
3 siblings, 0 replies; 7+ messages in thread
From: Kees Cook @ 2023-08-18 17:58 UTC (permalink / raw)
To: Miguel Ojeda, Kees Cook
Cc: Nick Desaulniers, Nathan Chancellor, Tom Rix, Gustavo A. R. Silva,
linux-kernel, llvm, linux-hardening
On Thu, 17 Aug 2023 13:06:03 -0700, Kees Cook wrote:
> GCC and Clang's current RFCs name this attribute "counted_by", and have
> moved away from using a string for the member name. Update the kernel's
> macros to match. Additionally provide a UAPI no-op macro for UAPI structs
> that will gain annotations.
Applied to for-next/hardening, thanks!
[1/1] Compiler Attributes: counted_by: Adjust name and identifier expansion
https://git.kernel.org/kees/c/c8248faf3ca2
Take care,
--
Kees Cook
^ permalink raw reply [flat|nested] 7+ messages in thread