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

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]

Date:	Tue, 19 Jan 2016 23:42:56 -0600
From:	Josh Poimboeuf <jpoimboe@...hat.com>
To:	Ingo Molnar <mingo@...nel.org>
Cc:	Borislav Petkov <bp@...en8.de>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>, x86@...nel.org,
	linux-kernel@...r.kernel.org, live-patching@...r.kernel.org,
	Michal Marek <mmarek@...e.cz>,
	Peter Zijlstra <peterz@...radead.org>,
	Andy Lutomirski <luto@...nel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andi Kleen <andi@...stfloor.org>,
	Pedro Alves <palves@...hat.com>,
	Namhyung Kim <namhyung@...il.com>,
	Bernd Petrovitsch <bernd@...rovitsch.priv.at>,
	Chris J Arges <chris.j.arges@...onical.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Jiri Slaby <jslaby@...e.cz>,
	Arnaldo Carvalho de Melo <acme@...nel.org>
Subject: Re: [PATCH v15 13/25] x86/reboot: Add ljmp instructions to stacktool
 whitelist

On Fri, Jan 15, 2016 at 12:00:00PM +0100, Ingo Molnar wrote:
> 
> * Borislav Petkov <bp@...en8.de> wrote:
> 
> > On Fri, Jan 15, 2016 at 12:06:52AM -0600, Josh Poimboeuf wrote:
> > > - xen_cpuid() uses some custom xen instructions which start with
> > >   XEN_EMULATE_PREFIX.  It corresponds to the following x86 instructions:
> > > 
> > >     ffffffff8107e572:       0f 0b                   ud2
> > >     ffffffff8107e574:       78 65                   js ffffffff8107e5db <xen_get_debugreg+0xa>
> > >     ffffffff8107e576:       6e                      outsb %ds:(%rsi),(%dx)
> > > 
> > >   Apparently(?) xen treats the ud2 special when it's followed by "78 65
> > >   6e".  This is confusing for stacktool because ud2 is normally a dead
> > >   end, and it thinks the instructions after it will never run.
> > >   
> > >   (In theory stacktool could be taught to understand this hack, but
> > >   that's a bad idea IMO)
> > 
> > Why, because it is not generic enough?
> > 
> > Well, you could add a cmdline option "--kernel" which is supplied when
> > checking the kernel and such kernel "idiosyncrasies" are handled only
> > then and there. And since the tool is part of the kernel, changes to
> > XEN_EMULATE_PREFIX, will have to be updated in stacktool too...
> 
> So I think because we are talking about less than a dozen annotations, these are 
> technicalities - and it might in fact be better to have a single line of obvious 
> annotation in a function that does something weird (and arguably all of these 
> functions do something weird), than having dozens of lines of code on the tooling 
> side to avoid that single line on the kernel side.
> 
> That has a documentation value as well.
> 
> As long as the annotation itself is not stacktool specific, it should serve as 
> documentation as well - such as:
> 
>   __non_standard_stack_frame
> 
> or:
> 
>   __non_C_instructions
> 
> ?
> 
> All of the cases Josh listed involve some sort of special case where we do 
> something non-standard. (Where 'standard' == 'regular kernel C function'.)

I've now gotten the number of warnings down to 0 (except for a few
staging drivers), even with allyesconfig (with !CONFIG_GCOV).

I've also managed to make stacktool a little smarter such that the
in-code STACKTOOL_IGNORE_INSN markers are no longer needed, woot!

There's still a need for 4 STACKTOOL_IGNORE_FUNC(name) markers in the
entire tree, due to the weird cases I mentioned.  But they're placed
after the functions, so they're much less disruptive.

I'll be posting a v16 soon.

-- 
Josh