| Message ID | 20210910165851.3296624-1-keescook@chromium.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Compiler Attributes: Check GCC version for __alloc_size attribute | expand |
On Fri, Sep 10, 2021 at 9:58 AM Kees Cook <keescook@chromium.org> wrote: > > Unfortunately, just version checking the use of -Wno-alloc-size-larger-than > is not sufficient to make the __alloc_size attribute behave correctly > under older GCC versions. The attribute itself must be disabled in those > situations too, as there appears to be no way to reliably silence the > SIZE_MAX constant expression cases for GCC versions less than 9.1: > > In file included from ./include/linux/resource_ext.h:11, > from ./include/linux/pci.h:40, > from drivers/net/ethernet/intel/ixgbe/ixgbe.h:9, > from drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c:4: > In function 'kmalloc_node', > inlined from 'ixgbe_alloc_q_vector' at ./include/linux/slab.h:743:9: > ./include/linux/slab.h:618:9: error: argument 1 value '18446744073709551615' exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=] > return __kmalloc_node(size, flags, node); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > ./include/linux/slab.h: In function 'ixgbe_alloc_q_vector': > ./include/linux/slab.h:455:7: note: in a call to allocation function '__kmalloc_node' declared here > void *__kmalloc_node(size_t size, gfp_t flags, int node) __assume_slab_alignment __malloc; > ^~~~~~~~~~~~~~ > > Specifically: > -Wno-alloc-size-larger-than is not correctly handled by GCC < 9.1 > https://godbolt.org/z/hqsfG7q84 (doesn't disable) (heh, clang has had similar bugs with command line flags with `=` seperators) Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> Though some of the below examples don't make sense to me (the one above is fine). > https://godbolt.org/z/P9jdrPTYh (doesn't admit to not knowing about option) ^ technically your first link demonstrates that. This link doesn't add anything new and makes it look like there are more issues that there are. > https://godbolt.org/z/465TPMWKb (only warns when other warnings appear) ^ this example doesn't make sense to me. I couldn't reproduce what you're describing. > > -Walloc-size-larger-than=18446744073709551615 is not handled by GCC < 8 > https://godbolt.org/z/73hh1EPxz (ignores numeric value) Should this be GCC < 8.2? Some other feedback on the general use of godbolt. Under Output, please always disable Intel Asm Syntax; it causes me physical pain. Also under output, usually disabling Execute the Output is what you want, too. > > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Miguel Ojeda <ojeda@kernel.org> > Cc: Nathan Chancellor <nathan@kernel.org> > Cc: Nick Desaulniers <ndesaulniers@google.com> > Cc: Marco Elver <elver@google.com> > Cc: Will Deacon <will@kernel.org> > Cc: Arvind Sankar <nivedita@alum.mit.edu> > Cc: Masahiro Yamada <masahiroy@kernel.org> > Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org> > Cc: Sami Tolvanen <samitolvanen@google.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Ard Biesheuvel <ardb@kernel.org> > Cc: llvm@lists.linux.dev > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > I've been digging through older GCC build logs and found this needed to > be fixed even harder than prior attempts from what Randy had been seeing. > --- > include/linux/compiler-gcc.h | 9 +++++++++ > include/linux/compiler_attributes.h | 6 ------ > include/linux/compiler_types.h | 5 +++++ > 3 files changed, 14 insertions(+), 6 deletions(-) > > diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h > index 01985821944b..ffe0a7570d66 100644 > --- a/include/linux/compiler-gcc.h > +++ b/include/linux/compiler-gcc.h > @@ -151,3 +151,12 @@ > #else > #define __diag_GCC_8(s) > #endif > + > +/* > + * https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-alloc_005fsize-function-attribute > + * However, prior to 9.1, -Wno-alloc-size-larger-than does not work, > + * making this attribute unusable. > + */ > +#if GCC_VERSION < 90100 > +#define __alloc_size /**/ > +#endif > diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h > index 19f178e20e61..2487be0e7199 100644 > --- a/include/linux/compiler_attributes.h > +++ b/include/linux/compiler_attributes.h > @@ -53,12 +53,6 @@ > #define __aligned(x) __attribute__((__aligned__(x))) > #define __aligned_largest __attribute__((__aligned__)) > > -/* > - * gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-alloc_005fsize-function-attribute > - * clang: https://clang.llvm.org/docs/AttributeReference.html#alloc-size > - */ > -#define __alloc_size(x, ...) __attribute__((__alloc_size__(x, ## __VA_ARGS__))) > - > /* > * Note: users of __always_inline currently do not write "inline" themselves, > * which seems to be required by gcc to apply the attribute according > diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h > index c43308b0a9a9..91de9bead40d 100644 > --- a/include/linux/compiler_types.h > +++ b/include/linux/compiler_types.h > @@ -254,6 +254,11 @@ struct ftrace_likely_data { > #define asm_volatile_goto(x...) asm goto(x) > #endif > > +/* If not specifically disabled, allow the use of __alloc_size attribute. */ > +#ifndef __alloc_size > +# define __alloc_size(x, ...) __attribute__((__alloc_size__(x, ## __VA_ARGS__))) > +#endif > + > #ifdef CONFIG_CC_HAS_ASM_INLINE > #define asm_inline asm __inline > #else > -- > 2.30.2 >
On Fri, Sep 10, 2021 at 09:58:51AM -0700, Kees Cook wrote: > Unfortunately, just version checking the use of -Wno-alloc-size-larger-than > is not sufficient to make the __alloc_size attribute behave correctly > under older GCC versions. The attribute itself must be disabled in those > situations too, as there appears to be no way to reliably silence the > SIZE_MAX constant expression cases for GCC versions less than 9.1: > > In file included from ./include/linux/resource_ext.h:11, > from ./include/linux/pci.h:40, > from drivers/net/ethernet/intel/ixgbe/ixgbe.h:9, > from drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c:4: > In function 'kmalloc_node', > inlined from 'ixgbe_alloc_q_vector' at ./include/linux/slab.h:743:9: > ./include/linux/slab.h:618:9: error: argument 1 value '18446744073709551615' exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=] > return __kmalloc_node(size, flags, node); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > ./include/linux/slab.h: In function 'ixgbe_alloc_q_vector': > ./include/linux/slab.h:455:7: note: in a call to allocation function '__kmalloc_node' declared here > void *__kmalloc_node(size_t size, gfp_t flags, int node) __assume_slab_alignment __malloc; > ^~~~~~~~~~~~~~ > > Specifically: > -Wno-alloc-size-larger-than is not correctly handled by GCC < 9.1 > https://godbolt.org/z/hqsfG7q84 (doesn't disable) > https://godbolt.org/z/P9jdrPTYh (doesn't admit to not knowing about option) > https://godbolt.org/z/465TPMWKb (only warns when other warnings appear) > > -Walloc-size-larger-than=18446744073709551615 is not handled by GCC < 8 > https://godbolt.org/z/73hh1EPxz (ignores numeric value) > > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Miguel Ojeda <ojeda@kernel.org> > Cc: Nathan Chancellor <nathan@kernel.org> > Cc: Nick Desaulniers <ndesaulniers@google.com> > Cc: Marco Elver <elver@google.com> > Cc: Will Deacon <will@kernel.org> > Cc: Arvind Sankar <nivedita@alum.mit.edu> > Cc: Masahiro Yamada <masahiroy@kernel.org> > Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org> > Cc: Sami Tolvanen <samitolvanen@google.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Ard Biesheuvel <ardb@kernel.org> > Cc: llvm@lists.linux.dev > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > I've been digging through older GCC build logs and found this needed to > be fixed even harder than prior attempts from what Randy had been seeing. > --- > include/linux/compiler-gcc.h | 9 +++++++++ > include/linux/compiler_attributes.h | 6 ------ > include/linux/compiler_types.h | 5 +++++ > 3 files changed, 14 insertions(+), 6 deletions(-) > > diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h > index 01985821944b..ffe0a7570d66 100644 > --- a/include/linux/compiler-gcc.h > +++ b/include/linux/compiler-gcc.h > @@ -151,3 +151,12 @@ > #else > #define __diag_GCC_8(s) > #endif > + > +/* > + * https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-alloc_005fsize-function-attribute > + * However, prior to 9.1, -Wno-alloc-size-larger-than does not work, > + * making this attribute unusable. > + */ > +#if GCC_VERSION < 90100 > +#define __alloc_size /**/ Argh, this needs to be: #define __alloc_size(x, ...) /**/ This attribute has been such a pain. :P > +#endif > diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h > index 19f178e20e61..2487be0e7199 100644 > --- a/include/linux/compiler_attributes.h > +++ b/include/linux/compiler_attributes.h > @@ -53,12 +53,6 @@ > #define __aligned(x) __attribute__((__aligned__(x))) > #define __aligned_largest __attribute__((__aligned__)) > > -/* > - * gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-alloc_005fsize-function-attribute > - * clang: https://clang.llvm.org/docs/AttributeReference.html#alloc-size > - */ > -#define __alloc_size(x, ...) __attribute__((__alloc_size__(x, ## __VA_ARGS__))) > - > /* > * Note: users of __always_inline currently do not write "inline" themselves, > * which seems to be required by gcc to apply the attribute according > diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h > index c43308b0a9a9..91de9bead40d 100644 > --- a/include/linux/compiler_types.h > +++ b/include/linux/compiler_types.h > @@ -254,6 +254,11 @@ struct ftrace_likely_data { > #define asm_volatile_goto(x...) asm goto(x) > #endif > > +/* If not specifically disabled, allow the use of __alloc_size attribute. */ > +#ifndef __alloc_size > +# define __alloc_size(x, ...) __attribute__((__alloc_size__(x, ## __VA_ARGS__))) > +#endif > + > #ifdef CONFIG_CC_HAS_ASM_INLINE > #define asm_inline asm __inline > #else > -- > 2.30.2 >
On Fri, Sep 10, 2021 at 12:05:03PM -0700, Nick Desaulniers wrote: > On Fri, Sep 10, 2021 at 9:58 AM Kees Cook <keescook@chromium.org> wrote: > > > > Unfortunately, just version checking the use of -Wno-alloc-size-larger-than > > is not sufficient to make the __alloc_size attribute behave correctly > > under older GCC versions. The attribute itself must be disabled in those > > situations too, as there appears to be no way to reliably silence the > > SIZE_MAX constant expression cases for GCC versions less than 9.1: > > > > In file included from ./include/linux/resource_ext.h:11, > > from ./include/linux/pci.h:40, > > from drivers/net/ethernet/intel/ixgbe/ixgbe.h:9, > > from drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c:4: > > In function 'kmalloc_node', > > inlined from 'ixgbe_alloc_q_vector' at ./include/linux/slab.h:743:9: > > ./include/linux/slab.h:618:9: error: argument 1 value '18446744073709551615' exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=] > > return __kmalloc_node(size, flags, node); > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > ./include/linux/slab.h: In function 'ixgbe_alloc_q_vector': > > ./include/linux/slab.h:455:7: note: in a call to allocation function '__kmalloc_node' declared here > > void *__kmalloc_node(size_t size, gfp_t flags, int node) __assume_slab_alignment __malloc; > > ^~~~~~~~~~~~~~ > > > > Specifically: > > -Wno-alloc-size-larger-than is not correctly handled by GCC < 9.1 > > https://godbolt.org/z/hqsfG7q84 (doesn't disable) > > (heh, clang has had similar bugs with command line flags with `=` seperators) > > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > > Though some of the below examples don't make sense to me (the one > above is fine). > > > https://godbolt.org/z/P9jdrPTYh (doesn't admit to not knowing about option) > > ^ technically your first link demonstrates that. This link doesn't add > anything new and makes it look like there are more issues that there > are. Yes, but it's messy. I wanted to show each part of it. The above shows that given just "-Wno-alloc-size-larger-than" and no other errors, the compiler happily takes the argument. (i.e. $(call cc-option,...) would think it succeeded). > > https://godbolt.org/z/465TPMWKb (only warns when other warnings appear) > > ^ this example doesn't make sense to me. I couldn't reproduce what > you're describing. In this one, the only difference between it and the one above it is having an unrelated warning present: <source>: In function 'main': <source>:8:9: warning: unused variable 'trigger_an_extra_warning' [-Wunused-variable] int trigger_an_extra_warning; ^~~~~~~~~~~~~~~~~~~~~~~~ At which point it suddenly DOES admit to not knowing about -Wno-alloc-size-larger-than: <source>: At top level: cc1: warning: unrecognized command line option '-Wno-alloc-size-larger-than' > > > > > -Walloc-size-larger-than=18446744073709551615 is not handled by GCC < 8 > > https://godbolt.org/z/73hh1EPxz (ignores numeric value) > > Should this be GCC < 8.2? Yeah, that's true, the note here should say "8.2", but for the purposes of this attempted it, everything below 9.1 is broken in various ways. Better to just avoid everything. I wanted to document all the ways it is broken, though, so folks don't go through my same pain. :P > Some other feedback on the general use of godbolt. Under Output, > please always disable Intel Asm Syntax; it causes me physical pain. > Also under output, usually disabling Execute the Output is what you > want, too. Good points, yes! Thanks for checking this out. :)
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h index 01985821944b..ffe0a7570d66 100644 --- a/include/linux/compiler-gcc.h +++ b/include/linux/compiler-gcc.h @@ -151,3 +151,12 @@ #else #define __diag_GCC_8(s) #endif + +/* + * https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-alloc_005fsize-function-attribute + * However, prior to 9.1, -Wno-alloc-size-larger-than does not work, + * making this attribute unusable. + */ +#if GCC_VERSION < 90100 +#define __alloc_size /**/ +#endif diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h index 19f178e20e61..2487be0e7199 100644 --- a/include/linux/compiler_attributes.h +++ b/include/linux/compiler_attributes.h @@ -53,12 +53,6 @@ #define __aligned(x) __attribute__((__aligned__(x))) #define __aligned_largest __attribute__((__aligned__)) -/* - * gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-alloc_005fsize-function-attribute - * clang: https://clang.llvm.org/docs/AttributeReference.html#alloc-size - */ -#define __alloc_size(x, ...) __attribute__((__alloc_size__(x, ## __VA_ARGS__))) - /* * Note: users of __always_inline currently do not write "inline" themselves, * which seems to be required by gcc to apply the attribute according diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h index c43308b0a9a9..91de9bead40d 100644 --- a/include/linux/compiler_types.h +++ b/include/linux/compiler_types.h @@ -254,6 +254,11 @@ struct ftrace_likely_data { #define asm_volatile_goto(x...) asm goto(x) #endif +/* If not specifically disabled, allow the use of __alloc_size attribute. */ +#ifndef __alloc_size +# define __alloc_size(x, ...) __attribute__((__alloc_size__(x, ## __VA_ARGS__))) +#endif + #ifdef CONFIG_CC_HAS_ASM_INLINE #define asm_inline asm __inline #else
Unfortunately, just version checking the use of -Wno-alloc-size-larger-than is not sufficient to make the __alloc_size attribute behave correctly under older GCC versions. The attribute itself must be disabled in those situations too, as there appears to be no way to reliably silence the SIZE_MAX constant expression cases for GCC versions less than 9.1: In file included from ./include/linux/resource_ext.h:11, from ./include/linux/pci.h:40, from drivers/net/ethernet/intel/ixgbe/ixgbe.h:9, from drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c:4: In function 'kmalloc_node', inlined from 'ixgbe_alloc_q_vector' at ./include/linux/slab.h:743:9: ./include/linux/slab.h:618:9: error: argument 1 value '18446744073709551615' exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=] return __kmalloc_node(size, flags, node); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ./include/linux/slab.h: In function 'ixgbe_alloc_q_vector': ./include/linux/slab.h:455:7: note: in a call to allocation function '__kmalloc_node' declared here void *__kmalloc_node(size_t size, gfp_t flags, int node) __assume_slab_alignment __malloc; ^~~~~~~~~~~~~~ Specifically: -Wno-alloc-size-larger-than is not correctly handled by GCC < 9.1 https://godbolt.org/z/hqsfG7q84 (doesn't disable) https://godbolt.org/z/P9jdrPTYh (doesn't admit to not knowing about option) https://godbolt.org/z/465TPMWKb (only warns when other warnings appear) -Walloc-size-larger-than=18446744073709551615 is not handled by GCC < 8 https://godbolt.org/z/73hh1EPxz (ignores numeric value) Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Miguel Ojeda <ojeda@kernel.org> Cc: Nathan Chancellor <nathan@kernel.org> Cc: Nick Desaulniers <ndesaulniers@google.com> Cc: Marco Elver <elver@google.com> Cc: Will Deacon <will@kernel.org> Cc: Arvind Sankar <nivedita@alum.mit.edu> Cc: Masahiro Yamada <masahiroy@kernel.org> Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org> Cc: Sami Tolvanen <samitolvanen@google.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Ard Biesheuvel <ardb@kernel.org> Cc: llvm@lists.linux.dev Signed-off-by: Kees Cook <keescook@chromium.org> --- I've been digging through older GCC build logs and found this needed to be fixed even harder than prior attempts from what Randy had been seeing. --- include/linux/compiler-gcc.h | 9 +++++++++ include/linux/compiler_attributes.h | 6 ------ include/linux/compiler_types.h | 5 +++++ 3 files changed, 14 insertions(+), 6 deletions(-)