ARM: Blacklist GCC 4.8.0 to GCC 4.8.2 - PR58854
PATCH 1/1 ARM: Blacklist GCC 4.8.0 to GCC 4.8.2 - PR58854
These stock GCC versions miscompile the kernel by incorrectly optimising the function epilogue code - by first increasing the stack pointer, and then loading entries from below the stack. This means that an opportune interrupt or exception will corrupt the current function's saved state, which may result in the parent function seeing different register values.
As this bug has been known to result in corrupted filesystems, and these buggy compiler versions seem to be frequently used, we have little option but to blacklist these compiler versions.
Distributions may have fixed PR58854, but as their compilers are totally indistinguishable from the buggy versions, it is unfortunate that this also results in those also being blacklisted. Given the filesystem corruption potential of the original, this is the lesser evil. People who want to build with their fixed compiler versions will need to adjust the kernel source. (Distros need to think about the implications of fixing such a compiler bug, and consider how to ensure that their fixed compiler versions can be detected if they wish to avoid this.)
---
This is what I came up with - this places the build check right at the beginning of the kernel build, rather than at some point where linux/compiler.h gets included. Note that this is where we have previous ARM specific GCC version blacklisting. I'm blacklisting GCC 4.8.0 to GCC 4.8.2 inclussive, which seems to be the right versions for stock GCC.
I was in two minds whether to include 4.8.3 as Linaro released a buggy toolchain which identifies itself as 4.8.3, but I decided that's also a distro problem. IMHO Linaro should really think about taking that compiler down given the seriousness of this bug and it being indistinguishable from the fixed stock version.
arch/arm/kernel/asm-offsets.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
arch/arm/kernel/asm-offsets.c +11 -1
@@ -10,6 +10,7 @@* it under the terms of the GNU General Public License version 2 as * published by the Free Software Foundation. */ +#include <linux/compiler.h> #include <linux/sched.h> #include <linux/mm.h> #include <linux/dma-mapping.h>
@@ -39,10 +40,19 @@* GCC 3.2.x: miscompiles NEW_AUX_ENT in fs/binfmt_elf.c * (http://gcc.gnu.org/PR8896) and incorrect structure * initialisation in fs/jffs2/erase.c +* GCC 4.8.0-4.8.2: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58854 +* miscompiles find_get_entry(), and can result in EXT3 and EXT4 +* filesystem corruption (possibly other FS too). */ +#ifdef __GNUC__#if (__GNUC__ ==3 && __GNUC_MINOR__ <3 )#error Your compiler is too buggy; it is known to miscompile kernels. -#error Known good compilers: 3.3 +#error Known good compilers: 3.3, 4.x +#endif +#if GCC_VERSION >=40800 ||GCC_VERSION <40803 +#error Your compiler is too buggy; it is known to miscompile kernels +#error and result in filesystem corruption and oopses. +#endif #endif int main (void )
Signature · 1 line
1.8.3.1
PATCH 1/1 ARM: Blacklist GCC 4.8.0 to GCC 4.8.2 - PR58854
Quoted 30 lines · Russell King
These stock GCC versions miscompile the kernel by incorrectly optimising the function epilogue code - by first increasing the stack pointer, and then loading entries from below the stack. This means that an opportune interrupt or exception will corrupt the current function's saved state, which may result in the parent function seeing different register values.
As this bug has been known to result in corrupted filesystems, and these buggy compiler versions seem to be frequently used, we have little option but to blacklist these compiler versions.
Distributions may have fixed PR58854, but as their compilers are totally indistinguishable from the buggy versions, it is unfortunate that this also results in those also being blacklisted. Given the filesystem corruption potential of the original, this is the lesser evil. People who want to build with their fixed compiler versions will need to adjust the kernel source. (Distros need to think about the implications of fixing such a compiler bug, and consider how to ensure that their fixed compiler versions can be detected if they wish to avoid this.)
Signed-off-by Russell King <[email protected]>---
This is what I came up with - this places the build check right at the beginning of the kernel build, rather than at some point where
linux/compiler.hgets included. Note that this is where we have previous ARM specific GCC version blacklisting. I'm blacklisting GCC 4.8.0 to GCC 4.8.2 inclussive, which seems to be the right versions for stock GCC.
Quoted 5 lines
I was in two minds whether to include 4.8.3 as Linaro released a buggy toolchain which identifies itself as 4.8.3, but I decided that's also a distro problem. IMHO Linaro should really think about taking that compiler down given the seriousness of this bug and it being indistinguishable from the fixed stock version.
Maybe it's unfair to blame them; Linaro just took a snapshot and released what was there.
If gcc is going to retain the "change release number then add all the new features" model, some kind of prerelease indicator would help eliminate this kind of problem. And that indicator should be both a preprocessor define and parseable from the command line :)
Quoted 20 lines
arch/arm/kernel/asm-offsets.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)arch/arm/kernel/asm-offsets.c +2
-0@@ -10,6 +10,7 @@* it under the terms of the GNU General Public License version 2 as * published by the Free Software Foundation. */ +#include <linux/compiler.h> #include <linux/sched.h> #include <linux/mm.h> #include <linux/dma-mapping.h> @@ -39,10 +40,19 @@* GCC 3.2.x: miscompiles NEW_AUX_ENT in fs/binfmt_elf.c * (http://gcc.gnu.org/PR8896) and incorrect structure * initialisation in fs/jffs2/erase.c +* GCC 4.8.0-4.8.2: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58854
arch/arm/kernel/asm-offsets.c line 52
Quoted 10 lines
arch/arm/kernel/asm-offsets.c +6
-1+* miscompiles find_get_entry(), and can result in EXT3 and EXT4 +* filesystem corruption (possibly other FS too). */ +#ifdef __GNUC__#if (__GNUC__ ==3 && __GNUC_MINOR__ <3 )#error Your compiler is too buggy; it is known to miscompile kernels. -#error Known good compilers: 3.3 +#error Known good compilers: 3.3, 4.x +#endif +#if GCC_VERSION >=40800 ||GCC_VERSION <40803
^^
&&arch/arm/kernel/asm-offsets.c line 58
Quoted 7 lines
arch/arm/kernel/asm-offsets.c +3
-0+#error Your compiler is too buggy; it is known to miscompile kernels +#error and result in filesystem corruption and oopses. +#endif #endif int main (void )
Regards,
Peter Hurley
PATCH 1/1 ARM: Blacklist GCC 4.8.0 to GCC 4.8.2 - PR58854
Quoted 7 lines · Peter Hurley
Quoted 5 lines · Russell King
I was in two minds whether to include 4.8.3 as Linaro released a buggy toolchain which identifies itself as 4.8.3, but I decided that's also a distro problem. IMHO Linaro should really think about taking that compiler down given the seriousness of this bug and it being indistinguishable from the fixed stock version.
Quoted 7 lines
Maybe it's unfair to blame them; Linaro just took a snapshot and released what was there.
If gcc is going to retain the "change release number then add all the new features" model, some kind of prerelease indicator would help eliminate this kind of problem. And that indicator should be both a preprocessor define and parseable from the command line :)
My comment is not to attribute blame to them, my comment is entirely on a technical level.
My reasoning is that the bug is just as prevalent in userspace, though it will occur less often. Any program which uses signal handlers is a candidate for exactly the same kind of corruption, since you can receive that signal between the point that the stack pointer is modified and the function loads the parent context.
Of course, there are ways around that: don't use signal handlers, or if you do, use alternate signal stacks. Neither of those can be guaranteed for any program though.
So, let me put this another way: a compiler with this bug is _completely_ unsuitable for use for compiling programs for use under the Linux kernel _as well_ as the Linux kernel itself.
The difference is that the Linaro compilers come with an expectation that they are usable on ARM... whereas stock versions cover a lot more and so the ARM arch is probably very small number of their users.
Hence why I recommend that Linaro takes down their buggy compiler. Their 4.8.3 version should not be used *anywhere*, just the same as the stock 4.8 to 4.8.2 inclusive should also not be used anywhere on ARM either.
Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net.
PATCH 1/1 ARM: Blacklist GCC 4.8.0 to GCC 4.8.2 - PR58854
Quoted 4 lines · Russell King - ARM Linux
Hence why I recommend that Linaro takes down their buggy compiler. Their 4.8.3 version should not be used *anywhere*, just the same as the stock 4.8 to 4.8.2 inclusive should also not be used anywhere on ARM either.
Completely agree.
PATCH 1/1 ARM: Blacklist GCC 4.8.0 to GCC 4.8.2 - PR58854
In reply to Russell King - ARM LinuxQuoted 4 lines · Russell King - ARM Linux
So, let me put this another way: a compiler with this bug is _completely_ unsuitable for use for compiling programs for use under the Linux kernel
_aswell_as the Linux kernel itself.
Quoted 8 lines
The difference is that the Linaro compilers come with an expectation that they are usable on ARM... whereas stock versions cover a lot more and so the ARM arch is probably very small number of their users.
Hence why I recommend that Linaro takes down their buggy compiler. Their 4.8.3 version should not be used *anywhere*, just the same as the stock 4.8 to 4.8.2 inclusive should also not be used anywhere on ARM either.
Here's the answer from the toolchain team:
On Thu, 16 Oct 2014, Yvan Roux <[email protected]> wrote:
| Hi Nicolas,
|
| thanks for bringing this to our knowledge.
|
| The fix for PR58854 was included in our releases since the GCC
| 4.8-2013.12 which is based on a 4.8.3 prerelease version (at svn
| revision 205577). As we are doing monthly releases based on a
| revision of the related FSF branch, using GCC_VERSION (__GNUC__,
| __GNUC_MINOR,__GNUC_PATCHLEVEL) is not accurate enough to identify the
| release version (all our releases between November 2013 and May 2014
| will be 4.8.3) the __VERSION__ predefined macro is a bit more accurate
| here ("4.8.3 20131202 (prerelease)") but not completely satisfactory.
|
| I completely agree that we should at least mention in our impacted
| releases download pages that this bug is present.Nicolas
PATCH 1/1 ARM: Blacklist GCC 4.8.0 to GCC 4.8.2 - PR58854
Quoted 19 lines · Nicolas Pitre
Quoted 4 lines · Russell King - ARM Linux
So, let me put this another way: a compiler with this bug is _completely_ unsuitable for use for compiling programs for use under the Linux kernel
_aswell_as the Linux kernel itself.Quoted 8 lines
The difference is that the Linaro compilers come with an expectation that they are usable on ARM... whereas stock versions cover a lot more and so the ARM arch is probably very small number of their users.
Hence why I recommend that Linaro takes down their buggy compiler. Their 4.8.3 version should not be used *anywhere*, just the same as the stock 4.8 to 4.8.2 inclusive should also not be used anywhere on ARM either.
Here's the answer from the toolchain team:
On Thu, 16 Oct 2014, Yvan Roux <[email protected]> wrote:
Quoted 15 lines
| Hi Nicolas,
|
| thanks for bringing this to our knowledge.
|
| The fix for PR58854 was included in our releases since the GCC
| 4.8-2013.12 which is based on a 4.8.3 prerelease version (at svn
| revision 205577). As we are doing monthly releases based on a
| revision of the related FSF branch, using GCC_VERSION (__GNUC__,
| __GNUC_MINOR,__GNUC_PATCHLEVEL) is not accurate enough to identify the
| release version (all our releases between November 2013 and May 2014
| will be 4.8.3) the __VERSION__ predefined macro is a bit more accurate
| here ("4.8.3 20131202 (prerelease)") but not completely satisfactory.
|
| I completely agree that we should at least mention in our impacted
| releases download pages that this bug is present.Unfortunately, __VERSION__ doesn't help us identify that the bug has been fixed - it's not something which can be tested at preprocessor time.
Those which identify themselves as 4.8.3 won't be impacted by the patch, which means that the Linaro 4.8.3 GCC versions will continue to build the kernel just fine. Even the buggy versions.
I don't see the point of continuing to offer the buggy versions for download though - the compiler is totally unsuitable for building any Linux related binaries, be that kernel or userspace, and so should not be used under any circumstances, period.
Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net.
PATCH 1/1 ARM: Blacklist GCC 4.8.0 to GCC 4.8.2 - PR58854
Quoted 40 lines · Russell King - ARM Linux
Quoted 19 lines · Nicolas Pitre
Quoted 4 lines · Russell King - ARM Linux
So, let me put this another way: a compiler with this bug is _completely_ unsuitable for use for compiling programs for use under the Linux kernel
_aswell_as the Linux kernel itself.Quoted 8 lines
The difference is that the Linaro compilers come with an expectation that they are usable on ARM... whereas stock versions cover a lot more and so the ARM arch is probably very small number of their users.
Hence why I recommend that Linaro takes down their buggy compiler. Their 4.8.3 version should not be used *anywhere*, just the same as the stock 4.8 to 4.8.2 inclusive should also not be used anywhere on ARM either.
Here's the answer from the toolchain team:
On Thu, 16 Oct 2014, Yvan Roux <[email protected]> wrote:
Quoted 15 lines
| Hi Nicolas, | | thanks for bringing this to our knowledge. | | The fix for PR58854 was included in our releases since the GCC | 4.8-2013.12 which is based on a 4.8.3 prerelease version (at svn | revision 205577). As we are doing monthly releases based on a | revision of the related FSF branch, using GCC_VERSION (__GNUC__, | __GNUC_MINOR,__GNUC_PATCHLEVEL) is not accurate enough to identify the | release version (all our releases between November 2013 and May 2014 | will be 4.8.3) the __VERSION__ predefined macro is a bit more accurate | here ("4.8.3 20131202 (prerelease)") but not completely satisfactory. | | I completely agree that we should at least mention in our impacted | releases download pages that this bug is present.Unfortunately,
__VERSION__doesn't help us identify that the bug has been fixed - it's not something which can be tested at preprocessor time.
Quoted 8 lines
Those which identify themselves as 4.8.3 won't be impacted by the patch, which means that the Linaro 4.8.3 GCC versions will continue to build the kernel just fine. Even the buggy versions.
I don't see the point of continuing to offer the buggy versions for download though - the compiler is totally unsuitable for building any Linux related binaries, be that kernel or userspace, and so should not be used under any circumstances, period.
Right, with the test fixed (&& instead of ||), I've just pushed it out this morning, and received the results from Olof's kernel builder, which shows that he's using a gcc 4.8.x version which gets caught by the test.
I think Olof needs to do something about his gcc version there.
Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net.
A message that is not in the archive (sent off-list or not yet received): CA+55aFw_N=E9O-7VknWTzjNgYG-tB-8736diA1r7K=50b+0czQ@mail.gmail.com
PATCH 1/1 ARM: Blacklist GCC 4.8.0 to GCC 4.8.2 - PR58854
Quoted 8 lines · Linus Torvalds
Quoted 6 lines · Russell King - ARM Linux
Right, with the test fixed (&& instead of ||), I've just pushed it out this morning, and received the results from Olof's kernel builder, which shows that he's using a gcc 4.8.x version which gets caught by the test.
Quoted 5 lines
Quoted 1 line
I think Olof needs to do something about his gcc version there.
... or maybe you could just add a "I have a working compiler" config option override to show people to say "I have one of the affected versions, but my compiler has the fix"?
Does that work for things like allnoconfig and randconfig? I guess people /could/ seed those appropriately, but that seems to be something that Olof wants to avoid doing.
Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net.
PATCH 1/1 ARM: Blacklist GCC 4.8.0 to GCC 4.8.2 - PR58854
Quoted 4 lines · Russell King - ARM Linux
Does that work for things like allnoconfig and randconfig? I guess people /could/ seed those appropriately, but that seems to be something that Olof wants to avoid doing.
You can force particular config options for those, using something like
export KCONFIG_ALLCONFIG=custom.config
and filling the custom config file with the required config setting.
So the downside is that you do need to have this custom support to seed particular things, but yes, it's doable, and it only requires a one-time setup with an environment variable, so it doesn't need changes to the actual testing infrastructure.
Linus
PATCH 1/1 ARM: Blacklist GCC 4.8.0 to GCC 4.8.2 - PR58854
Quoted 7 lines · Linus Torvalds
Quoted 4 lines · Russell King - ARM Linux
Does that work for things like allnoconfig and randconfig? I guess people /could/ seed those appropriately, but that seems to be something that Olof wants to avoid doing.
Quoted 5 lines
You can force particular config options for those, using something like
export KCONFIG_ALLCONFIG=custom.config
and filling the custom config file with the required config setting.
Yes, this is what I meant above by "seed"ing the configurations, and it is something I do all the time with my autobuilder here for things like allnoconfig where I want to ensure that certain options have specific values.
When I talked to Olof about it, he seemed to be against using the facility in his autobuilder. I'm not clear on why that was, but I suspect it's because Olof wants to test real allnoconfig builds.
An alternative is we could have a HAVE_GCC_PR58854_FIX preprocessor conditional, and pass in KCFLAGS=-DHAVE_GCC_PR58854_FIX to the kernel to bypass the check.
Either way, I think we need Olof's input on this to say what he's willing to do.
Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net.
PATCH 1/1 ARM: Blacklist GCC 4.8.0 to GCC 4.8.2 - PR58854
Hi,
Quoted 23 lines · Russell King - ARM Linux
Quoted 7 lines · Linus Torvalds
Quoted 4 lines · Russell King - ARM Linux
Does that work for things like allnoconfig and randconfig? I guess people /could/ seed those appropriately, but that seems to be something that Olof wants to avoid doing.
Quoted 5 lines
You can force particular config options for those, using something like
export KCONFIG_ALLCONFIG=custom.config
and filling the custom config file with the required config setting.
Yes, this is what I meant above by "seed"ing the configurations, and it is something I do all the time with my autobuilder here for things like allnoconfig where I want to ensure that certain options have specific values.
When I talked to Olof about it, he seemed to be against using the facility in his autobuilder. I'm not clear on why that was, but I suspect it's because Olof wants to test real allnoconfig builds.
Quoted 6 lines
An alternative is we could have a
HAVE_GCC_PR58854_FIXpreprocessor conditional, and pass in KCFLAGS=-DHAVE_GCC_PR58854_FIX to the kernel to bypass the check.Either way, I think we need Olof's input on this to say what he's willing to do.
I definitely don't mind adjusting the script, I've got some plans to add debug info on the builds that I boot as well, so I need to deal with a few fragments anyway. I haven't had much time to do that just yet, so this might raise it on the list of priorities a bit.
However, in this particular case I think I'll just upgrade gcc instead. Last time I tried 4.9, it spewed a lot of new warnings. As long as they're mostly gone I'll just switch over.
Russell: You might get a few duplicate reports as I try out the new toolchain.
-Olof
PATCH 1/1 ARM: Blacklist GCC 4.8.0 to GCC 4.8.2 - PR58854
In reply to Russell KingFrom: Russell King
Quoted 54 lines
These stock GCC versions miscompile the kernel by incorrectly optimising the function epilogue code - by first increasing the stack pointer, and then loading entries from below the stack. This means that an opportune interrupt or exception will corrupt the current function's saved state, which may result in the parent function seeing different register values.
As this bug has been known to result in corrupted filesystems, and these buggy compiler versions seem to be frequently used, we have little option but to blacklist these compiler versions.
Distributions may have fixed PR58854, but as their compilers are totally indistinguishable from the buggy versions, it is unfortunate that this also results in those also being blacklisted. Given the filesystem corruption potential of the original, this is the lesser evil. People who want to build with their fixed compiler versions will need to adjust the kernel source. (Distros need to think about the implications of fixing such a compiler bug, and consider how to ensure that their fixed compiler versions can be detected if they wish to avoid this.)
Signed-off-by Russell King <[email protected]>---
This is what I came up with - this places the build check right at the beginning of the kernel build, rather than at some point where
linux/compiler.hgets included. Note that this is where we have previous ARM specific GCC version blacklisting. I'm blacklisting GCC 4.8.0 to GCC 4.8.2 inclussive, which seems to be the right versions for stock GCC.I was in two minds whether to include 4.8.3 as Linaro released a buggy toolchain which identifies itself as 4.8.3, but I decided that's also a distro problem. IMHO Linaro should really think about taking that compiler down given the seriousness of this bug and it being indistinguishable from the fixed stock version.
arch/arm/kernel/asm-offsets.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)arch/arm/kernel/asm-offsets.c +1
-0@@ -10,6 +10,7 @@* it under the terms of the GNU General Public License version 2 as * published by the Free Software Foundation. */ +#include <linux/compiler.h> #include <linux/sched.h> #include <linux/mm.h> #include <linux/dma-mapping.h> @@ -39,10 +40,19 @@* GCC 3.2.x: miscompiles NEW_AUX_ENT in fs/binfmt_elf.c * (http://gcc.gnu.org/PR8896) and incorrect structure
arch/arm/kernel/asm-offsets.c line 50
Quoted 10 lines
arch/arm/kernel/asm-offsets.c +5
-1* initialisation in fs/jffs2/erase.c +* GCC 4.8.0-4.8.2: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58854 +* miscompiles find_get_entry(), and can result in EXT3 and EXT4 +* filesystem corruption (possibly other FS too). */ +#ifdef __GNUC__#if (__GNUC__ ==3 && __GNUC_MINOR__ <3 )#error Your compiler is too buggy; it is known to miscompile kernels. -#error Known good compilers: 3.3 +#error Known good compilers: 3.3, 4.x
Except that isn't true since 4.8.0 isn't a good compiler.
arch/arm/kernel/asm-offsets.c line 56
Quoted 6 lines
arch/arm/kernel/asm-offsets.c +5
-0+#endif +#if GCC_VERSION >=40800 ||GCC_VERSION <40803 +#error Your compiler is too buggy; it is known to miscompile kernels +#error and result in filesystem corruption and oopses. +#endif #endif
You are mixing the style of the version check.
Why not the single test:
#if GCC_VERSION < 30300 || (GCC_VERSION >= 40800 && GCC_VERSION < 40803) #error Your compiler is too buggy; it is known to miscompile code. #error Known good compilers: 3.3 onwards excluding 4.8.0 through 4.8.2 #endif
David
Quoted 4 lines
arch/arm/kernel/asm-offsets.c +0
-1int main (void ) --1.8.3.1
PATCH 1/1 ARM: Blacklist GCC 4.8.0 to GCC 4.8.2 - PR58854
Quoted 8 lines · David Laight
arch/arm/kernel/asm-offsets.c line 50
Quoted 5 lines
arch/arm/kernel/asm-offsets.c +2
-1+#ifdef __GNUC__#if (__GNUC__ ==3 && __GNUC_MINOR__ <3 )#error Your compiler is too buggy; it is known to miscompile kernels. -#error Known good compilers: 3.3 +#error Known good compilers: 3.3, 4.xExcept that isn't true since 4.8.0 isn't a good compiler.
Quoted 13 lines
arch/arm/kernel/asm-offsets.c line 56
Quoted 6 lines
arch/arm/kernel/asm-offsets.c +5
-0+#endif +#if GCC_VERSION >=40800 ||GCC_VERSION <40803 +#error Your compiler is too buggy; it is known to miscompile kernels +#error and result in filesystem corruption and oopses. +#endif #endif You are mixing the style of the version check.
Why not the single test:
#ifGCC_VERSION< 30300 || (GCC_VERSION>= 40800 &&GCC_VERSION< 40803) #error Your compiler is too buggy; it is known to miscompile code. #error Known good compilers: 3.3 onwards excluding 4.8.0 through 4.8.2 #endif
As you have just nicely demonstrated, changing what's already there can cause unintentional changes to creep in. The original test tested for GCC 3 compilers older than GCC 3.3, excluding GCC 2.96 and older. Your version blacklists everything before GCC 3.3.
Such a change should be a separate patch. Moreover, if we really do want to blacklist all compilers older than GCC 3.3, then that too needs an explanation in the comments above this block, and in commit message.
Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net.