| 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
| ||
|
Message-ID: <20101029200245.GA10702@Krystal> Date: Fri, 29 Oct 2010 16:02:45 -0400 From: Mathieu Desnoyers <mathieu.desnoyers@...icios.com> To: Steven Rostedt <rostedt@...dmis.org> Cc: linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>, Andrew Morton <akpm@...ux-foundation.org>, Thomas Gleixner <tglx@...utronix.de>, Peter Zijlstra <peterz@...radead.org>, Frederic Weisbecker <fweisbec@...il.com>, Peter Zijlstra <a.p.zijlstra@...llo.nl>, Jason Baron <jbaron@...hat.com>, "H. Peter Anvin" <hpa@...or.com>, David Daney <ddaney@...iumnetworks.com>, Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>, David Miller <davem@...emloft.net>, Richard Henderson <rth@...hat.com> Subject: Re: [PATCH 7/7] jump label: Add work around to i386 gcc asm goto bug * Steven Rostedt (rostedt@...dmis.org) wrote: > From: Steven Rostedt <srostedt@...hat.com> > > On i386 (not x86_64) early implementations of gcc would have a bug > with asm goto causing it to produce code like the following: > > (This was noticed by Peter Zijlstra) > > 56 pushl 0 > 67 nopl jmp 0x6f > popl > jmp 0x8c > > 6f mov > test > je 0x8c > > 8c mov > call *(%esp) > > The jump added in the asm goto skipped over the popl that matched > the pushl 0, which lead up to a quick crash of the system when > the jump was enabled. The nopl is defined in the asm goto () statement > and when tracepoints are enabled, the nop changes to a jump to the label > that was specified by the asm goto. asm goto is suppose to tell gcc that > the code in the asm might jump to an external label. Here gcc obviously > fails to make that work. > > The bug report for gcc is here: > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46226 > > The bug only appears on x86 when not compiled with > -maccumulate-outgoing-args. This option is always set on x86_64 and it > is also the work around for a function graph tracer i386 bug. > (See commit: 746357d6a526d6da9d89a2ec645b28406e959c2e) > This explains why the bug only showed up on i386 when function graph > tracer was not enabled. > > This patch now adds a CONFIG_JUMP_LABEL option that is default > off instead of using jump labels by default. When jump labels are > enabled, the -maccumulate-outgoing-args will be used (causing a > slightly larger kernel image on i386). This option will exist > until we have a way to detect if the gcc compiler in use is safe > to use on all configurations without the work around. > > Note, there exists such a test, but for now we will keep the enabling > of jump label as a manual option. > > Archs that know the compiler is safe with asm goto, may choose to > select JUMP_LABEL and enable it by default. Looks good. You can add my Acked-by: Mathieu Desnoyers <mathieu.desnoyers@...icios.com> if you feel like it. Thanks, Mathieu > > Reported-by: Ingo Molnar <mingo@...e.hu> > Cause-discovered-by: Peter Zijlstra <a.p.zijlstra@...llo.nl> > Cc: Peter Zijlstra <a.p.zijlstra@...llo.nl> > Cc: Thomas Gleixner <tglx@...utronix.de> > Cc: Jason Baron <jbaron@...hat.com> > Cc: H. Peter Anvin <hpa@...or.com> > Cc: David Daney <ddaney@...iumnetworks.com> > Cc: Mathieu Desnoyers <mathieu.desnoyers@...icios.com> > Cc: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com> > Cc: David Miller <davem@...emloft.net> > Cc: Richard Henderson <rth@...hat.com> > LKML-Reference: <1288028746.3673.11.camel@...top> > Signed-off-by: Steven Rostedt <rostedt@...dmis.org> > --- > arch/Kconfig | 14 ++++++++++++++ > arch/x86/Makefile_32.cpu | 13 ++++++++++++- > include/linux/jump_label.h | 2 +- > 3 files changed, 27 insertions(+), 2 deletions(-) > > diff --git a/arch/Kconfig b/arch/Kconfig > index 53d7f61..8bf0fa6 100644 > --- a/arch/Kconfig > +++ b/arch/Kconfig > @@ -42,6 +42,20 @@ config KPROBES > for kernel debugging, non-intrusive instrumentation and testing. > If in doubt, say "N". > > +config JUMP_LABEL > + bool "Optimize trace point call sites" > + depends on HAVE_ARCH_JUMP_LABEL > + help > + If it is detected that the compiler has support for "asm goto", > + the kernel will compile trace point locations with just a > + nop instruction. When trace points are enabled, the nop will > + be converted to a jump to the trace function. This technique > + lowers overhead and stress on the branch prediction of the > + processor. > + > + On i386, options added to the compiler flags may increase > + the size of the kernel slightly. > + > config OPTPROBES > def_bool y > depends on KPROBES && HAVE_OPTPROBES > diff --git a/arch/x86/Makefile_32.cpu b/arch/x86/Makefile_32.cpu > index 1255d95..f2ee1ab 100644 > --- a/arch/x86/Makefile_32.cpu > +++ b/arch/x86/Makefile_32.cpu > @@ -51,7 +51,18 @@ cflags-$(CONFIG_X86_GENERIC) += $(call tune,generic,$(call tune,i686)) > # prologue (push %ebp, mov %esp, %ebp) which breaks the function graph > # tracer assumptions. For i686, generic, core2 this is set by the > # compiler anyway > -cflags-$(CONFIG_FUNCTION_GRAPH_TRACER) += $(call cc-option,-maccumulate-outgoing-args) > +ifeq ($(CONFIG_FUNCTION_GRAPH_TRACER), y) > +ADD_ACCUMULATE_OUTGOING_ARGS := y > +endif > + > +# Work around to a bug with asm goto with first implementations of it > +# in gcc causing gcc to mess up the push and pop of the stack in some > +# uses of asm goto. > +ifeq ($(CONFIG_JUMP_LABEL), y) > +ADD_ACCUMULATE_OUTGOING_ARGS := y > +endif > + > +cflags-$(ADD_ACCUMULATE_OUTGOING_ARGS) += $(call cc-option,-maccumulate-outgoing-args) > > # Bug fix for binutils: this option is required in order to keep > # binutils from generating NOPL instructions against our will. > diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h > index 1947a12..7880f18 100644 > --- a/include/linux/jump_label.h > +++ b/include/linux/jump_label.h > @@ -1,7 +1,7 @@ > #ifndef _LINUX_JUMP_LABEL_H > #define _LINUX_JUMP_LABEL_H > > -#if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_HAVE_ARCH_JUMP_LABEL) > +#if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL) > # include <asm/jump_label.h> > # define HAVE_JUMP_LABEL > #endif > -- > 1.7.1 > > -- Mathieu Desnoyers Operating System Efficiency R&D Consultant EfficiOS Inc. http://www.efficios.com -- 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/
Powered by blists - more mailing lists