Message ID | 20230517190841.gonna.796-kees@kernel.org (mailing list archive) |
---|---|
State | Mainlined |
Commit | dd06e72e68bcb4070ef211be100d2896e236c8fb |
Headers | show |
Series | [v2] Compiler Attributes: Add __counted_by macro | expand |
On Wed, May 17, 2023 at 12:08:44PM -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: Gustavo A. R. Silva <gustavoars@kernel.org> Thanks -- Gustavo > --- > v2: - update "Optional" comments > v1: https://lore.kernel.org/all/20230504181636.never.222-kees@kernel.org/ > --- > include/linux/compiler_attributes.h | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h > index e659cb6fded3..a92d8887e8f0 100644 > --- a/include/linux/compiler_attributes.h > +++ b/include/linux/compiler_attributes.h > @@ -123,6 +123,19 @@ > # 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 > * > -- > 2.34.1 >
On Wed, May 17, 2023 at 12:08:44PM -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> > --- > v2: - update "Optional" comments > v1: https://lore.kernel.org/all/20230504181636.never.222-kees@kernel.org/ > --- > include/linux/compiler_attributes.h | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h > index e659cb6fded3..a92d8887e8f0 100644 > --- a/include/linux/compiler_attributes.h > +++ b/include/linux/compiler_attributes.h > @@ -123,6 +123,19 @@ > # 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 > * > -- > 2.34.1 >
On Wed, 17 May 2023 12:08:44 -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. > > [...] FYI, applied to for-next/hardening: [1/1] Compiler Attributes: Add __counted_by macro https://git.kernel.org/kees/c/86a76e91cbab
On Fri, May 26, 2023 at 7:16 PM Kees Cook <keescook@chromium.org> wrote: > > FYI, applied to for-next/hardening: > > [1/1] Compiler Attributes: Add __counted_by macro > https://git.kernel.org/kees/c/86a76e91cbab Sorry, I was going to apply it soon -- in case you want it: Acked-by: Miguel Ojeda <ojeda@kernel.org> And thanks Nathan for resubmitting the `Reviewed-by` from v1! Cheers, Miguel
On Fri, May 26, 2023 at 07:47:03PM +0200, Miguel Ojeda wrote: > On Fri, May 26, 2023 at 7:16 PM Kees Cook <keescook@chromium.org> wrote: > > > > FYI, applied to for-next/hardening: > > > > [1/1] Compiler Attributes: Add __counted_by macro > > https://git.kernel.org/kees/c/86a76e91cbab > > Sorry, I was going to apply it soon -- in case you want it: > > Acked-by: Miguel Ojeda <ojeda@kernel.org> Thanks! > And thanks Nathan for resubmitting the `Reviewed-by` from v1! Yes, apologies for missing this in my v2 submission!
On Fri, May 26, 2023 at 11:56 AM Kees Cook <keescook@chromium.org> wrote: > > On Fri, May 26, 2023 at 07:47:03PM +0200, Miguel Ojeda wrote: > > On Fri, May 26, 2023 at 7:16 PM Kees Cook <keescook@chromium.org> wrote: > > > > > > FYI, applied to for-next/hardening: > > > > > > [1/1] Compiler Attributes: Add __counted_by macro > > > https://git.kernel.org/kees/c/86a76e91cbab > > > > Sorry, I was going to apply it soon -- in case you want it: > > > > Acked-by: Miguel Ojeda <ojeda@kernel.org> > > Thanks! > > > And thanks Nathan for resubmitting the `Reviewed-by` from v1! > > Yes, apologies for missing this in my v2 submission! > > -- > Kees Cook > https://discourse.llvm.org/t/rfc-enforcing-bounds-safety-in-c-fbounds-safety/70854 proposes a macro __counted_by as well. This patch uses the same name: > # define __counted_by(member) __attribute__((__element_count__(member))) I wonder whether the two use cases are compatible so that using the same macro name will be fine. #if defined(__has_feature) && __has_feature(bounds_safety) #define __counted_by(T) __attribute__((__counted_by__(T))) // ... other bounds annotations #else #define __counted_by(T) // defined as nothing // ... other bounds annotations #endif
On Fri, May 26, 2023 at 12:48:26PM -0700, Fangrui Song wrote: > On Fri, May 26, 2023 at 11:56 AM Kees Cook <keescook@chromium.org> wrote: > > > > On Fri, May 26, 2023 at 07:47:03PM +0200, Miguel Ojeda wrote: > > > On Fri, May 26, 2023 at 7:16 PM Kees Cook <keescook@chromium.org> wrote: > > > > > > > > FYI, applied to for-next/hardening: > > > > > > > > [1/1] Compiler Attributes: Add __counted_by macro > > > > https://git.kernel.org/kees/c/86a76e91cbab > > > > > > Sorry, I was going to apply it soon -- in case you want it: > > > > > > Acked-by: Miguel Ojeda <ojeda@kernel.org> > > > > Thanks! > > > > > And thanks Nathan for resubmitting the `Reviewed-by` from v1! > > > > Yes, apologies for missing this in my v2 submission! > > > > -- > > Kees Cook > > > > https://discourse.llvm.org/t/rfc-enforcing-bounds-safety-in-c-fbounds-safety/70854 > proposes a macro __counted_by as well. > This patch uses the same name: > > > # define __counted_by(member) __attribute__((__element_count__(member))) > > I wonder whether the two use cases are compatible so that using the > same macro name will be fine. Yeah, I have suggest the name change for the GCC proposal. However, given that there is still no code to test for -fbounds-safety, I'm sticking with __element_count for the moment, as there is code implementing that name in both GCC and Clang today. > #if defined(__has_feature) && __has_feature(bounds_safety) > #define __counted_by(T) __attribute__((__counted_by__(T))) > // ... other bounds annotations > #else > #define __counted_by(T) // defined as nothing // ... other bounds annotations > #endif Right. My main consideration for getting __counted_by defined by the kernel at all is so that annotation can begin. We can adjust the define's contents as needed. :)
diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h index e659cb6fded3..a92d8887e8f0 100644 --- a/include/linux/compiler_attributes.h +++ b/include/linux/compiler_attributes.h @@ -123,6 +123,19 @@ # 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 *
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> --- v2: - update "Optional" comments v1: https://lore.kernel.org/all/20230504181636.never.222-kees@kernel.org/ --- include/linux/compiler_attributes.h | 13 +++++++++++++ 1 file changed, 13 insertions(+)