[PATCH] cpumask: work around false-postive stringop-overread errors

Nilay Shroff posted 1 patch 1 year, 2 months ago
There is a newer version of this series
kernel/Makefile | 1 +
1 file changed, 1 insertion(+)
[PATCH] cpumask: work around false-postive stringop-overread errors
Posted by Nilay Shroff 1 year, 2 months ago
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.

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)
 obj-$(CONFIG_PADATA) += padata.o
 obj-$(CONFIG_JUMP_LABEL) += jump_label.o
 obj-$(CONFIG_CONTEXT_TRACKING) += context_tracking.o
-- 
2.45.2

Re: [PATCH] cpumask: work around false-postive stringop-overread errors
Posted by Brian Norris 1 year, 2 months ago
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
> 
Re: [PATCH] cpumask: work around false-postive stringop-overread errors
Posted by Nilay Shroff 1 year, 2 months ago

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