linux-kernel - Re: [PATCH][GIT PULL][v2.6.32] tracing/x86: Add check to detect GCC messing with mcount prologue

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: <1258736456.22249.1032.camel@gandalf.stny.rr.com>
Date:	Fri, 20 Nov 2009 12:00:56 -0500
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	Ingo Molnar <mingo@...e.hu>, "H. Peter Anvin" <hpa@...or.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Heiko Carstens <heiko.carstens@...ibm.com>,
	feng.tang@...el.com, Peter Zijlstra <peterz@...radead.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	David Daney <ddaney@...iumnetworks.com>,
	Andrew Haley <aph@...hat.com>,
	Richard Guenther <richard.guenther@...il.com>,
	jakub@...hat.com, gcc <gcc@....gnu.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH][GIT PULL][v2.6.32] tracing/x86: Add check to detect
 GCC messing with mcount prologue

Ingo, Thomas and Linus,

I know Thomas did a patch to force the -mtune=generic, but just in case
gcc decides to do something crazy again, this patch will catch it.

Should we try to get this in now?

-- Steve

On Fri, 2009-11-20 at 00:23 -0500, Steven Rostedt wrote:
> commit c7715fb611c69ac4b7f722a891de08b206fb7686
> Author: Steven Rostedt <srostedt@...hat.com>
> Date:   Thu Nov 19 23:41:02 2009 -0500
> 
>     tracing/x86: Add check to detect GCC messing with mcount prologue
>     
>     Latest versions of GCC create a funny prologue for some functions.
>     Instead of the typical:
>     
>     		push   %ebp
>     		mov    %esp,%ebp
>     		and    $0xffffffe0,%esp
>     		[...]
>     		call   mcount
>     
>     GCC may try to align the stack before setting up the frame pointer
>     register:
>     
>     		push   %edi
>     		lea    0x8(%esp),%edi
>     		and    $0xffffffe0,%esp
>     		pushl  -0x4(%edi)
>     		push   %ebp
>     		mov    %esp,%ebp
>     		[...]
>     		call   mcount
>     
>     This crazy code places a copy of the return address into the
>     frame pointer. The function graph tracer uses this pointer to
>     save and replace the return address of the calling function to jump
>     to the function graph tracer's return handler, which will put back
>     the return address. But instead instead of the typical return:
>     
>     		mov    %ebp,%esp
>     		pop    %ebp
>     		ret
>     
>     The return of the function performs:
>     
>     		lea    -0x8(%edi),%esp
>     		pop    %edi
>     		ret
>     
>     The function graph tracer return handler will not be called at the exit
>     of the function, but the parent function will call it. Because we missed
>     the return of the child function, the handler will replace the parent's
>     return address with that of the child. Obviously this will cause a crash
>     (Note, there is code to detect this case and safely panic the kernel).
>     
>     The kicker is that this happens to just a handful of functions.
>     And only with certain gcc options.
>     
>     Compiling with: 	-march=pentium-mmx
>     will cause the problem to appear. But if you were to change
>     pentium-mmx to i686 or add -mtune=generic, then the problem goes away.
>     
>     I first saw this problem when compiling with optimize for size.
>     But it seems that various other options may cause this issue to arise.
>     
>     Instead of completely disabling the function graph tracer for i386 builds
>     this patch adds a check to recordmcount.pl to make sure that all
>     functions that contain a call to mcount start with "push %ebp".
>     If not, it will fail the compile and print out the nasty warning:
>     
>       CC      kernel/time/timer_stats.o
>     
>     ********************************************************
>       Your version of GCC breaks the function graph tracer
>       Please disable CONFIG_FUNCTION_GRAPH_TRACER
>       Failed function was "timer_stats_update_stats"
>     ********************************************************
>     
>     The script recordmcount.pl is given a new parameter "do_check". If
>     this is negative, the script will only perform this check without
>     creating the mcount caller section. This will be executed for x86_32
>     when CONFIG_FUNCTION_GRAPH_TRACER is enabled and CONFIG_DYNAMIC_FTRACE
>     is not.
>     
>     If the arch is x86_32 and $do_check is greater than 1, it will perform
>     the check while processing the mcount callers. If $do_check is 0, then
>     no check will be performed. This is for non x86_32 archs and when
>     compiling without CONFIG_FUNCTION_GRAPH_TRACER enabled, even on x86_32.
>     
>     Reported-by: Thomas Gleixner <tglx@...utronix.de>
>     LKML-Reference: <alpine.LFD.2.00.0911191423190.24119@...alhost.localdomain>
>     Signed-off-by: Steven Rostedt <rostedt@...dmis.org>


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/