linux-kernel - Re: [PATCHv3] gcc: disable '-Wstrignop-overread' universally for gcc-13+ and FORTIFY

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241209200300.GB1597021@ax162>
Date: Mon, 9 Dec 2024 13:03:00 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Nilay Shroff <nilay@...ux.ibm.com>, linux-kernel@...r.kernel.org,
	briannorris@...omium.org, yury.norov@...il.com, kees@...nel.org,
	gustavoars@...nel.org, steffen.klassert@...unet.com,
	daniel.m.jordan@...cle.com, gjoyce@....com,
	linux-crypto@...r.kernel.org, linux@...ssschuh.net
Subject: Re: [PATCHv3] gcc: disable '-Wstrignop-overread' universally for
 gcc-13+ and FORTIFY_SOURCE

On Mon, Dec 09, 2024 at 07:45:51AM +0100, Greg Kroah-Hartman wrote:
> > 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
> 
> I don't want this disabled for all files in the kernel, we only have one
> that this is a problem for.  I think you disable this, the whole fortify
> logic is disabled which is not the goal, why not just force the fortify
> feature OFF if we have a "bad compiler" that can not support it?

I am glad we agree that disabling -Wstringop-overread for the kernel
entirely is the wrong way to approach this but I think that turning off
ALL of FORTIFY_SOURCE for these compiler versions (which are some of the
latest available) is also the wrong approach, especially in this current
situation where it seems like it is unreasonable to expect the compiler
not to warn about a potential overread here with the amount of
information available to it but maybe I am misunderstanding something
here:

https://lore.kernel.org/20241209193558.GA1597021@ax162/

If it is not possible to shut the compiler up by changing the source to
be more robust, I think we should strongly consider disabling it in
directly in cpumask_copy() using the __diag infrastructure. I think it
is underutilized as a solution to warnings like this.

> And it's odd that we are the only 2 people hitting it, has everyone else
> just given up on gcc and moved on to using clang?

Maybe people are not using CONFIG_WERROR=y and W=e when hitting this so
they do not notice? It also only became visible in 6.12 because of the
'inline' -> '__always_inline' changes in bitmap.h and cpumask.h, since
prior to that, the size of the objects being passed to memcpy() were not
known, so FORTIFY could not catch them (another +1 for that change).
>From what I can tell, it also requires a CONFIG_NR_CPUS value of 256 or
greater, otherwise large_cpumask_bits is NR_CPUS, a compile time
constant.

Fedora clearly sees this though but it does not break their builds so I
am guessing they have not reported it upstream yet?

https://koji.fedoraproject.org/koji/taskinfo?taskID=126644404
https://kojipkgs.fedoraproject.org//work/tasks/4404/126644404/build.log

For the record, clang has no warning like this because with how clang is
currently architectured, the vast majority of warnings happen in the
front end, where there is usually not enough reliable information
available to make these kind of warnings be accurate, at least from my
understanding.

Cheers,
Nathan