All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marco Elver <elver@google.com>
To: elver@google.com, Andrew Morton <akpm@linux-foundation.org>
Cc: Andrey Konovalov <andreyknvl@gmail.com>,
	Alexander Potapenko <glider@google.com>,
	 Dmitry Vyukov <dvyukov@google.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	linux-kernel@vger.kernel.org,  linux-mm@kvack.org,
	kasan-dev@googlegroups.com,
	 Stephen Rothwell <sfr@canb.auug.org.au>,
	"Gustavo A . R . Silva" <gustavoars@kernel.org>,
	 Kees Cook <keescook@chromium.org>
Subject: [PATCH -mm] stackdepot: do not use flex_array_size() in memcpy()
Date: Thu,  1 Feb 2024 09:31:35 +0100	[thread overview]
Message-ID: <20240201083259.1734865-1-elver@google.com> (raw)

Since 113a61863ecb ("Makefile: Enable -Wstringop-overflow globally")
string overflow checking is enabled by default. Unfortunately the
compiler still isn't smart enough to always see that the size will never
overflow.

Specifically, in stackdepot, we have this before memcpy()'ing a
stacktrace:

  if (nr_entries > CONFIG_STACKDEPOT_MAX_FRAMES)
  	nr_entries = CONFIG_STACKDEPOT_MAX_FRAMES;
  ...
  memcpy(stack->entries, entries, flex_array_size(stack, entries, nr_entries));

Where 'entries' is an array of unsigned long, and STACKDEPOT_MAX_FRAMES
is 64 by default (configurable up to 256), thus the maximum size in
bytes (on 32-bit) would be 1024. For some reason the compiler (GCC
13.2.0) assumes that an overflow may be possible and flex_array_size()
can return SIZE_MAX (4294967295 on 32-bit), resulting in this warning:

 In function 'depot_alloc_stack',
     inlined from 'stack_depot_save_flags' at lib/stackdepot.c:688:4:
 arch/x86/include/asm/string_32.h:150:25: error: '__builtin_memcpy' specified bound 4294967295 exceeds maximum object size 2147483647 [-Werror=stringop-overflow=]
   150 | #define memcpy(t, f, n) __builtin_memcpy(t, f, n)
       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~
 lib/stackdepot.c:459:9: note: in expansion of macro 'memcpy'
   459 |         memcpy(stack->entries, entries, flex_array_size(stack, entries, nr_entries));
       |         ^~~~~~
 cc1: all warnings being treated as errors

Silence the false positive warning by inlining the multiplication
ourselves.

Link: https://lore.kernel.org/all/20240201135747.18eca98e@canb.auug.org.au/
Fixes: d869d3fb362c ("stackdepot: use variable size records for non-evictable entries")
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Marco Elver <elver@google.com>
Cc: Gustavo A. R. Silva <gustavoars@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
---
 lib/stackdepot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/stackdepot.c b/lib/stackdepot.c
index 8f3b2c84ec2d..e6047f58ad62 100644
--- a/lib/stackdepot.c
+++ b/lib/stackdepot.c
@@ -456,7 +456,7 @@ depot_alloc_stack(unsigned long *entries, int nr_entries, u32 hash, depot_flags_
 	stack->hash = hash;
 	stack->size = nr_entries;
 	/* stack->handle is already filled in by depot_pop_free_pool(). */
-	memcpy(stack->entries, entries, flex_array_size(stack, entries, nr_entries));
+	memcpy(stack->entries, entries, nr_entries * sizeof(entries[0]));
 
 	if (flags & STACK_DEPOT_FLAG_GET) {
 		refcount_set(&stack->count, 1);
-- 
2.43.0.429.g432eaa2c6b-goog


             reply	other threads:[~2024-02-01  8:35 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-01  8:31 Marco Elver [this message]
2024-02-01  8:52 ` [PATCH -mm] stackdepot: do not use flex_array_size() in memcpy() Marco Elver

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=20240201083259.1734865-1-elver@google.com \
    --to=elver@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=andreyknvl@gmail.com \
    --cc=dvyukov@google.com \
    --cc=glider@google.com \
    --cc=gustavoars@kernel.org \
    --cc=kasan-dev@googlegroups.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=sfr@canb.auug.org.au \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.