2014-10-10 16:26:46

by Peter Hurley

[permalink] [raw]
Subject: [PATCH] arm: Blacklist gcc 4.8.[012] and 4.9.0 with CONFIG_FRAME_POINTER

gcc versions 4.8.[012] and 4.9.0 generates code that prematurely
adjusts the stack pointer such that still-to-be-referenced locals
are below the stack pointer, which allows them to be overwritten
by interrupts.

On 10/09/2014 04:41 PM, Rabin Vincent wrote:
> 4.8.1 and 4.8.2 are known to miscompile the ARM kernel and these
> find_get_entry() crashes with 0xffffffff involved smell a lot like the
> earlier reports from kernels build with those compilers:
>
> https://lkml.org/lkml/2014/6/25/456
> https://lkml.org/lkml/2014/6/30/375
> https://lkml.org/lkml/2014/6/30/660
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58854
> https://lkml.org/lkml/2014/5/9/330
>
> Also, I didn't see any public email making a definitive link between GCC
> PR 58854 that Nathan pointed out in https://lkml.org/lkml/2014/6/30/660
> and the earlier find_get_entry() crashes, but I just built GCC 4.8.1 and
> an ARM kernel with that, and the GCC bug is clearly seen in
> radix_tree_lookup_slot() which returns the pointer which
> find_get_entry() is dereferencing:
>
> <radix_tree_lookup_slot>:
> e1a0c00d mov ip, sp
> e92dd800 push {fp, ip, lr, pc}
> e24cb004 sub fp, ip, #4
> e24dd008 sub sp, sp, #8
> e3a02000 mov r2, #0
> e24b3010 sub r3, fp, #16
> ebffffc5 bl c0176ab8 <__radix_tree_lookup>
> e24bd00c sub sp, fp, #12 <--- sp moved up
> e3500000 cmp r0, #0
> 151b0010 ldrne r0, [fp, #-16] <--- load from under sp
> e89da800 ldm sp, {fp, sp, pc}
>

See gcc PR 58854, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58854

Reported-by: Aaro Koskinen <[email protected]>
Reported-by: Felipe Balbi <[email protected]>
Cc: Rabin Vincent <[email protected]>
Signed-off-by: Peter Hurley <[email protected]>
---
include/linux/compiler-gcc4.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)

diff --git a/include/linux/compiler-gcc4.h b/include/linux/compiler-gcc4.h
index 2507fd2..4069fb2 100644
--- a/include/linux/compiler-gcc4.h
+++ b/include/linux/compiler-gcc4.h
@@ -86,3 +86,17 @@
#define __HAVE_BUILTIN_BSWAP16__
#endif
#endif /* CONFIG_ARCH_USE_BUILTIN_BSWAP */
+
+/*
+ * For ARCH=arm, gcc versions 4.8.[012] and 4.9.0 generate code that closes
+ * the stack frame prematurely which allows still-in-use locals to be
+ * overwritten by interrupts
+ *
+ * see PR 58854, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58854
+ */
+#ifdef __KERNEL__
+# if ((GCC_VERSION >= 40800 && GCC_VERSION <= 40802) || GCC_VERSION == 40900) \
+ && defined(__arm__) && defined(CONFIG_FRAME_POINTER)
+# error Your version of gcc miscompiles stack frames
+# endif
+#endif
--
2.1.1


2014-10-10 16:36:25

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH] arm: Blacklist gcc 4.8.[012] and 4.9.0 with CONFIG_FRAME_POINTER

On Fri, Oct 10, 2014 at 12:26:14PM -0400, Peter Hurley wrote:
> gcc versions 4.8.[012] and 4.9.0 generates code that prematurely
> adjusts the stack pointer such that still-to-be-referenced locals
> are below the stack pointer, which allows them to be overwritten
> by interrupts.

I would much rather do this in asm-offsets.c, along side the other ARM
specific buggy compiler test(s). I'm presently putting together such
a patch.

The information in the thread on linux-omap says only GCC 4.8.1 and
GCC 4.8.2. Where do you get the other versions from?

--
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

2014-10-10 16:55:41

by Peter Hurley

[permalink] [raw]
Subject: Re: [PATCH] arm: Blacklist gcc 4.8.[012] and 4.9.0 with CONFIG_FRAME_POINTER

On 10/10/2014 12:36 PM, Russell King - ARM Linux wrote:
> On Fri, Oct 10, 2014 at 12:26:14PM -0400, Peter Hurley wrote:
>> gcc versions 4.8.[012] and 4.9.0 generates code that prematurely
>> adjusts the stack pointer such that still-to-be-referenced locals
>> are below the stack pointer, which allows them to be overwritten
>> by interrupts.
>
> I would much rather do this in asm-offsets.c, along side the other ARM
> specific buggy compiler test(s). I'm presently putting together such
> a patch.
>
> The information in the thread on linux-omap says only GCC 4.8.1 and
> GCC 4.8.2. Where do you get the other versions from?

The gcc PR linked in the commit message; see the "Known to fail" field.

Regards,
Peter Hurley

2014-10-11 16:33:49

by Mikael Pettersson

[permalink] [raw]
Subject: Re: [PATCH] arm: Blacklist gcc 4.8.[012] and 4.9.0 with CONFIG_FRAME_POINTER

Peter Hurley writes:
> On 10/10/2014 12:36 PM, Russell King - ARM Linux wrote:
> > On Fri, Oct 10, 2014 at 12:26:14PM -0400, Peter Hurley wrote:
> >> gcc versions 4.8.[012] and 4.9.0 generates code that prematurely
> >> adjusts the stack pointer such that still-to-be-referenced locals
> >> are below the stack pointer, which allows them to be overwritten
> >> by interrupts.
> >
> > I would much rather do this in asm-offsets.c, along side the other ARM
> > specific buggy compiler test(s). I'm presently putting together such
> > a patch.
> >
> > The information in the thread on linux-omap says only GCC 4.8.1 and
> > GCC 4.8.2. Where do you get the other versions from?
>
> The gcc PR linked in the commit message; see the "Known to fail" field.

The 4.8.0 release is broken, but the 4.9.0 one is not. It's unfortunate,
but "4.9.0" may refer to "the 4.9.0 release" or to "some point after trunk
forked 4.8 branch up to and including the 4.9.0 release point". In this
case, it's the latter -- this can be inferred from the fact that the
fix went into trunk in October 2013 while 4.9.0 was branched and released
during the first half of 2014.

2014-10-11 18:04:53

by Peter Hurley

[permalink] [raw]
Subject: Re: [PATCH] arm: Blacklist gcc 4.8.[012] and 4.9.0 with CONFIG_FRAME_POINTER

On 10/11/2014 12:33 PM, Mikael Pettersson wrote:
> Peter Hurley writes:
> > On 10/10/2014 12:36 PM, Russell King - ARM Linux wrote:
> > > On Fri, Oct 10, 2014 at 12:26:14PM -0400, Peter Hurley wrote:
> > >> gcc versions 4.8.[012] and 4.9.0 generates code that prematurely
> > >> adjusts the stack pointer such that still-to-be-referenced locals
> > >> are below the stack pointer, which allows them to be overwritten
> > >> by interrupts.
> > >
> > > I would much rather do this in asm-offsets.c, along side the other ARM
> > > specific buggy compiler test(s). I'm presently putting together such
> > > a patch.
> > >
> > > The information in the thread on linux-omap says only GCC 4.8.1 and
> > > GCC 4.8.2. Where do you get the other versions from?
> >
> > The gcc PR linked in the commit message; see the "Known to fail" field.
>
> The 4.8.0 release is broken, but the 4.9.0 one is not. It's unfortunate,
> but "4.9.0" may refer to "the 4.9.0 release" or to "some point after trunk
> forked 4.8 branch up to and including the 4.9.0 release point". In this
> case, it's the latter -- this can be inferred from the fact that the
> fix went into trunk in October 2013 while 4.9.0 was branched and released
> during the first half of 2014.

Is there a reasonably quick way to determine if a particular commit is
in a particular release of gcc?

Starting from the mainline viewcvs revision page for this fix here,
https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=204203
(which is the link from the PR for the fix), navigation to anywhere
else in the gcc tree is impossible. I can't even look at the Changelog.

Same with the backport.

Inferring by date seems error-prone.

Regards,
Peter Hurley

2014-10-12 09:50:50

by Mikael Pettersson

[permalink] [raw]
Subject: Re: [PATCH] arm: Blacklist gcc 4.8.[012] and 4.9.0 with CONFIG_FRAME_POINTER

Peter Hurley writes:
> On 10/11/2014 12:33 PM, Mikael Pettersson wrote:
> > Peter Hurley writes:
> > > On 10/10/2014 12:36 PM, Russell King - ARM Linux wrote:
> > > > On Fri, Oct 10, 2014 at 12:26:14PM -0400, Peter Hurley wrote:
> > > >> gcc versions 4.8.[012] and 4.9.0 generates code that prematurely
> > > >> adjusts the stack pointer such that still-to-be-referenced locals
> > > >> are below the stack pointer, which allows them to be overwritten
> > > >> by interrupts.
> > > >
> > > > I would much rather do this in asm-offsets.c, along side the other ARM
> > > > specific buggy compiler test(s). I'm presently putting together such
> > > > a patch.
> > > >
> > > > The information in the thread on linux-omap says only GCC 4.8.1 and
> > > > GCC 4.8.2. Where do you get the other versions from?
> > >
> > > The gcc PR linked in the commit message; see the "Known to fail" field.
> >
> > The 4.8.0 release is broken, but the 4.9.0 one is not. It's unfortunate,
> > but "4.9.0" may refer to "the 4.9.0 release" or to "some point after trunk
> > forked 4.8 branch up to and including the 4.9.0 release point". In this
> > case, it's the latter -- this can be inferred from the fact that the
> > fix went into trunk in October 2013 while 4.9.0 was branched and released
> > during the first half of 2014.
>
> Is there a reasonably quick way to determine if a particular commit is
> in a particular release of gcc?

If you want the process to be fully automatic and the answer to be
absolutely precise, then "no".

If you're willing to manually map GCC PR fixes to release versions,
and to have some false negatives (some GCCs having a certain fix
will be flagged as not having it), then "yes".

For this ARM bug, PR58854, we know that 4.8.[0-2] have the bug, but
4.7 and older, 4.8.3 and newer, and 4.9 and newer are Ok.

A problem is that a GCC that identifies itself as 4.8.3 may be
(a) a 4.8.3 pre-release (i.e., close to 4.8.2),
(b) a 4.8.3 release, or
(c) a 4.8.4 pre-release that's been patched to say 4.8.3 (Red Hat does this).

Case (a) may or may not have the fix (we can't easily(*) tell), but
cases (b) and (c) are Ok. If you're willing to classify all three
as not having the fix (false negatives), then you want to test

#if (__GNUC__ == 4 && __GNUC_MINOR__ == 8 && __GNUC_PATCHLEVEL__ < 4)

for possibly broken versions.

A complication is that a bug has both starting and ending commits.
It's not uncommon for distros and others to backport changes, so a
compiler that claims to be e.g. 4.7.4 may include a backport of the
4.8 change that caused the bug you're trying to avoid. There is no
easy way to detect this, unless you have a runtime test case for the
bug. I'd ignore this case as "unfixable".

So I'd write the tests for vanilla upstream GCC only, and tell distro
users to complain to their distros if their kernels get miscompiled.

(*) __VERSION__ is defined like "4.8.3 20140515 (prerelease)" in
pre-releases but like "4.8.3" in ordinary releases, but this is not
something you can test for in the C preprocessor. A configure-time
check could extract the date and compare that with the date the fix
went into that particular branch, but case (c) above make detecting
pre-releases a bit more complicated.

> Starting from the mainline viewcvs revision page for this fix here,
> https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=204203
> (which is the link from the PR for the fix), navigation to anywhere
> else in the gcc tree is impossible. I can't even look at the Changelog.

https://gcc.gnu.org/viewcvs/gcc/
then descend trunk or branches as needed.

/Mikael