Message ID | 20241205123413.309388-1-nilay@linux.ibm.com |
---|---|
State | New |
Headers | show |
Series | [PATCHv2] cpumask: work around false-postive stringop-overread errors | expand |
On Fri, Dec 06, 2024 at 02:39:58PM +0530, Nilay Shroff wrote: > > Thank you Yuri for insightful comments! Please see my responses inline... > > On 12/5/24 21:53, Yury Norov wrote: > > On Thu, Dec 05, 2024 at 06:04:09PM +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, > >> | ~~~~~~~~~~~~~~^~~~~~~~ > >> > >> Apparently, above errors only manifests 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 errors appear to be false- > > > > If it works for pre-GCC13, and likely for clang, you shouldn't disable it > > for them. It should be enabled for CONFIG_FORTIFY_SOURCE=n, as well. > > > > Check config CC_NO_ARRAY_BOUNDS for an example of how versioned flags > > are implemented. > > > > >> 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. > > > > I'm 100% sure that Brian is a great person and his information is > > absolutely correct and complete. But this is just not how we write > > commit messages. > > > > Please avoid personal references, vague statements and news from > > the future. > > > Sure, I would do the needful for future patches. > > >> 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 didn't provide any evidence that this warning is specific for padata. > > > > Let me just show you the test matrix for the stringop-overread error: > > ARCH PowerPC: > compiler CONFIG_FORTIFY_SOURCE -Werror=stringop-overread > gcc 11.x Y N > gcc 11.x N N > gcc 12.x Y N > gcc 12.x N N > gcc 13.x Y Y > gcc 13.x N N > clang 18.x Y N > clang 18.x N N > > ARCH x86_64: > compiler CONFIG_FORTIFY_SOURCE -Werror=stringop-overread > gcc 11.x Y N > gcc 11.x N N > gcc 12.x Y N > gcc 12.x N N > gcc 13.x Y N > gcc 13.x N N > clang 18.x Y N > clang 18.x N N > > >From the above matrix, we could see that the sringop-overread error is only encountered > when we use gcc 13 on PowerPC machine and the stringop-overread error is seen only when we > enable CONFIG_FORTIFY_SOURCE. Furthermore, so far I've only encountered this error while > compiling kernel/padata.c file. > > > And indeed the subject states that this is a cpumasks-related warninig. > > Cpumask is a global subsystem. If you believe that this warning is > > false-positive, it may show up for any other random victim. Please > > suppress it globally. > > > Yes you were correct, this warning might appear for any other random victims. But as > I mentioned earlier, so far I've only encountered it with kernel/padata.c file. > So, if we want to reduce the blast radius then wouldn't it be sufficient to just silence > it only while compiling kernel/padata.c file? Or do you still suggest disabling it at > global level would be more helpful? I'm OK with either way moving forward. Please let > me know. You will reduce the radius significantly if you limit sringop-overread suppression to a specific config, compiler and architecture. Silencing random files is a gambling.
On Thu, Dec 05, 2024 at 06:04:09PM +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, > | ~~~~~~~~~~~~~~^~~~~~~~ > > Apparently, above errors only manifests 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 errors 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. I'm hitting this now on Linus's tree using gcc14 on x86-64 so this isn't just a problem with your arch. Let me try this patch locally and see if it helps... thanks, greg k-h
On Sat, Dec 07, 2024 at 12:43:19PM +0100, Greg KH wrote: > On Thu, Dec 05, 2024 at 06:04:09PM +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, > > | ~~~~~~~~~~~~~~^~~~~~~~ > > > > Apparently, above errors only manifests 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 errors 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. > > I'm hitting this now on Linus's tree using gcc14 on x86-64 so this isn't > just a problem with your arch. > > Let me try this patch locally and see if it helps... Yes, fixes the build for me, so either this is a real fix, or something else needs to be done for it, so I'll give a: Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> for now. thanks, greg k-h
On 12/7/24 17:14, Greg KH wrote: > On Sat, Dec 07, 2024 at 12:43:19PM +0100, Greg KH wrote: >> On Thu, Dec 05, 2024 at 06:04:09PM +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, >>> | ~~~~~~~~~~~~~~^~~~~~~~ >>> >>> Apparently, above errors only manifests 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 errors 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. >> >> I'm hitting this now on Linus's tree using gcc14 on x86-64 so this isn't >> just a problem with your arch. Thanks Greg for confirming that this is not isolated to PowerPC!! >> >> Let me try this patch locally and see if it helps... > > Yes, fixes the build for me, so either this is a real fix, or something > else needs to be done for it, so I'll give a: > > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > for now. Okay so now we've an evidence confirming that this is no longer PowerPC specific issue. Hence as Yury suggested, in another mail[1], fixing this error by disabling stringop-overread globally for GCC13+ and CONFIG_FORTIFY_SOURCE=n, I will spin a new patch and submit it. [1] https://lore.kernel.org/all/Z1HTdtvNjm-nZSvJ@yury-ThinkPad/ Thanks, --Nilay
On Sun, Dec 08, 2024 at 03:51:10PM +0530, Nilay Shroff wrote: > > > On 12/7/24 17:14, Greg KH wrote: > > On Sat, Dec 07, 2024 at 12:43:19PM +0100, Greg KH wrote: > >> On Thu, Dec 05, 2024 at 06:04:09PM +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, > >>> | ~~~~~~~~~~~~~~^~~~~~~~ > >>> > >>> Apparently, above errors only manifests 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 errors 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. > >> > >> I'm hitting this now on Linus's tree using gcc14 on x86-64 so this isn't > >> just a problem with your arch. > Thanks Greg for confirming that this is not isolated to PowerPC!! > >> > >> Let me try this patch locally and see if it helps... > > > > Yes, fixes the build for me, so either this is a real fix, or something > > else needs to be done for it, so I'll give a: > > > > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > > for now. > Okay so now we've an evidence confirming that this is no longer PowerPC specific > issue. Hence as Yury suggested, in another mail[1], fixing this error by disabling > stringop-overread globally for GCC13+ and CONFIG_FORTIFY_SOURCE=n, I will spin a > new patch and submit it. > > [1] https://lore.kernel.org/all/Z1HTdtvNjm-nZSvJ@yury-ThinkPad/ That feels wrong, unless this is a compiler bug. And if it's a compiler bug, can we fix the compiler please or at least submit a bug to the gcc developers? I'm slowly moving all my boxes/builds over to using clang to build the kernel due to rust kernel work, so I guess I can do that here as well as I don't think this issue shows up for that compiler, right? thanks, greg k-h
On 12/8/24 18:58, Greg KH wrote: > On Sun, Dec 08, 2024 at 03:51:10PM +0530, Nilay Shroff wrote: >> >> >> On 12/7/24 17:14, Greg KH wrote: >>> On Sat, Dec 07, 2024 at 12:43:19PM +0100, Greg KH wrote: >>>> On Thu, Dec 05, 2024 at 06:04:09PM +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, >>>>> | ~~~~~~~~~~~~~~^~~~~~~~ >>>>> >>>>> Apparently, above errors only manifests 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 errors 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. >>>> >>>> I'm hitting this now on Linus's tree using gcc14 on x86-64 so this isn't >>>> just a problem with your arch. >> Thanks Greg for confirming that this is not isolated to PowerPC!! >>>> >>>> Let me try this patch locally and see if it helps... >>> >>> Yes, fixes the build for me, so either this is a real fix, or something >>> else needs to be done for it, so I'll give a: >>> >>> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >>> >>> for now. >> Okay so now we've an evidence confirming that this is no longer PowerPC specific >> issue. Hence as Yury suggested, in another mail[1], fixing this error by disabling >> stringop-overread globally for GCC13+ and CONFIG_FORTIFY_SOURCE=n, I will spin a >> new patch and submit it. >> >> [1] https://lore.kernel.org/all/Z1HTdtvNjm-nZSvJ@yury-ThinkPad/ > > That feels wrong, unless this is a compiler bug. And if it's a compiler > bug, can we fix the compiler please or at least submit a bug to the gcc > developers? > Yes this seems to be a compiler bug. Please see here : [1] https://lore.kernel.org/all/202411021337.85E9BB06@keescook/ [2] https://gcc.gnu.org/pipermail/gcc-patches/2024-October/666872.html > I'm slowly moving all my boxes/builds over to using clang to build the > kernel due to rust kernel work, so I guess I can do that here as well as > I don't think this issue shows up for that compiler, right? > Yes this error doesn't show up for LLVM/clang. We've two options here: 1) To disable this error globally for GCC-13+ until we find the root cause. Maybe when GCC folks add more diagnostics and contexts around -Wstringop-* compiler warning as discussed in [2] above. or 2) Silence this error only for file kernel/padata.c compiling which this error manifests as of today. Yury suggested option #1 above so that we may avoid random victims hitting this error. What do you suggest? Thanks, --Nilay
On Sun, Dec 08, 2024 at 07:21:48PM +0530, Nilay Shroff wrote: > > > On 12/8/24 18:58, Greg KH wrote: > > On Sun, Dec 08, 2024 at 03:51:10PM +0530, Nilay Shroff wrote: > >> > >> > >> On 12/7/24 17:14, Greg KH wrote: > >>> On Sat, Dec 07, 2024 at 12:43:19PM +0100, Greg KH wrote: > >>>> On Thu, Dec 05, 2024 at 06:04:09PM +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, > >>>>> | ~~~~~~~~~~~~~~^~~~~~~~ > >>>>> > >>>>> Apparently, above errors only manifests 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 errors 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. > >>>> > >>>> I'm hitting this now on Linus's tree using gcc14 on x86-64 so this isn't > >>>> just a problem with your arch. > >> Thanks Greg for confirming that this is not isolated to PowerPC!! > >>>> > >>>> Let me try this patch locally and see if it helps... > >>> > >>> Yes, fixes the build for me, so either this is a real fix, or something > >>> else needs to be done for it, so I'll give a: > >>> > >>> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > >>> > >>> for now. > >> Okay so now we've an evidence confirming that this is no longer PowerPC specific > >> issue. Hence as Yury suggested, in another mail[1], fixing this error by disabling > >> stringop-overread globally for GCC13+ and CONFIG_FORTIFY_SOURCE=n, I will spin a > >> new patch and submit it. > >> > >> [1] https://lore.kernel.org/all/Z1HTdtvNjm-nZSvJ@yury-ThinkPad/ > > > > That feels wrong, unless this is a compiler bug. And if it's a compiler > > bug, can we fix the compiler please or at least submit a bug to the gcc > > developers? > > > Yes this seems to be a compiler bug. Please see here : > [1] https://lore.kernel.org/all/202411021337.85E9BB06@keescook/ > [2] https://gcc.gnu.org/pipermail/gcc-patches/2024-October/666872.html > > > I'm slowly moving all my boxes/builds over to using clang to build the > > kernel due to rust kernel work, so I guess I can do that here as well as > > I don't think this issue shows up for that compiler, right? > > > Yes this error doesn't show up for LLVM/clang. We've two options here: > 1) To disable this error globally for GCC-13+ until we find the root cause. Maybe when > GCC folks add more diagnostics and contexts around -Wstringop-* compiler warning as > discussed in [2] above. > or > 2) Silence this error only for file kernel/padata.c compiling which this error manifests > as of today. > > Yury suggested option #1 above so that we may avoid random victims hitting this error. > What do you suggest? I suggest the hardening maintainers should decide, as this is their area and feature they are supporting, not me :) thanks, greg k-h
On 12/8/24 19:27, Greg KH wrote: > On Sun, Dec 08, 2024 at 07:21:48PM +0530, Nilay Shroff wrote: >> >> >> On 12/8/24 18:58, Greg KH wrote: >>> On Sun, Dec 08, 2024 at 03:51:10PM +0530, Nilay Shroff wrote: >>>> >>>> >>>> On 12/7/24 17:14, Greg KH wrote: >>>>> On Sat, Dec 07, 2024 at 12:43:19PM +0100, Greg KH wrote: >>>>>> On Thu, Dec 05, 2024 at 06:04:09PM +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, >>>>>>> | ~~~~~~~~~~~~~~^~~~~~~~ >>>>>>> >>>>>>> Apparently, above errors only manifests 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 errors 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. >>>>>> >>>>>> I'm hitting this now on Linus's tree using gcc14 on x86-64 so this isn't >>>>>> just a problem with your arch. >>>> Thanks Greg for confirming that this is not isolated to PowerPC!! >>>>>> >>>>>> Let me try this patch locally and see if it helps... >>>>> >>>>> Yes, fixes the build for me, so either this is a real fix, or something >>>>> else needs to be done for it, so I'll give a: >>>>> >>>>> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >>>>> >>>>> for now. >>>> Okay so now we've an evidence confirming that this is no longer PowerPC specific >>>> issue. Hence as Yury suggested, in another mail[1], fixing this error by disabling >>>> stringop-overread globally for GCC13+ and CONFIG_FORTIFY_SOURCE=n, I will spin a >>>> new patch and submit it. >>>> >>>> [1] https://lore.kernel.org/all/Z1HTdtvNjm-nZSvJ@yury-ThinkPad/ >>> >>> That feels wrong, unless this is a compiler bug. And if it's a compiler >>> bug, can we fix the compiler please or at least submit a bug to the gcc >>> developers? >>> >> Yes this seems to be a compiler bug. Please see here : >> [1] https://lore.kernel.org/all/202411021337.85E9BB06@keescook/ >> [2] https://gcc.gnu.org/pipermail/gcc-patches/2024-October/666872.html >> >>> I'm slowly moving all my boxes/builds over to using clang to build the >>> kernel due to rust kernel work, so I guess I can do that here as well as >>> I don't think this issue shows up for that compiler, right? >>> >> Yes this error doesn't show up for LLVM/clang. We've two options here: >> 1) To disable this error globally for GCC-13+ until we find the root cause. Maybe when >> GCC folks add more diagnostics and contexts around -Wstringop-* compiler warning as >> discussed in [2] above. >> or >> 2) Silence this error only for file kernel/padata.c compiling which this error manifests >> as of today. >> >> Yury suggested option #1 above so that we may avoid random victims hitting this error. >> What do you suggest? > > I suggest the hardening maintainers should decide, as this is their area > and feature they are supporting, not me :) > Alright, then I would go ahead with option #1, as Yury suggested, for now while I spin next patch and keep in Cc all hardening maintainers to decide the disposition. Thanks, --Nilay
diff --git a/kernel/Makefile b/kernel/Makefile index 87866b037fbe..03242d8870c7 100644 --- a/kernel/Makefile +++ b/kernel/Makefile @@ -120,6 +120,10 @@ obj-$(CONFIG_CFI_CLANG) += cfi.o obj-$(CONFIG_PERF_EVENTS) += events/ obj-$(CONFIG_USER_RETURN_NOTIFIER) += user-return-notifier.o + +# Silence the false positive stringop-overread errors on GCC 13+ +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