linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: akpm@linux-foundation.org, apw@canonical.com, cl@linux.com,
	danielmicay@gmail.com, dennis@kernel.org,
	dwaipayanray1@gmail.com, iamjoonsoo.kim@lge.com, joe@perches.com,
	keescook@chromium.org, linux-mm@kvack.org,
	lukas.bulwahn@gmail.com, mm-commits@vger.kernel.org,
	nathan@kernel.org, ndesaulniers@google.com, ojeda@kernel.org,
	penberg@kernel.org, rdunlap@infradead.org, rientjes@google.com,
	tj@kernel.org, torvalds@linux-foundation.org, vbabka@suse.cz
Subject: [patch 3/9] Compiler Attributes: add __alloc_size() for better bounds checking
Date: Thu, 09 Sep 2021 20:10:26 -0700	[thread overview]
Message-ID: <20210910031026.thFt7MLEf%akpm@linux-foundation.org> (raw)
In-Reply-To: <20210909200948.090d4e213ca34b5ad1325a7e@linux-foundation.org>

From: Kees Cook <keescook@chromium.org>
Subject: Compiler Attributes: add __alloc_size() for better bounds checking

Patch series "Add __alloc_size() for better bounds checking", v2.

GCC and Clang both use the "alloc_size" attribute to assist with bounds
checking around the use of allocation functions.  Add the attribute,
adjust the Makefile to silence needless warnings, and add the hints to the
allocators where possible.  These changes have been in use for a while now
in GrapheneOS.


This patch (of 2):

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 may overflow (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).  Instead, just disable this check as it is
both a false positive and redundant.  (Clang does not have this warning
option.)

[keescook@chromium.org: adjust warning logic for pre-9.1 gcc behaviors]
  Link: https://lkml.kernel.org/r/20210827151327.2729736-1-keescook@chromium.org
Link: https://lkml.kernel.org/r/20210818214021.2476230-1-keescook@chromium.org
Link: https://lkml.kernel.org/r/20210818214021.2476230-2-keescook@chromium.org
Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
Tested-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Andy Whitcroft <apw@canonical.com>
Cc: Christoph Lameter <cl@linux.com>
Cc: Daniel Micay <danielmicay@gmail.com>
Cc: David Rientjes <rientjes@google.com>
Cc: Dennis Zhou <dennis@kernel.org>
Cc: Dwaipayan Ray <dwaipayanray1@gmail.com>
Cc: Joe Perches <joe@perches.com>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 Makefile                            |   15 +++++++++++++++
 include/linux/compiler_attributes.h |    6 ++++++
 2 files changed, 21 insertions(+)

--- a/include/linux/compiler_attributes.h~compiler-attributes-add-__alloc_size-for-better-bounds-checking
+++ a/include/linux/compiler_attributes.h
@@ -54,6 +54,12 @@
 #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
  * to its docs (and also "warning: always_inline function might not be
--- a/Makefile~compiler-attributes-add-__alloc_size-for-better-bounds-checking
+++ a/Makefile
@@ -1014,6 +1014,21 @@ ifdef CONFIG_CC_IS_GCC
 KBUILD_CFLAGS += -Wno-maybe-uninitialized
 endif
 
+ifdef CONFIG_CC_IS_GCC
+# The allocators already balk at large sizes, so silence the compiler
+# warnings for bounds checks involving those possible values. While
+# -Wno-alloc-size-larger-than would normally be used here, earlier versions
+# of gcc (<9.1) weirdly don't handle the option correctly when _other_
+# warnings are produced (?!). Using -Walloc-size-larger-than=SIZE_MAX
+# doesn't work (as it is documented to), silently resolving to "0" prior to
+# version 9.1 (and producing an error more recently). Numeric values larger
+# than PTRDIFF_MAX also don't work prior to version 9.1, which are silently
+# ignored, continuing to default to PTRDIFF_MAX. So, left with no other
+# choice, we must perform a versioned check to disable this warning.
+# https://lore.kernel.org/lkml/20210824115859.187f272f@canb.auug.org.au
+KBUILD_CFLAGS += $(call cc-ifversion, -ge, 0901, -Wno-alloc-size-larger-than)
+endif
+
 # disable invalid "can't wrap" optimizations for signed / pointers
 KBUILD_CFLAGS	+= -fno-strict-overflow
 
_


  parent reply	other threads:[~2021-09-10  3:10 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-10  3:09 incoming Andrew Morton
2021-09-10  3:10 ` [patch 1/9] mm: move kvmalloc-related functions to slab.h Andrew Morton
2021-09-10  3:10 ` [patch 2/9] rapidio: avoid bogus __alloc_size warning Andrew Morton
2021-09-10  3:10 ` Andrew Morton [this message]
2021-09-10  3:10 ` [patch 4/9] checkpatch: add __alloc_size() to known $Attribute Andrew Morton
2021-09-10  3:10 ` [patch 5/9] slab: clean up function declarations Andrew Morton
2021-09-10  3:10 ` [patch 6/9] slab: add __alloc_size attributes for better bounds checking Andrew Morton
2021-09-10  3:10 ` [patch 7/9] mm/page_alloc: " Andrew Morton
2021-09-10  3:10 ` [patch 8/9] percpu: " Andrew Morton
2021-09-10  3:10 ` [patch 9/9] mm/vmalloc: " Andrew Morton
2021-09-10 17:23   ` Linus Torvalds
2021-09-10 18:43     ` Kees Cook
2021-09-10 19:17       ` Linus Torvalds
2021-09-10 19:32         ` Kees Cook
2021-09-10 19:49     ` Nick Desaulniers
2021-09-10 20:16       ` Linus Torvalds
2021-09-10 20:47         ` Kees Cook
2021-09-10 20:58           ` Nick Desaulniers
2021-09-10 21:07             ` Kees Cook
2021-09-11  5:29     ` Joe Perches
2021-09-21 23:37     ` Kees Cook
2021-09-21 23:45       ` Joe Perches
2021-09-22  2:25         ` function prototype element ordering Kees Cook
2021-09-22  4:24           ` Joe Perches
2021-09-24 19:43             ` Kees Cook
2021-09-22  7:24           ` Alexey Dobriyan
2021-09-22  8:51             ` Joe Perches
2021-09-22 10:45               ` Alexey Dobriyan
2021-09-22 11:19             ` Jani Nikula
2021-09-22 21:15             ` Linus Torvalds
2021-09-23  5:10               ` Joe Perches
2021-09-25 19:40               ` David Laight
2021-09-26 21:03                 ` Linus Torvalds
2021-09-27  8:21                   ` David Laight
2021-09-27  9:22                     ` Willy Tarreau
2021-09-10 17:11 ` incoming Kees Cook
2021-09-10 20:13   ` incoming Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210910031026.thFt7MLEf%akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=apw@canonical.com \
    --cc=cl@linux.com \
    --cc=danielmicay@gmail.com \
    --cc=dennis@kernel.org \
    --cc=dwaipayanray1@gmail.com \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=joe@perches.com \
    --cc=keescook@chromium.org \
    --cc=linux-mm@kvack.org \
    --cc=lukas.bulwahn@gmail.com \
    --cc=mm-commits@vger.kernel.org \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=ojeda@kernel.org \
    --cc=penberg@kernel.org \
    --cc=rdunlap@infradead.org \
    --cc=rientjes@google.com \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).