Re: [PATCH v15 13/25] x86/reboot: Add ljmp instructions to stacktool whitelist
From: Josh Poimboeuf
Date: Wed Jan 20 2016 - 00:43:23 EST
On Fri, Jan 15, 2016 at 12:00:00PM +0100, Ingo Molnar wrote:
> * Borislav Petkov <bp@xxxxxxxxx> 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:
> 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.