lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20241208161315.730138-1-nilay@linux.ibm.com>
Date: Sun,  8 Dec 2024 21:42:28 +0530
From: Nilay Shroff <nilay@...ux.ibm.com>
To: linux-kernel@...r.kernel.org
Cc: Nilay Shroff <nilay@...ux.ibm.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        briannorris@...omium.org, yury.norov@...il.com, kees@...nel.org,
        gustavoars@...nel.org, nathan@...nel.org, steffen.klassert@...unet.com,
        daniel.m.jordan@...cle.com, gjoyce@....com,
        linux-crypto@...r.kernel.org, linux@...ssschuh.net
Subject: [PATCHv3] gcc: disable '-Wstrignop-overread' universally for gcc-13+ and FORTIFY_SOURCE

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@...uxfoundation.org>
Cc: briannorris@...omium.org
Cc: yury.norov@...il.com
Cc: kees@...nel.org
Cc: gustavoars@...nel.org
Cc: nathan@...nel.org
Cc: steffen.klassert@...unet.com
Cc: daniel.m.jordan@...cle.com
Cc: gjoyce@....com
Cc: linux-crypto@...r.kernel.org
Cc: linux@...ssschuh.net
Signed-off-by: Nilay Shroff <nilay@...ux.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


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ