Hey Christoffer,

On 31/01/2019 08:08, Christoffer Dall wrote:

On Thu, Jan 24, 2019 at 04:32:54PM +0000, James Morse wrote:

quoted

On systems with VHE the kernel and KVM's world-switch code run at the
same exception level. Code that is only used on a VHE system does not
need to be annotated as __hyp_text as it can reside anywhere in the
kernel text.

__hyp_text was also used to prevent kprobes from patching breakpoint
instructions into this region, as this code runs at a different
exception level. While this is no longer true with VHE, KVM still
switches VBAR_EL1, meaning a kprobe's breakpoint executed in the
world-switch code will cause a hyp-panic.

Forgive potentially very stupid questions here, but:

 (1) Would it make sense to move the save/restore VBAR_EL1 to the last
     possible moment, and would that actually allow kprobes to work for
     the world-switch code, or does that just result in other weird
     problems?

This would work for taking the debug exception. But next kprobes wants to
single-step the probed instruction in an out-of-line slot. I don't think we can
do this if we've already configured the debug hardware for the guest.
(If could at least turn single-step off when we return to guest-EL0, which
guest-EL1 was single-stepping)

 (2) Are we sure that this catches every call path of every non-inlined
     function called after switchign VBAR_EL1?  Can kprobes only be
     called on exported symbols, or can you (if you know the address
     somehow) put a kprobe on a static function as well.  If there are
     any concerns in this area, we might want to consider (1) more
     closely.

Hmmm, good question. The blacklisting applies to whole symbols as seen by
kallsyms, the compiler has no idea what is going on.

If it chose not to inline something, it would be kprobe'able yes.

__kprobes uses a section function-attribute instead. The gcc manual[0] doesn't
say what happens when inline and the section attributes are used together. (or
at least I couldn't find it)

A quick experiment with gcc 8.2.0 shows adding __kprobes on the inlines gets
discarded when they are inlined. I'm not sure how to trick the compiler into
not-inlining it to see what happens, but adding 'noinline' to the header file
causes it to duplicate the function everywhere, but puts it in the __kprobes
section.

(For KVM we could use the 'flatten' attribute, but that does say 'if possible'.
Alternatively we can decorate all the inline helpers we know we use with
__kprobes as a safety net.)

I think this is a wider problem with kprobes.


Thanks,

James

[0]
https://gcc.gnu.org/onlinedocs/gcc-8.2.0/gcc/Common-Function-Attributes.html#Common-Function-Attributes

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel