[PATCHv3] gcc: disable '-Wstrignop-overread' universally for gcc-13+ and FORTIFY_SOURCE

Nilay Shroff
2024-12-08T16:14:22Z

While building the powerpc code using gcc 13.x, 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,
| ~~~~~~~~~~~~~~^~~~~~~~

The above error appears to be false-positive:
From the distro relevant config,
CONFIG_PPC64=y
CONFIG_CC_IS_GCC=y
CONFIG_GCC_VERSION=130301
CONFIG_NR_CPUS=2048
CONFIG_FORTIFY_SOURCE=y

From the source code,
unsigned long name[BITS_TO_LONGS(bits)]
...
typedef struct cpumask { DECLARE_BITMAP(bits, NR_CPUS); } cpumask_t;
typedef struct cpumask cpumask_var_t[1];
...

extern unsigned int nr_cpu_ids;
...
static __always_inline
void bitmap_copy(unsigned long *dst, const unsigned long *src, unsigned
int nbits)
{
unsigned int len = bitmap_size(nbits);

if (small_const_nbits(nbits))
*dst = *src;
else
memcpy(dst, src, len);
}

static __always_inline
void cpumask_copy(struct cpumask *dstp, const struct cpumask *srcp)
{
bitmap_copy(cpumask_bits(dstp), cpumask_bits(srcp), large_cpumask_bits);
}
...
static int __padata_set_cpumasks(struct padata_instance *pinst,
cpumask_var_t pcpumask,
cpumask_var_t cbcpumask)
{
...
cpumask_copy(pinst->cpumask.pcpu, pcpumask);
cpumask_copy(pinst->cpumask.cbcpu, cbcpumask);
...
}

So the above statements expands to:
memcpy(pinst->cpumask.pcpu->bits, pcpumask->bits, nr_cpu_ids)
memcpy(pinst->cpumask.cbcpu->bits, cbcpumask->bits, nr_cpu_ids)

Now the compiler complains about "error: ‘__builtin_memcpy’ reading
between 257 and 536870904 bytes from a region of size 256". So the
value of nr_cpu_ids which gcc calculated is between 257 and 536870904.
This looks strange and incorrect.

Later similar errors[1] were also observed on x86-64 platform using
gcc-14. Apparently, above errors menifests using gcc-13+ and config
option CONFIG_FORTIFY_SOURCE=y. Moreover, I don't encounter above
error when,
- using gcc 11.x or gcc 12.x or LLVM/Clang compiler
or
- disabling CONFIG_FORTIFY_SOURCE option

So for now, silence above errors globally while using gcc-13+ and
CONFIG_FORTIFY_SOURCE=y. We may later revert this change when we
find root cause of the error. Per this discussion[2], GCC developers
are working to add extra diagnostics and context when this error
menifests and that might help to find the root cause.

[1] https://lore.kernel.org/all/2024120757-lustrous-equinox-77f0@gregkh/
[2] https://gcc.gnu.org/pipermail/gcc-patches/2024-October/666870.html

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: briannorris@chromium.org
Cc: yury.norov@gmail.com
Cc: kees@kernel.org
Cc: gustavoars@kernel.org
Cc: nathan@kernel.org
Cc: steffen.klassert@secunet.com
Cc: daniel.m.jordan@oracle.com
Cc: gjoyce@ibm.com
Cc: linux-crypto@vger.kernel.org
Cc: linux@weissschuh.net
Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
Changes from v2:
- Globally disable -Werror-stringop-overread while using gcc-13+ and
FORTIFY_SOURCE (Yury Norov)
- Updated the patch subject line to make it clear that this is
possiblt gcc error and not related to cpumask.
Changes from v1:
- Fix spell error in the commit message (Brian Norris)
- Add commentary around change to note that changes are needed to
avoid false positive on gcc 13+ (Brian Norris)
- Add the kerenl/padata.c file maintainers (Brian Norris)
---
init/Kconfig | 8 ++++++++
scripts/Makefile.extrawarn | 1 +
2 files changed, 9 insertions(+)

diff --git a/init/Kconfig b/init/Kconfig
index a20e6efd3f0f..ff2f54520551 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -920,6 +920,14 @@ config CC_STRINGOP_OVERFLOW
bool
default y if CC_IS_GCC && !CC_NO_STRINGOP_OVERFLOW

+# Currently, disable -Wstringop-overread for gcc-13+ and FORTIFY_SOURCE globally.
+config GCC13_NO_STRINGOP_OVERREAD
+ def_bool y
+
+config CC_NO_STRINGOP_OVERREAD
+ bool
+ default y if CC_IS_GCC && GCC_VERSION >= 130000 && GCC13_NO_STRINGOP_OVERREAD && FORTIFY_SOURCE
+
#
# For architectures that know their GCC __int128 support is sound
#
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 1d13cecc7cc7..1abd41269fd0 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -27,6 +27,7 @@ endif
KBUILD_CPPFLAGS-$(CONFIG_WERROR) += -Werror
KBUILD_CPPFLAGS += $(KBUILD_CPPFLAGS-y)
KBUILD_CFLAGS-$(CONFIG_CC_NO_ARRAY_BOUNDS) += -Wno-array-bounds
+KBUILD_CFLAGS-$(CONFIG_CC_NO_STRINGOP_OVERREAD) += -Wno-stringop-overread

ifdef CONFIG_CC_IS_CLANG
# The kernel builds with '-std=gnu11' so use of GNU extensions is acceptable.
--
2.45.2