History

Compiler Attributes: add __alloc_size() for better bounds checking

GCC and Clang can use the "alloc_size" attribute to better inform the results of __builtin_object_size() (for compile-time constant values). Clang can additionally use alloc_size to inform the results of __builtin_dynamic_object_size() (for run-time values). Because GCC sees the frequent use of struct_size() as an allocator size argument, and notices it can return SIZE_MAX (the overflow indication), it complains about these call sites overflowing (since SIZE_MAX is greater than the default -Walloc-size-larger-than=PTRDIFF_MAX). This isn't helpful since we already know a SIZE_MAX will be caught at run-time (this was an intentional design). To deal with this, we must disable this check as it is both a false positive and redundant. (Clang does not have this warning option.) Unfortunately, just checking the -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.2 https://godbolt.org/z/73hh1EPxz (ignores numeric value) Since anything marked with __alloc_size would also qualify for marking with __malloc, just include __malloc along with it to avoid redundant markings. (Suggested by Linus Torvalds.) Finally, make sure checkpatch.pl doesn't get confused about finding the __alloc_size attribute on functions. (Thanks to Joe Perches.)

Kees Cook committed 5 years ago

Commit 86cffec1 parent 75da0eb5 files · +47 −1Browse files

Unified · Split

5 files
  1. Makefile +15 −0
  2. include/linux/compiler-gcc.h +8 −0
  3. include/linux/compiler_attributes.h +10 −0
  4. include/linux/compiler_types.h +12 −0
  5. scripts/checkpatch.pl +2 −1
Makefile+15−0
@@ -1008,6 +1008,21 @@ ifdef CONFIG_CC_IS_GCC · 1007 unmodified lines
10081008 KBUILD_CFLAGS += -Wno-maybe-uninitialized
10091009 endif
10101010
1011+ifdef CONFIG_CC_IS_GCC
1012+# The allocators already balk at large sizes, so silence the compiler
1013+# warnings for bounds checks involving those possible values. While
1014+# -Wno-alloc-size-larger-than would normally be used here, earlier versions
1015+# of gcc (<9.1) weirdly don't handle the option correctly when _other_
1016+# warnings are produced (?!). Using -Walloc-size-larger-than=SIZE_MAX
1017+# doesn't work (as it is documented to), silently resolving to "0" prior to
1018+# version 9.1 (and producing an error more recently). Numeric values larger
1019+# than PTRDIFF_MAX also don't work prior to version 9.1, which are silently
1020+# ignored, continuing to default to PTRDIFF_MAX. So, left with no other
1021+# choice, we must perform a versioned check to disable this warning.
1022+# https://lore.kernel.org/lkml/20210824115859.187f272f@canb.auug.org.au
1023+KBUILD_CFLAGS += $(call cc-ifversion, -ge, 0901, -Wno-alloc-size-larger-than)
1024+endif
1025+
10111026 # disable invalid "can't wrap" optimizations for signed / pointers
10121027 KBUILD_CFLAGS += -fno-strict-overflow
10131028
include/linux/compiler-gcc.h+8−0
@@ -144,3 +144,11 @@ · 143 unmodified lines
144144 #else
145145 #define __diag_GCC_8(s)
146146 #endif
147+
148+/*
149+ * Prior to 9.1, -Wno-alloc-size-larger-than (and therefore the "alloc_size"
150+ * attribute) do not work, and must be disabled.
151+ */
152+#if GCC_VERSION < 90100
153+#undef __alloc_size__
154+#endif
include/linux/compiler_attributes.h+10−0
@@ -33,6 +33,15 @@ · 32 unmodified lines
3333 #define __aligned(x) __attribute__((__aligned__(x)))
3434 #define __aligned_largest __attribute__((__aligned__))
3535
36+/*
37+ * Note: do not use this directly. Instead, use __alloc_size() since it is conditionally
38+ * available and includes other attributes.
39+ *
40+ * gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-alloc_005fsize-function-attribute
41+ * clang: https://clang.llvm.org/docs/AttributeReference.html#alloc-size
42+ */
43+#define __alloc_size__(x, ...) __attribute__((__alloc_size__(x, ## __VA_ARGS__)))
44+
3645 /*
3746 * Note: users of __always_inline currently do not write "inline" themselves,
3847 * which seems to be required by gcc to apply the attribute according
@@ -153,6 +162,7 @@ · 114 unmodified lines
153162
154163 /*
155164 * gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-malloc-function-attribute
165+ * clang: https://clang.llvm.org/docs/AttributeReference.html#malloc
156166 */
157167 #define __malloc __attribute__((__malloc__))
158168
include/linux/compiler_types.h+12−0
@@ -250,6 +250,18 @@ struct ftrace_likely_data { · 249 unmodified lines
250250 # define __cficanonical
251251 #endif
252252
253+/*
254+ * Any place that could be marked with the "alloc_size" attribute is also
255+ * a place to be marked with the "malloc" attribute. Do this as part of the
256+ * __alloc_size macro to avoid redundant attributes and to avoid missing a
257+ * __malloc marking.
258+ */
259+#ifdef __alloc_size__
260+# define __alloc_size(x, ...) __alloc_size__(x, ## __VA_ARGS__) __malloc
261+#else
262+# define __alloc_size(x, ...) __malloc
263+#endif
264+
253265 #ifndef asm_volatile_goto
254266 #define asm_volatile_goto(x...) asm goto(x)
255267 #endif
scripts/checkpatch.pl+2−1
@@ -489,7 +489,8 @@ our $Attribute = qr{ · 488 unmodified lines
489489 ____cacheline_aligned|
490490 ____cacheline_aligned_in_smp|
491491 ____cacheline_internodealigned_in_smp|
492− __weak
492+ __weak|
493+ __alloc_size\s*\(\s*\d+\s*(?:,\s*\d+\s*)?\)
493494 }x;
494495 our $Modifier;
495496 our $Inline = qr{inline|__always_inline|noinline|__inline|__inline__};

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t