* Re: [PATCH] cpumask: work around false-postive stringop-overread errors
[not found] <20241112124127.1666300-1-nilay@linux.ibm.com>
@ 2024-12-04 19:38 ` Brian Norris
2024-12-05 12:05 ` Nilay Shroff
0 siblings, 1 reply; 2+ messages in thread
From: Brian Norris @ 2024-12-04 19:38 UTC (permalink / raw)
To: Nilay Shroff
Cc: linux-kernel, kees, nathan, yury.norov, linux, gjoyce,
Steffen Klassert, Daniel Jordan, linux-crypto
Hi Nilay,
I see you didn't CC the maintainers for this file. You might consider
looking through:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html
And:
$ scripts/get_maintainer.pl -f kernel/padata.c
Steffen Klassert <steffen.klassert@secunet.com> (maintainer:PADATA PARALLEL EXECUTION MECHANISM)
Daniel Jordan <daniel.m.jordan@oracle.com> (maintainer:PADATA PARALLEL EXECUTION MECHANISM)
linux-crypto@vger.kernel.org (open list:PADATA PARALLEL EXECUTION MECHANISM)
linux-kernel@vger.kernel.org (open list:PADATA PARALLEL EXECUTION MECHANISM)
I'll leave the full contents intact below for their sake, with a few
inline comments as well:
On Tue, Nov 12, 2024 at 06:11:24PM +0530, Nilay Shroff wrote:
> While building the powerpc code using gcc 13, I came across following
> errors generated for kernel/padata.c file:
>
> CC kernel/padata.o
> In file included from ./include/linux/string.h:390,
> from ./arch/powerpc/include/asm/paca.h:16,
> from ./arch/powerpc/include/asm/current.h:13,
> from ./include/linux/thread_info.h:23,
> from ./include/asm-generic/preempt.h:5,
> from ./arch/powerpc/include/generated/asm/preempt.h:1,
> from ./include/linux/preempt.h:79,
> from ./include/linux/spinlock.h:56,
> from ./include/linux/swait.h:7,
> from ./include/linux/completion.h:12,
> from kernel/padata.c:14:
> In function ‘bitmap_copy’,
> inlined from ‘cpumask_copy’ at ./include/linux/cpumask.h:839:2,
> inlined from ‘__padata_set_cpumasks’ at kernel/padata.c:730:2:
> ./include/linux/fortify-string.h:114:33: error: ‘__builtin_memcpy’ reading between 257 and 536870904 bytes from a region of size 256 [-Werror=stringop-overread]
> 114 | #define __underlying_memcpy __builtin_memcpy
> | ^
> ./include/linux/fortify-string.h:633:9: note: in expansion of macro ‘__underlying_memcpy’
> 633 | __underlying_##op(p, q, __fortify_size); \
> | ^~~~~~~~~~~~~
> ./include/linux/fortify-string.h:678:26: note: in expansion of macro ‘__fortify_memcpy_chk’
> 678 | #define memcpy(p, q, s) __fortify_memcpy_chk(p, q, s, \
> | ^~~~~~~~~~~~~~~~~~~~
> ./include/linux/bitmap.h:259:17: note: in expansion of macro ‘memcpy’
> 259 | memcpy(dst, src, len);
> | ^~~~~~
> kernel/padata.c: In function ‘__padata_set_cpumasks’:
> kernel/padata.c:713:48: note: source object ‘pcpumask’ of size [0, 256]
> 713 | cpumask_var_t pcpumask,
> | ~~~~~~~~~~~~~~^~~~~~~~
> In function ‘bitmap_copy’,
> inlined from ‘cpumask_copy’ at ./include/linux/cpumask.h:839:2,
> inlined from ‘__padata_set_cpumasks’ at kernel/padata.c:730:2:
> ./include/linux/fortify-string.h:114:33: error: ‘__builtin_memcpy’ reading between 257 and 536870904 bytes from a region of size 256 [-Werror=stringop-overread]
> 114 | #define __underlying_memcpy __builtin_memcpy
> | ^
> ./include/linux/fortify-string.h:633:9: note: in expansion of macro ‘__underlying_memcpy’
> 633 | __underlying_##op(p, q, __fortify_size); \
> | ^~~~~~~~~~~~~
> ./include/linux/fortify-string.h:678:26: note: in expansion of macro ‘__fortify_memcpy_chk’
> 678 | #define memcpy(p, q, s) __fortify_memcpy_chk(p, q, s, \
> | ^~~~~~~~~~~~~~~~~~~~
> ./include/linux/bitmap.h:259:17: note: in expansion of macro ‘memcpy’
> 259 | memcpy(dst, src, len);
> | ^~~~~~
> kernel/padata.c: In function ‘__padata_set_cpumasks’:
> kernel/padata.c:713:48: note: source object ‘pcpumask’ of size [0, 256]
> 713 | cpumask_var_t pcpumask,
> | ~~~~~~~~~~~~~~^~~~~~~~
>
> Apparentrly, above errors only menifests with GCC 13.x and config option
> CONFIG_FORTIFY_SOURCE. Furthermore, if I use gcc 11.x or gcc 12.x then I
> don't encounter above errors. Prima facie, these erros appear to be false-
> positive. Brian informed me that currently some efforts are underway by
> GCC developers to emit more verbose information when GCC detects string
> overflow errors and that might help to further narrow down the root cause
> of this error. So for now, silence these errors using -Wno-stringop-
> overread gcc option while building kernel/padata.c file until we find the
> root cause.
You might consider running this paragraph through a spelling checker if you submit a
v2.
> Link: https://lore.kernel.org/all/7cbbd751-8332-4ab2-afa7-8c353834772a@linux.ibm.com/
> Cc: briannorris@chromium.org
> Cc: kees@kernel.org
> Cc: nathan@kernel.org
> Cc: yury.norov@gmail.com
> Cc: linux@weissschuh.net
> Cc: gjoyce@ibm.com
> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
> ---
> kernel/Makefile | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/kernel/Makefile b/kernel/Makefile
> index 87866b037fbe..e5adba7a30f1 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -120,6 +120,7 @@ obj-$(CONFIG_CFI_CLANG) += cfi.o
> obj-$(CONFIG_PERF_EVENTS) += events/
>
> obj-$(CONFIG_USER_RETURN_NOTIFIER) += user-return-notifier.o
> +CFLAGS_padata.o += $(call cc-disable-warning, stringop-overread)
Personally, I'd recommend a comment here, noting that these warnings
seem to produce false positives on GCC 13+. But otherwise, this seems OK
to me:
Reviewed-by: Brian Norris <briannorris@chromium.org>
> obj-$(CONFIG_PADATA) += padata.o
> obj-$(CONFIG_JUMP_LABEL) += jump_label.o
> obj-$(CONFIG_CONTEXT_TRACKING) += context_tracking.o
> --
> 2.45.2
>
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] cpumask: work around false-postive stringop-overread errors
2024-12-04 19:38 ` [PATCH] cpumask: work around false-postive stringop-overread errors Brian Norris
@ 2024-12-05 12:05 ` Nilay Shroff
0 siblings, 0 replies; 2+ messages in thread
From: Nilay Shroff @ 2024-12-05 12:05 UTC (permalink / raw)
To: Brian Norris
Cc: linux-kernel, kees, nathan, yury.norov, linux, gjoyce,
Steffen Klassert, Daniel Jordan, linux-crypto
On 12/5/24 01:08, Brian Norris wrote:
> Hi Nilay,
>
> I see you didn't CC the maintainers for this file. You might consider
> looking through:
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
> And:
>
> $ scripts/get_maintainer.pl -f kernel/padata.c
> Steffen Klassert <steffen.klassert@secunet.com> (maintainer:PADATA PARALLEL EXECUTION MECHANISM)
> Daniel Jordan <daniel.m.jordan@oracle.com> (maintainer:PADATA PARALLEL EXECUTION MECHANISM)
> linux-crypto@vger.kernel.org (open list:PADATA PARALLEL EXECUTION MECHANISM)
> linux-kernel@vger.kernel.org (open list:PADATA PARALLEL EXECUTION MECHANISM)
>
> I'll leave the full contents intact below for their sake, with a few
> inline comments as well:
>
> On Tue, Nov 12, 2024 at 06:11:24PM +0530, Nilay Shroff wrote:
>> While building the powerpc code using gcc 13, I came across following
>> errors generated for kernel/padata.c file:
>>
>> CC kernel/padata.o
>> In file included from ./include/linux/string.h:390,
>> from ./arch/powerpc/include/asm/paca.h:16,
>> from ./arch/powerpc/include/asm/current.h:13,
>> from ./include/linux/thread_info.h:23,
>> from ./include/asm-generic/preempt.h:5,
>> from ./arch/powerpc/include/generated/asm/preempt.h:1,
>> from ./include/linux/preempt.h:79,
>> from ./include/linux/spinlock.h:56,
>> from ./include/linux/swait.h:7,
>> from ./include/linux/completion.h:12,
>> from kernel/padata.c:14:
>> In function ‘bitmap_copy’,
>> inlined from ‘cpumask_copy’ at ./include/linux/cpumask.h:839:2,
>> inlined from ‘__padata_set_cpumasks’ at kernel/padata.c:730:2:
>> ./include/linux/fortify-string.h:114:33: error: ‘__builtin_memcpy’ reading between 257 and 536870904 bytes from a region of size 256 [-Werror=stringop-overread]
>> 114 | #define __underlying_memcpy __builtin_memcpy
>> | ^
>> ./include/linux/fortify-string.h:633:9: note: in expansion of macro ‘__underlying_memcpy’
>> 633 | __underlying_##op(p, q, __fortify_size); \
>> | ^~~~~~~~~~~~~
>> ./include/linux/fortify-string.h:678:26: note: in expansion of macro ‘__fortify_memcpy_chk’
>> 678 | #define memcpy(p, q, s) __fortify_memcpy_chk(p, q, s, \
>> | ^~~~~~~~~~~~~~~~~~~~
>> ./include/linux/bitmap.h:259:17: note: in expansion of macro ‘memcpy’
>> 259 | memcpy(dst, src, len);
>> | ^~~~~~
>> kernel/padata.c: In function ‘__padata_set_cpumasks’:
>> kernel/padata.c:713:48: note: source object ‘pcpumask’ of size [0, 256]
>> 713 | cpumask_var_t pcpumask,
>> | ~~~~~~~~~~~~~~^~~~~~~~
>> In function ‘bitmap_copy’,
>> inlined from ‘cpumask_copy’ at ./include/linux/cpumask.h:839:2,
>> inlined from ‘__padata_set_cpumasks’ at kernel/padata.c:730:2:
>> ./include/linux/fortify-string.h:114:33: error: ‘__builtin_memcpy’ reading between 257 and 536870904 bytes from a region of size 256 [-Werror=stringop-overread]
>> 114 | #define __underlying_memcpy __builtin_memcpy
>> | ^
>> ./include/linux/fortify-string.h:633:9: note: in expansion of macro ‘__underlying_memcpy’
>> 633 | __underlying_##op(p, q, __fortify_size); \
>> | ^~~~~~~~~~~~~
>> ./include/linux/fortify-string.h:678:26: note: in expansion of macro ‘__fortify_memcpy_chk’
>> 678 | #define memcpy(p, q, s) __fortify_memcpy_chk(p, q, s, \
>> | ^~~~~~~~~~~~~~~~~~~~
>> ./include/linux/bitmap.h:259:17: note: in expansion of macro ‘memcpy’
>> 259 | memcpy(dst, src, len);
>> | ^~~~~~
>> kernel/padata.c: In function ‘__padata_set_cpumasks’:
>> kernel/padata.c:713:48: note: source object ‘pcpumask’ of size [0, 256]
>> 713 | cpumask_var_t pcpumask,
>> | ~~~~~~~~~~~~~~^~~~~~~~
>>
>> Apparentrly, above errors only menifests with GCC 13.x and config option
>> CONFIG_FORTIFY_SOURCE. Furthermore, if I use gcc 11.x or gcc 12.x then I
>> don't encounter above errors. Prima facie, these erros appear to be false-
>> positive. Brian informed me that currently some efforts are underway by
>> GCC developers to emit more verbose information when GCC detects string
>> overflow errors and that might help to further narrow down the root cause
>> of this error. So for now, silence these errors using -Wno-stringop-
>> overread gcc option while building kernel/padata.c file until we find the
>> root cause.
>
> You might consider running this paragraph through a spelling checker if you submit a
> v2.
>
>> Link: https://lore.kernel.org/all/7cbbd751-8332-4ab2-afa7-8c353834772a@linux.ibm.com/
>> Cc: briannorris@chromium.org
>> Cc: kees@kernel.org
>> Cc: nathan@kernel.org
>> Cc: yury.norov@gmail.com
>> Cc: linux@weissschuh.net
>> Cc: gjoyce@ibm.com
>> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
>> ---
>> kernel/Makefile | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/kernel/Makefile b/kernel/Makefile
>> index 87866b037fbe..e5adba7a30f1 100644
>> --- a/kernel/Makefile
>> +++ b/kernel/Makefile
>> @@ -120,6 +120,7 @@ obj-$(CONFIG_CFI_CLANG) += cfi.o
>> obj-$(CONFIG_PERF_EVENTS) += events/
>>
>> obj-$(CONFIG_USER_RETURN_NOTIFIER) += user-return-notifier.o
>> +CFLAGS_padata.o += $(call cc-disable-warning, stringop-overread)
>
> Personally, I'd recommend a comment here, noting that these warnings
> seem to produce false positives on GCC 13+. But otherwise, this seems OK
> to me:
>
> Reviewed-by: Brian Norris <briannorris@chromium.org>
>
>> obj-$(CONFIG_PADATA) += padata.o
>> obj-$(CONFIG_JUMP_LABEL) += jump_label.o
>> obj-$(CONFIG_CONTEXT_TRACKING) += context_tracking.o
>> --
>> 2.45.2
>>
Thank you Brian for your review!
I would incorporate all your comments and spin a new patch and send it out soon...
--Nilay
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-12-05 12:06 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20241112124127.1666300-1-nilay@linux.ibm.com>
2024-12-04 19:38 ` [PATCH] cpumask: work around false-postive stringop-overread errors Brian Norris
2024-12-05 12:05 ` Nilay Shroff
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox