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

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

[Peter Hurley]

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 <aaro.koskinen@iki.fi>
Reported-by: Felipe Balbi <balbi@ti.com>
Cc: Rabin Vincent <rabin@rab.in>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 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

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

[Russell King - ARM Linux]

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.

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

[Peter Hurley]

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

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

[Mikael Pettersson]

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.

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

[Peter Hurley]

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

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

[Mikael Pettersson]

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