[PATCH] compiler.h: Raise minimum version of GCC to 5.1 for arm64

[PATCH] compiler.h: Raise minimum version of GCC to 5.1 for arm64

[Will Deacon]

GCC versions >= 4.9 and < 5.1 have been shown to emit memory references
beyond the stack pointer, resulting in memory corruption if an interrupt
is taken after the stack pointer has been adjusted but before the
reference has been executed. This leads to subtle, infrequent data
corruption such as the EXT4 problems reported by Russell King at the
link below.

Life is too short for buggy compilers, so raise the minimum GCC version
required by arm64 to 5.1.

Cc: Theodore Ts'o <tytso@mit.edu>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Florian Weimer <fweimer@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Reported-by: Russell King <linux@armlinux.org.uk>
Suggested-by: Arnd Bergmann <arnd@kernel.org>
Link: https://lore.kernel.org/r/20210105154726.GD1551@shell.armlinux.org.uk
Signed-off-by: Will Deacon <will@kernel.org>
---
 include/linux/compiler-gcc.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 74c6c0486eed..555ab0fddbef 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -13,6 +13,12 @@
 /* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 */
 #if GCC_VERSION < 40900
 # error Sorry, your version of GCC is too old - please use 4.9 or newer.
+#elif defined(CONFIG_ARM64) && GCC_VERSION < 50100
+/*
+ * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293
+ * https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk
+ */
+# error Sorry, your version of GCC is too old - please use 5.1 or newer.
 #endif
 
 /*
-- 
2.30.0.284.gd98b1dd5eaa7-goog

Re: [PATCH] compiler.h: Raise minimum version of GCC to 5.1 for arm64

[Nick Desaulniers]

On Tue, Jan 12, 2021 at 2:48 PM Will Deacon <will@kernel.org> wrote:
>
> GCC versions >= 4.9 and < 5.1 have been shown to emit memory references
> beyond the stack pointer, resulting in memory corruption if an interrupt
> is taken after the stack pointer has been adjusted but before the
> reference has been executed. This leads to subtle, infrequent data
> corruption such as the EXT4 problems reported by Russell King at the
> link below.
>
> Life is too short for buggy compilers, so raise the minimum GCC version
> required by arm64 to 5.1.
>
> Cc: Theodore Ts'o <tytso@mit.edu>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Florian Weimer <fweimer@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Reported-by: Russell King <linux@armlinux.org.uk>
> Suggested-by: Arnd Bergmann <arnd@kernel.org>
> Link: https://lore.kernel.org/r/20210105154726.GD1551@shell.armlinux.org.uk
> Signed-off-by: Will Deacon <will@kernel.org>

Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>  include/linux/compiler-gcc.h | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index 74c6c0486eed..555ab0fddbef 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -13,6 +13,12 @@
>  /* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 */
>  #if GCC_VERSION < 40900
>  # error Sorry, your version of GCC is too old - please use 4.9 or newer.
> +#elif defined(CONFIG_ARM64) && GCC_VERSION < 50100
> +/*
> + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293
> + * https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk
> + */
> +# error Sorry, your version of GCC is too old - please use 5.1 or newer.
>  #endif
>
>  /*
> --
> 2.30.0.284.gd98b1dd5eaa7-goog
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH] compiler.h: Raise minimum version of GCC to 5.1 for arm64

[Nathan Chancellor]

On Tue, Jan 12, 2021 at 10:48:32PM +0000, Will Deacon wrote:
> GCC versions >= 4.9 and < 5.1 have been shown to emit memory references
> beyond the stack pointer, resulting in memory corruption if an interrupt
> is taken after the stack pointer has been adjusted but before the
> reference has been executed. This leads to subtle, infrequent data
> corruption such as the EXT4 problems reported by Russell King at the
> link below.
> 
> Life is too short for buggy compilers, so raise the minimum GCC version
> required by arm64 to 5.1.
> 
> Cc: Theodore Ts'o <tytso@mit.edu>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Florian Weimer <fweimer@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Reported-by: Russell King <linux@armlinux.org.uk>
> Suggested-by: Arnd Bergmann <arnd@kernel.org>
> Link: https://lore.kernel.org/r/20210105154726.GD1551@shell.armlinux.org.uk
> Signed-off-by: Will Deacon <will@kernel.org>

Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>

> ---
>  include/linux/compiler-gcc.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index 74c6c0486eed..555ab0fddbef 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -13,6 +13,12 @@
>  /* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 */
>  #if GCC_VERSION < 40900
>  # error Sorry, your version of GCC is too old - please use 4.9 or newer.
> +#elif defined(CONFIG_ARM64) && GCC_VERSION < 50100
> +/*
> + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293
> + * https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk
> + */
> +# error Sorry, your version of GCC is too old - please use 5.1 or newer.
>  #endif
>  
>  /*
> -- 
> 2.30.0.284.gd98b1dd5eaa7-goog
> 

Re: [PATCH] compiler.h: Raise minimum version of GCC to 5.1 for arm64

[Linus Torvalds]

On Tue, Jan 12, 2021 at 2:48 PM Will Deacon <will@kernel.org> wrote:
>
> Life is too short for buggy compilers, so raise the minimum GCC version
> required by arm64 to 5.1.

Ack. I'll assume I get this the usual ways from the arm64 tree..

           Linus

Re: [PATCH] compiler.h: Raise minimum version of GCC to 5.1 for arm64

[Linus Torvalds]

On Tue, Jan 12, 2021 at 6:14 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Ack. I'll assume I get this the usual ways from the arm64 tree..

Oh.. Actually, while you can use my ack if you decide to go this way,
I do wonder if it might not be better to introduce a notion of an
error at Kconfig time, and then we could make this whole GCC_VERSION
check be something that gets covered much earlier - when configuring
the kernel, rather than randomly (ok, very early) when building it.

We already have the CONFIG_GCC_VERSION config variable, after all.

And Kconfig already has an error functionality, which it uses for
things like compilers not found etc.

So something like

     $(error-if,CC_IS_GCC && GCC_VERSION < 90100,"Gcc version too old")

in the arm64 Kconfig file should do it.

Adding Masahiro, because I couldn't actually get it to work. I'm
probably doing something wrong, but it might also be that it cannot
depend on config variables currently (our only use is for running
shell script tests unconditionally).

           Linus

Re: [PATCH] compiler.h: Raise minimum version of GCC to 5.1 for arm64

[Catalin Marinas]

On Tue, Jan 12, 2021 at 06:35:50PM -0800, Linus Torvalds wrote:
> On Tue, Jan 12, 2021 at 6:14 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > Ack. I'll assume I get this the usual ways from the arm64 tree..
> 
> Oh.. Actually, while you can use my ack if you decide to go this way,
> I do wonder if it might not be better to introduce a notion of an
> error at Kconfig time, and then we could make this whole GCC_VERSION
> check be something that gets covered much earlier - when configuring
> the kernel, rather than randomly (ok, very early) when building it.
> 
> We already have the CONFIG_GCC_VERSION config variable, after all.
> 
> And Kconfig already has an error functionality, which it uses for
> things like compilers not found etc.
> 
> So something like
> 
>      $(error-if,CC_IS_GCC && GCC_VERSION < 90100,"Gcc version too old")
> 
> in the arm64 Kconfig file should do it.

$(error-if) seems to expect a y/n as a condition. We do have $(failure)
and $(success) but they translate a (shell) command's return code to
y/n. Even with something like:

config GCC_IS_OLD
	def_bool CC_IS_GCC && GCC_VERSION < ...

I can't get $(error-if,GCC_IS_OLD) to expand the config value, no matter
what other. GCC_VERSION is also a config option in your example.

I'll queue Will's patch in the meantime.

-- 
Catalin

Re: [PATCH] compiler.h: Raise minimum version of GCC to 5.1 for arm64

[Catalin Marinas]

On Tue, 12 Jan 2021 22:48:32 +0000, Will Deacon wrote:
> GCC versions >= 4.9 and < 5.1 have been shown to emit memory references
> beyond the stack pointer, resulting in memory corruption if an interrupt
> is taken after the stack pointer has been adjusted but before the
> reference has been executed. This leads to subtle, infrequent data
> corruption such as the EXT4 problems reported by Russell King at the
> link below.
> 
> [...]

Applied to arm64 (for-next/fixes), thanks!

[1/1] compiler.h: Raise minimum version of GCC to 5.1 for arm64
      https://git.kernel.org/arm64/c/1f1244a5ddb7

-- 
Catalin

Re: [PATCH] compiler.h: Raise minimum version of GCC to 5.1 for arm64

[Masahiro Yamada]

On Thu, Jan 14, 2021 at 1:08 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Tue, 12 Jan 2021 22:48:32 +0000, Will Deacon wrote:
> > GCC versions >= 4.9 and < 5.1 have been shown to emit memory references
> > beyond the stack pointer, resulting in memory corruption if an interrupt
> > is taken after the stack pointer has been adjusted but before the
> > reference has been executed. This leads to subtle, infrequent data
> > corruption such as the EXT4 problems reported by Russell King at the
> > link below.
> >
> > [...]
>
> Applied to arm64 (for-next/fixes), thanks!
>
> [1/1] compiler.h: Raise minimum version of GCC to 5.1 for arm64
>       https://git.kernel.org/arm64/c/1f1244a5ddb7
>
> --
> Catalin
>


Maybe, we can raise the minimal version to gcc 5.1
for all architectures.


-- 
Best Regards
Masahiro Yamada

Re: [PATCH] compiler.h: Raise minimum version of GCC to 5.1 for arm64

[Joe Perches]

On Thu, 2021-01-14 at 02:57 +0900, Masahiro Yamada wrote:
> On Thu, Jan 14, 2021 at 1:08 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > 
> > On Tue, 12 Jan 2021 22:48:32 +0000, Will Deacon wrote:
> > > GCC versions >= 4.9 and < 5.1 have been shown to emit memory references
> > > beyond the stack pointer, resulting in memory corruption if an interrupt
> > > is taken after the stack pointer has been adjusted but before the
> > > reference has been executed. This leads to subtle, infrequent data
> > > corruption such as the EXT4 problems reported by Russell King at the
> > > link below.
> > > 
> > > [...]
> > 
> > Applied to arm64 (for-next/fixes), thanks!
> > 
> > [1/1] compiler.h: Raise minimum version of GCC to 5.1 for arm64
> >       https://git.kernel.org/arm64/c/1f1244a5ddb7
[]
> Maybe, we can raise the minimal version to gcc 5.1
> for all architectures.

If really raising the required minimum, maybe use 7.1 so kasan v5
and fallthrough are always enabled too.

https://gcc.gnu.org/releases.html

GCC 7.1 	May 2, 2017
GCC 5.1 	April 22, 2015
GCC 4.9.0 	April 22, 2014

Re: [PATCH] compiler.h: Raise minimum version of GCC to 5.1 for arm64

[Linus Torvalds]

On Wed, Jan 13, 2021 at 9:58 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Maybe, we can raise the minimal version to gcc 5.1
> for all architectures.

It was discussed, but the immediate reason for this thing really does
seem to be specific to just arm64 (ie this is not some generic gcc
stack access bug that just happens to rear its head on arm64 - the
patch to fix this in the gcc bugzilla is very much arm64-only).

            Linus

Re: [PATCH] compiler.h: Raise minimum version of GCC to 5.1 for arm64

[Russell King - ARM Linux admin]

On Wed, Jan 13, 2021 at 11:15:09AM -0800, Linus Torvalds wrote:
> On Wed, Jan 13, 2021 at 9:58 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > Maybe, we can raise the minimal version to gcc 5.1
> > for all architectures.
> 
> It was discussed, but the immediate reason for this thing really does
> seem to be specific to just arm64 (ie this is not some generic gcc
> stack access bug that just happens to rear its head on arm64 - the
> patch to fix this in the gcc bugzilla is very much arm64-only).

I seem to remember during the discussion of the arm64 problem, that
there was a similar bug on e.g. sparc, but they patched the kernel.
*digs through irc logs...*

https://patchwork.kernel.org/project/linux-crypto/patch/20170602.112854.571030442583332811.davem@davemloft.net/
https://marc.info/?l=linux-sparc&m=149636946609980&w=2

(and they even reference the arm64 bug). If you move on two messages,
then the disassembly clearly shows that it is the same bug on Sparc.

DaveM came up with the following to fix it:

commit d41519a69b35b10af7fda867fb9100df24fdf403
Author: David Miller <davem@davemloft.net>
Date:   Fri Jun 2 11:28:54 2017 -0400

    crypto: Work around deallocated stack frame reference gcc bug on sparc.

which added a bunch of barriers across the kernel to cater for this,
but for them, ext4 was not impacted at that time.

Apparently, davem's justification for not changing ext4 was:

"Actually, ext4 doesn't trigger the problem because the on-stack object
used in ext4 is a fixed size at compile time"

So, maybe the Sparc issue was just a similar but different bug in gcc
4.9.x.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH] compiler.h: Raise minimum version of GCC to 5.1 for arm64

[Linus Torvalds]

On Wed, Jan 13, 2021 at 1:44 PM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> So, maybe the Sparc issue was just a similar but different bug in gcc
> 4.9.x.

Good catch. And I know this bug has happened independently on
different architectures several times (I remember this on x86-64 as
well), so I started looking around.

And in fact, 4.9 was buggy on x86-64 too:

    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61904

And yeah, _that_ gcc bug wasn't actually x86-64 specific, but
apparently a generic instruction scheduling bug.

So it's an independent bug, but I do have to admit that the arguments
against 4.9 are piling up (even if that particular fix apparently got
fixed in the gcc branches and apparently backported to distro
compilers too).

         Linus

Re: [PATCH] compiler.h: Raise minimum version of GCC to 5.1 for arm64

[Ard Biesheuvel]

On Wed, 13 Jan 2021 at 23:09, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Wed, Jan 13, 2021 at 1:44 PM Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> >
> > So, maybe the Sparc issue was just a similar but different bug in gcc
> > 4.9.x.
>
> Good catch. And I know this bug has happened independently on
> different architectures several times (I remember this on x86-64 as
> well), so I started looking around.
>
> And in fact, 4.9 was buggy on x86-64 too:
>
>     https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61904
>
> And yeah, _that_ gcc bug wasn't actually x86-64 specific, but
> apparently a generic instruction scheduling bug.
>
> So it's an independent bug, but I do have to admit that the arguments
> against 4.9 are piling up (even if that particular fix apparently got
> fixed in the gcc branches and apparently backported to distro
> compilers too).
>

So if the arguments are piling up, what is holding us back, other than
inertia? RHEL 7 used to be a factor, but it ships with 4.8 not 4.9, so
its users already need to upgrade. Is anyone aware of a good reason to
keep 4.9 supported? Are any other long term supported distros using
4.9.x?

I know that distros probably backported fixes for all of these issues,
but without a way to interrogate the compiler about this, that doesn't
really make a difference IMHO.

Note that banning 4.9 for arm64 and banning it in general should be
two different changes in any case, as the former will need to be
backported to -stable kernels as well.

 --
Ard.

Re: [PATCH] compiler.h: Raise minimum version of GCC to 5.1 for arm64

[Linus Torvalds]

On Thu, Jan 14, 2021 at 12:18 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> So if the arguments are piling up, what is holding us back, other than
> inertia?

I think we can most certainly just try increasing the minimum version
to 5.1 in the next merge window and see.

> Note that banning 4.9 for arm64 and banning it in general should be
> two different changes in any case, as the former will need to be
> backported to -stable kernels as well.

Yes. The arm64 issue is a clear and known bug, plus I suspect gcc-4.9
is ridiculously old in the arm64 ecosystem anyway.

So the arm64 issue is a bug-fix, the follow-up of just upgrading gcc
requirements in general would be a "keep up with the times, and allow
those variable declarations in loops".

            Linus

Re: [PATCH] compiler.h: Raise minimum version of GCC to 5.1 for arm64

[Joe Perches]

On Thu, 2021-01-14 at 10:43 -0800, Linus Torvalds wrote:
> On Thu, Jan 14, 2021 at 12:18 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > 
> > So if the arguments are piling up, what is holding us back, other than
> > inertia?
> 
> I think we can most certainly just try increasing the minimum version
> to 5.1 in the next merge window and see.
> 
> > Note that banning 4.9 for arm64 and banning it in general should be
> > two different changes in any case, as the former will need to be
> > backported to -stable kernels as well.
> 
> Yes. The arm64 issue is a clear and known bug, plus I suspect gcc-4.9
> is ridiculously old in the arm64 ecosystem anyway.
> 
> So the arm64 issue is a bug-fix, the follow-up of just upgrading gcc
> requirements in general would be a "keep up with the times, and allow
> those variable declarations in loops".

Given the upgrade requirement, and how clang version requirements
constantly change, how much more difficult would it be for others
to use gcc 7.1 or higher now instead of later?

Re: [PATCH] compiler.h: Raise minimum version of GCC to 5.1 for arm64

[Linus Torvalds]

On Thu, Jan 14, 2021 at 11:52 AM Joe Perches <joe@perches.com> wrote:
>
> Given the upgrade requirement, and how clang version requirements
> constantly change, how much more difficult would it be for others
> to use gcc 7.1 or higher now instead of later?

What was the argument for jumping all the way to gcc-7.1?

I do think we want to have real reasons we can point to, rather than a
"just because".

            Linus

Re: [PATCH] compiler.h: Raise minimum version of GCC to 5.1 for arm64

[Joe Perches]

On Thu, 2021-01-14 at 13:18 -0800, Linus Torvalds wrote:
> On Thu, Jan 14, 2021 at 11:52 AM Joe Perches <joe@perches.com> wrote:
> > 
> > Given the upgrade requirement, and how clang version requirements
> > constantly change, how much more difficult would it be for others
> > to use gcc 7.1 or higher now instead of later?
> 
> What was the argument for jumping all the way to gcc-7.1?
> 
> I do think we want to have real reasons we can point to, rather than a
> "just because".

KASAN v5 instead of all the old versions
gcc 7.1 supports fallthrough.

Probably more, but those might be sufficient.

Re: [PATCH] compiler.h: Raise minimum version of GCC to 5.1 for arm64

[Linus Torvalds]

On Thu, Jan 14, 2021 at 4:30 PM Joe Perches <joe@perches.com> wrote:
>
> KASAN v5 instead of all the old versions
> gcc 7.1 supports fallthrough.

Considering that most people won't even enable KASAN, I think that's
not a huge reason to then force people to potentially upgrade their
compilers.

That said, I do think that it might be good to just try to standardize
on the KASAN v5 code - but that could easily be done by simply making
KASAN depend as a feature on having a newer compiler version.

So the KASAN option itself could just have something like

       depends on !CC_IS_GCC || GCC_VERSION >= 70000

in it, which would allow us to just say "only v5 need apply".

As to "fallthrough", I think it's more than enough that lots of people
build with compilers that support it, and then any warnings they see
will be fixed. No need to force upgrades - it doesn't buy kernel
developers anything.

          Linus

Re: [PATCH] compiler.h: Raise minimum version of GCC to 5.1 for arm64

[Masahiro Yamada]

Hi Linus,


On Fri, Jan 15, 2021 at 3:43 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Thu, Jan 14, 2021 at 12:18 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > So if the arguments are piling up, what is holding us back, other than
> > inertia?
>
> I think we can most certainly just try increasing the minimum version
> to 5.1 in the next merge window and see.


Just a friendly reminder.

It will be nice to increase the minimum GCC version
to GCC 5.x globally.

Now, the version check has been moved to
scripts/cc-version.sh, which is invoked from
Kconfig.

If you decide to do this, please update
'gcc_min_version' in that file.


I'd like to suggest GCC 5.2 instead of 5.1
so that we can drop the following line
from arch/powerpc/Kconfig.

if GCC_VERSION >= 50200   # plugin support on gcc <= 5.1 is buggy on PPC





> > Note that banning 4.9 for arm64 and banning it in general should be
> > two different changes in any case, as the former will need to be
> > backported to -stable kernels as well.
>
> Yes. The arm64 issue is a clear and known bug, plus I suspect gcc-4.9
> is ridiculously old in the arm64 ecosystem anyway.
>
> So the arm64 issue is a bug-fix, the follow-up of just upgrading gcc
> requirements in general would be a "keep up with the times, and allow
> those variable declarations in loops".
>
>             Linus



-- 
Best Regards
Masahiro Yamada