* [PATCH -mm v2] stackdepot: fix -Wstringop-overflow warning
@ 2024-02-01 9:04 Marco Elver
2024-02-01 14:08 ` Andrey Konovalov
2024-02-01 22:09 ` Stephen Rothwell
0 siblings, 2 replies; 3+ messages in thread
From: Marco Elver @ 2024-02-01 9:04 UTC (permalink / raw)
To: elver, Andrew Morton
Cc: Andrey Konovalov, Alexander Potapenko, Dmitry Vyukov,
Vlastimil Babka, linux-kernel, linux-mm, kasan-dev,
Stephen Rothwell
Since 113a61863ecb ("Makefile: Enable -Wstringop-overflow globally")
string overflow checking is enabled by default. Within stackdepot, the
compiler (GCC 13.2.0) assumes that a multiplication 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
This is due to depot_alloc_stack() accepting an 'int nr_entries' which
could be negative without deeper analysis of callers.
The call to depot_alloc_stack() from stack_depot_save_flags(), however,
only passes in its nr_entries which is unsigned int. Fix the warning by
switching depot_alloc_stack()'s nr_entries to also be unsigned.
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>
---
v2:
* Just switch 'nr_entries' to unsigned int which is already the case
elsewhere.
---
lib/stackdepot.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/stackdepot.c b/lib/stackdepot.c
index 8f3b2c84ec2d..4a7055a63d9f 100644
--- a/lib/stackdepot.c
+++ b/lib/stackdepot.c
@@ -420,7 +420,7 @@ static inline size_t depot_stack_record_size(struct stack_record *s, unsigned in
/* Allocates a new stack in a stack depot pool. */
static struct stack_record *
-depot_alloc_stack(unsigned long *entries, int nr_entries, u32 hash, depot_flags_t flags, void **prealloc)
+depot_alloc_stack(unsigned long *entries, unsigned int nr_entries, u32 hash, depot_flags_t flags, void **prealloc)
{
struct stack_record *stack = NULL;
size_t record_size;
--
2.43.0.429.g432eaa2c6b-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH -mm v2] stackdepot: fix -Wstringop-overflow warning
2024-02-01 9:04 [PATCH -mm v2] stackdepot: fix -Wstringop-overflow warning Marco Elver
@ 2024-02-01 14:08 ` Andrey Konovalov
2024-02-01 22:09 ` Stephen Rothwell
1 sibling, 0 replies; 3+ messages in thread
From: Andrey Konovalov @ 2024-02-01 14:08 UTC (permalink / raw)
To: Marco Elver
Cc: Andrew Morton, Alexander Potapenko, Dmitry Vyukov,
Vlastimil Babka, linux-kernel, linux-mm, kasan-dev,
Stephen Rothwell
On Thu, Feb 1, 2024 at 10:04 AM Marco Elver <elver@google.com> wrote:
>
> Since 113a61863ecb ("Makefile: Enable -Wstringop-overflow globally")
> string overflow checking is enabled by default. Within stackdepot, the
> compiler (GCC 13.2.0) assumes that a multiplication 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
>
> This is due to depot_alloc_stack() accepting an 'int nr_entries' which
> could be negative without deeper analysis of callers.
>
> The call to depot_alloc_stack() from stack_depot_save_flags(), however,
> only passes in its nr_entries which is unsigned int. Fix the warning by
> switching depot_alloc_stack()'s nr_entries to also be unsigned.
>
> 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>
> ---
> v2:
> * Just switch 'nr_entries' to unsigned int which is already the case
> elsewhere.
> ---
> lib/stackdepot.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/stackdepot.c b/lib/stackdepot.c
> index 8f3b2c84ec2d..4a7055a63d9f 100644
> --- a/lib/stackdepot.c
> +++ b/lib/stackdepot.c
> @@ -420,7 +420,7 @@ static inline size_t depot_stack_record_size(struct stack_record *s, unsigned in
>
> /* Allocates a new stack in a stack depot pool. */
> static struct stack_record *
> -depot_alloc_stack(unsigned long *entries, int nr_entries, u32 hash, depot_flags_t flags, void **prealloc)
> +depot_alloc_stack(unsigned long *entries, unsigned int nr_entries, u32 hash, depot_flags_t flags, void **prealloc)
> {
> struct stack_record *stack = NULL;
> size_t record_size;
> --
> 2.43.0.429.g432eaa2c6b-goog
>
Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH -mm v2] stackdepot: fix -Wstringop-overflow warning
2024-02-01 9:04 [PATCH -mm v2] stackdepot: fix -Wstringop-overflow warning Marco Elver
2024-02-01 14:08 ` Andrey Konovalov
@ 2024-02-01 22:09 ` Stephen Rothwell
1 sibling, 0 replies; 3+ messages in thread
From: Stephen Rothwell @ 2024-02-01 22:09 UTC (permalink / raw)
To: Marco Elver
Cc: Andrew Morton, Andrey Konovalov, Alexander Potapenko,
Dmitry Vyukov, Vlastimil Babka, linux-kernel, linux-mm,
kasan-dev
[-- Attachment #1: Type: text/plain, Size: 2663 bytes --]
Hi all,
On Thu, 1 Feb 2024 10:04:30 +0100 Marco Elver <elver@google.com> wrote:
>
> Since 113a61863ecb ("Makefile: Enable -Wstringop-overflow globally")
> string overflow checking is enabled by default. Within stackdepot, the
> compiler (GCC 13.2.0) assumes that a multiplication 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
>
> This is due to depot_alloc_stack() accepting an 'int nr_entries' which
> could be negative without deeper analysis of callers.
>
> The call to depot_alloc_stack() from stack_depot_save_flags(), however,
> only passes in its nr_entries which is unsigned int. Fix the warning by
> switching depot_alloc_stack()'s nr_entries to also be unsigned.
>
> 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>
> ---
> v2:
> * Just switch 'nr_entries' to unsigned int which is already the case
> elsewhere.
> ---
> lib/stackdepot.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/stackdepot.c b/lib/stackdepot.c
> index 8f3b2c84ec2d..4a7055a63d9f 100644
> --- a/lib/stackdepot.c
> +++ b/lib/stackdepot.c
> @@ -420,7 +420,7 @@ static inline size_t depot_stack_record_size(struct stack_record *s, unsigned in
>
> /* Allocates a new stack in a stack depot pool. */
> static struct stack_record *
> -depot_alloc_stack(unsigned long *entries, int nr_entries, u32 hash, depot_flags_t flags, void **prealloc)
> +depot_alloc_stack(unsigned long *entries, unsigned int nr_entries, u32 hash, depot_flags_t flags, void **prealloc)
> {
> struct stack_record *stack = NULL;
> size_t record_size;
> --
> 2.43.0.429.g432eaa2c6b-goog
>
I have applied this patch to the merge of the mm tree today.
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-02-01 22:09 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-01 9:04 [PATCH -mm v2] stackdepot: fix -Wstringop-overflow warning Marco Elver
2024-02-01 14:08 ` Andrey Konovalov
2024-02-01 22:09 ` Stephen Rothwell
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).