On Wed, Jan 13, 2016 at 11:55:03AM +0100, Ingo Molnar wrote:
>
> * Josh Poimboeuf <jpoimboe@xxxxxxxxxx> wrote:
>
> > > 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."
>
> How common are these markers? Like with lockdep, it all depends on magnitude:
>
> - If it's less than 10 I'd say it's OK.
>
> - If it's dozens then it's ho-hum.
>
> - If certain types of annotations can go over 100, then they are unacceptable.
>
> all such in-code overhead has to be balanced against the utility of the tooling.
Sounds reasonable. After looking at all the warnings on an allyesconfig
kernel, I'm pretty sure it'll be less than 10. Most of the cases are
mentioned below.
> > 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.
>
> That's true - but I think Boris tried to ask something slightly different: can
> stacktool be taught to detect weird signatures automatically, and to ignore them
> automatically?
>
> Stuff like 16-bit code sure wounds 'weird' and the tool could detect that?
Some of the 'weird' cases:
- Some functions use 'ljmp' or 'lret'. After looking at this some more,
I think it would be safe for stacktool to translate the use of these
instructions to mean "I know what I'm doing" and just ignore the
function. 100% of the functions are safe to ignore anyway. So we can
get rid of those markers and just make stacktool smarter.
- 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)
- The error path in arch/x86/net/bpf_jit.S uses 'leaveq' to do a double
return so that it returns from its caller's context. stacktool
doesn't know how to distinguish this from a frame pointer programming
bug. I think the only way to avoid a whitelist marker here would be
to rewrite the bpf code to conform with more traditional rbp usage
(but I don't know if that would really be a good idea because it would
probably result in slower/more code).
- __bpf_prog_run() uses a jump table:
goto *jumptable[insn->code];
stacktool doesn't have an x86 emulator, so it doesn't know how to
deterministically follow all possible branches for a dynamic jump.
- schedule() mucks with the frame pointer which is normally not allowed.
--
Josh
--
To unsubscribe from this list: send the line "unsubscribe live-patching" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
