* Re: [PATCH] Compiler Attributes: Add __counted_by macro
2023-05-04 18:16 [PATCH] Compiler Attributes: Add __counted_by macro Kees Cook
@ 2023-05-04 18:19 ` Kees Cook
2023-05-04 18:27 ` Miguel Ojeda
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2023-05-04 18:19 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Bill Wendling, Qing Zhao, Gustavo A . R . Silva,
Nick Desaulniers, Nathan Chancellor, Tom Rix, llvm, linux-kernel,
linux-hardening
On Thu, May 04, 2023 at 11:16:40AM -0700, Kees Cook wrote:
> In an effort to annotate all flexible array members with their run-time
> size information, the "element_count" attribute is being introduced by
> Clang[1] and GCC[2] in future releases. This annotation will provide
> the CONFIG_UBSAN_BOUNDS and CONFIG_FORTIFY_SOURCE features the ability
> to perform run-time bounds checking on otherwise unknown-size flexible
> arrays.
>
> Even though the attribute is under development, we can start the
> annotation process in the kernel. This requires defining a macro for
And FWIW, I've done a first-pass at this annotation with Coccinelle. There
are plenty more to do:
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/commit/?h=devel/counted_by&id=adc5b3cb48a049563dc673f348eab7b6beba8a9b
155 files changed, 158 insertions, 158 deletions
--
Kees Cook
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Compiler Attributes: Add __counted_by macro
2023-05-04 18:16 [PATCH] Compiler Attributes: Add __counted_by macro Kees Cook
2023-05-04 18:19 ` Kees Cook
@ 2023-05-04 18:27 ` Miguel Ojeda
2023-05-04 23:44 ` Kees Cook
2023-05-04 18:37 ` Gustavo A. R. Silva
2023-05-04 21:18 ` Nathan Chancellor
3 siblings, 1 reply; 6+ messages in thread
From: Miguel Ojeda @ 2023-05-04 18:27 UTC (permalink / raw)
To: Kees Cook
Cc: Miguel Ojeda, Bill Wendling, Qing Zhao, Gustavo A . R . Silva,
Nick Desaulniers, Nathan Chancellor, Tom Rix, llvm, linux-kernel,
linux-hardening
On Thu, May 4, 2023 at 8:16 PM Kees Cook <keescook@chromium.org> wrote:
>
> + * Optional: future support coming in clang 17 and gcc 14
Should we just say:
Optional: only supported since clang >= 17
Optional: only supported since gcc >= 14
even if they are in the future? That way we avoid changing it later.
If somebody asks, you already say it it is in the future the commit
message, so it should be clear enough... :)
And if the compilers end up not supporting it on those versions for
some unexpected reason, well, we will need to fix the comment either
way.
(I can change it on my side if you prefer)
Thanks!
Cheers,
Miguel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Compiler Attributes: Add __counted_by macro
2023-05-04 18:27 ` Miguel Ojeda
@ 2023-05-04 23:44 ` Kees Cook
0 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2023-05-04 23:44 UTC (permalink / raw)
To: Miguel Ojeda, Kees Cook
Cc: Miguel Ojeda, Bill Wendling, Qing Zhao, Gustavo A . R . Silva,
Nick Desaulniers, Nathan Chancellor, Tom Rix, llvm, linux-kernel,
linux-hardening
On May 4, 2023 11:27:32 AM PDT, Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:
>On Thu, May 4, 2023 at 8:16 PM Kees Cook <keescook@chromium.org> wrote:
>>
>> + * Optional: future support coming in clang 17 and gcc 14
>
>Should we just say:
>
> Optional: only supported since clang >= 17
> Optional: only supported since gcc >= 14
>
>even if they are in the future? That way we avoid changing it later.
>If somebody asks, you already say it it is in the future the commit
>message, so it should be clear enough... :)
>
>And if the compilers end up not supporting it on those versions for
>some unexpected reason, well, we will need to fix the comment either
>way.
Yeah, fair point!
>
>(I can change it on my side if you prefer)
Sure, thanks! (But I'm happy to send a v2 if that's easier for you, too.)
-Kees
--
Kees Cook
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Compiler Attributes: Add __counted_by macro
2023-05-04 18:16 [PATCH] Compiler Attributes: Add __counted_by macro Kees Cook
2023-05-04 18:19 ` Kees Cook
2023-05-04 18:27 ` Miguel Ojeda
@ 2023-05-04 18:37 ` Gustavo A. R. Silva
2023-05-04 21:18 ` Nathan Chancellor
3 siblings, 0 replies; 6+ messages in thread
From: Gustavo A. R. Silva @ 2023-05-04 18:37 UTC (permalink / raw)
To: Kees Cook
Cc: Miguel Ojeda, Bill Wendling, Qing Zhao, Nick Desaulniers,
Nathan Chancellor, Tom Rix, llvm, linux-kernel, linux-hardening
On Thu, May 04, 2023 at 11:16:40AM -0700, Kees Cook wrote:
> In an effort to annotate all flexible array members with their run-time
> size information, the "element_count" attribute is being introduced by
> Clang[1] and GCC[2] in future releases. This annotation will provide
> the CONFIG_UBSAN_BOUNDS and CONFIG_FORTIFY_SOURCE features the ability
> to perform run-time bounds checking on otherwise unknown-size flexible
> arrays.
It's happening! :D
>
> Even though the attribute is under development, we can start the
> annotation process in the kernel. This requires defining a macro for
> it, even if we have to change the name of the actual attribute later.
> Since it is likely that this attribute may change its name to "counted_by"
> in the future (to better align with a future total bytes "sized_by"
> attribute), name the wrapper macro "__counted_by", which also reads more
> clearly (and concisely) in structure definitions.
>
> [1] https://reviews.llvm.org/D148381
> [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896
>
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Cc: Bill Wendling <morbo@google.com>
> Cc: Qing Zhao <qing.zhao@oracle.com>
> Cc: Gustavo A. R. Silva <gustavoars@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>
Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Thanks!
--
Gustavo
> ---
> include/linux/compiler_attributes.h | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> index e659cb6fded3..9d63fe2024d5 100644
> --- a/include/linux/compiler_attributes.h
> +++ b/include/linux/compiler_attributes.h
> @@ -123,6 +123,18 @@
> # define __designated_init
> #endif
>
> +/*
> + * Optional: future support coming in clang 17 and gcc 14
> + *
> + * 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
> *
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Compiler Attributes: Add __counted_by macro
2023-05-04 18:16 [PATCH] Compiler Attributes: Add __counted_by macro Kees Cook
` (2 preceding siblings ...)
2023-05-04 18:37 ` Gustavo A. R. Silva
@ 2023-05-04 21:18 ` Nathan Chancellor
3 siblings, 0 replies; 6+ messages in thread
From: Nathan Chancellor @ 2023-05-04 21:18 UTC (permalink / raw)
To: Kees Cook
Cc: Miguel Ojeda, Bill Wendling, Qing Zhao, Gustavo A . R . Silva,
Nick Desaulniers, Tom Rix, llvm, linux-kernel, linux-hardening
On Thu, May 04, 2023 at 11:16:40AM -0700, Kees Cook wrote:
> In an effort to annotate all flexible array members with their run-time
> size information, the "element_count" attribute is being introduced by
> Clang[1] and GCC[2] in future releases. This annotation will provide
> the CONFIG_UBSAN_BOUNDS and CONFIG_FORTIFY_SOURCE features the ability
> to perform run-time bounds checking on otherwise unknown-size flexible
> arrays.
>
> Even though the attribute is under development, we can start the
> annotation process in the kernel. This requires defining a macro for
> it, even if we have to change the name of the actual attribute later.
> Since it is likely that this attribute may change its name to "counted_by"
> in the future (to better align with a future total bytes "sized_by"
> attribute), name the wrapper macro "__counted_by", which also reads more
> clearly (and concisely) in structure definitions.
>
> [1] https://reviews.llvm.org/D148381
> [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896
>
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Cc: Bill Wendling <morbo@google.com>
> Cc: Qing Zhao <qing.zhao@oracle.com>
> Cc: Gustavo A. R. Silva <gustavoars@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>
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
I agree with Miguel's comment formatting and content suggestions. Thanks
for the links, they look good. If we have to update the name of the
attribute later, it is not the end of the world, as getting the coversion
started at this phase will make the roll out quicker.
> ---
> include/linux/compiler_attributes.h | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> index e659cb6fded3..9d63fe2024d5 100644
> --- a/include/linux/compiler_attributes.h
> +++ b/include/linux/compiler_attributes.h
> @@ -123,6 +123,18 @@
> # define __designated_init
> #endif
>
> +/*
> + * Optional: future support coming in clang 17 and gcc 14
> + *
> + * 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
> *
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread