Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1307683

Re: [PATCH v15 13/25] x86/reboot: Add ljmp instructions to stacktool whitelist

Show all headers | View raw

On Tue, Jan 12, 2016 at 05:47:11PM +0100, Borislav Petkov wrote:
> On Fri, Dec 18, 2015 at 06:39:27AM -0600, Josh Poimboeuf wrote:
> > stacktool reports a false positive warning for the ljmp instruction in
> > machine_real_restart().  Normally, ljmp isn't allowed in a function, but
> > this is a special case where it's jumping into real mode.
> > 
> > Add the jumps to a whitelist which tells stacktool to ignore them.
> > 
> > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> > ---
> >  arch/x86/kernel/reboot.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
> > index 02693dd..1ea1c5e 100644
> > --- a/arch/x86/kernel/reboot.c
> > +++ b/arch/x86/kernel/reboot.c
> > @@ -9,6 +9,7 @@
> >  #include <linux/sched.h>
> >  #include <linux/tboot.h>
> >  #include <linux/delay.h>
> > +#include <linux/stacktool.h>
> >  #include <acpi/reboot.h>
> >  #include <asm/io.h>
> >  #include <asm/apic.h>
> > @@ -97,11 +98,13 @@ void __noreturn machine_real_restart(unsigned int type)
> >  
> >  	/* Jump to the identity-mapped low memory code */
> >  #ifdef CONFIG_X86_32
> > -	asm volatile("jmpl *%0" : :
> > +	asm volatile(STACKTOOL_IGNORE_INSN
> > +		     "jmpl *%0;" : :
> >  		     "rm" (real_mode_header->machine_real_restart_asm),
> >  		     "a" (type));
> >  #else
> > -	asm volatile("ljmpl *%0" : :
> > +	asm volatile(STACKTOOL_IGNORE_INSN
> > +		     "ljmpl *%0" : :
> >  		     "m" (real_mode_header->machine_real_restart_asm),
> >  		     "D" (type));
> >  #endif
> 
> Well, I can't say that I'm crazy about all those new tools adding
> markers to unrelated kernel code.
> 
> Can't you teach stacktool to ignore the whole machine_real_restart()
> function simply?

Well, these STACKTOOL_IGNORE whitelist markers are only needed in a
handful of places, and only for code that does very weird things.  Yes,
they're a bit ugly, but IMO they also communicate valuable information:
"be careful, this code does something very weird."

As for whether to put the whitelist info in the code vs hard-coding it
in stacktool, I think it's clearer and less "magical" to put them
directly in the code.

It's also more resilient to future code changes, e.g. if the offending
instruction gets moved or if the function gets renamed.

And it gives you the ability to more granularly whitelist instructions
rather than entire functions, which could cause other offending stack
violations in the function to get overlooked.

Another thing is that stacktool could be a nice general purpose tool for
finding stack issues in other code bases, and so I think requiring it to
have hard-coded knowledge about the code base would greatly limit its
general usefulness.  (Though maybe this problem could be remediated with
a user-provided whitelist file which lists functions to be ignored.)

-- 
Josh

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Re: [PATCH v15 13/25] x86/reboot: Add ljmp instructions to stacktool  whitelist Borislav Petkov <bp@alien8.de> - 2016-01-12 17:50 +0100
  Re: [PATCH v15 13/25] x86/reboot: Add ljmp instructions to stacktool  whitelist Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-12 18:50 +0100
    Re: [PATCH v15 13/25] x86/reboot: Add ljmp instructions to stacktool  whitelist Borislav Petkov <bp@alien8.de> - 2016-01-12 19:00 +0100
      Re: [PATCH v15 13/25] x86/reboot: Add ljmp instructions to stacktool  whitelist Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-12 20:00 +0100
        Re: [PATCH v15 13/25] x86/reboot: Add ljmp instructions to stacktool  whitelist Borislav Petkov <bp@alien8.de> - 2016-01-12 20:40 +0100
    Re: [PATCH v15 13/25] x86/reboot: Add ljmp instructions to stacktool  whitelist Ingo Molnar <mingo@kernel.org> - 2016-01-13 12:00 +0100
      Re: [PATCH v15 13/25] x86/reboot: Add ljmp instructions to stacktool  whitelist Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-15 07:10 +0100

csiph-web