Message ID | 20190508015559.767152678@goodmis.org (mailing list archive) |
---|---|
Headers | show |
Series | x86_64/ftrace: Emulate calls from int3 when patching functions | expand |
On Tue, 07 May 2019 21:55:59 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > > [ > This is the non-RFC version. > > It went through and passed all my tests. If there's no objections > I'm going to include this in my pull request. I still have patches > in my INBOX that may still be included, so I need to run those through > my tests as well, so a pull request wont be immediate. > ] > > Nicolai Stange discovered that Live Kernel Patching can have unforseen > consequences if tracing is enabled when there are functions that are > patched. The reason being, is that Live Kernel patching is built on top > of ftrace, which will have the patched functions call the live kernel > trampoline directly, and that trampoline will modify the regs->ip address > to return to the patched function. > > But in the transition between changing the call to the customized > trampoline, the tracing code is needed to have its handler called > an well, so the function fentry location must be changed from calling > the live kernel patching trampoline, to the ftrace_reg_caller trampoline > which will iterate through all the registered ftrace handlers for > that function. > > During this transition, a break point is added to do the live code > modifications. But if that break point is hit, it just skips calling > any handler, and makes the call site act as a nop. For tracing, the > worse that can happen is that you miss a function being traced, but > for live kernel patching the affects are more severe, as the old buggy > function is now called. > > To solve this, an int3_emulate_call() is created for x86_64 to allow > ftrace on x86_64 to emulate the call to ftrace_regs_caller() which will > make sure all the registered handlers to that function are still called. > And this keeps live kernel patching happy! Out of curiosity, would you have any idea to re-use these function for other use-case? Maybe kprobes can reuse it, but very limited use-case. > To mimimize the changes, and to avoid controversial patches, this > only changes x86_64. Due to the way x86_32 implements the regs->sp > the complexity of emulating calls on that platform is too much for > stable patches, and live kernel patching does not support x86_32 anyway. This series looks good to me. Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org> Thanks!
Steven Rostedt <rostedt@goodmis.org> writes: > [ > This is the non-RFC version. > > It went through and passed all my tests. If there's no objections > I'm going to include this in my pull request. I still have patches > in my INBOX that may still be included, so I need to run those through > my tests as well, so a pull request wont be immediate. > ] <snip /> > Josh Poimboeuf (1): > x86_64: Add gap to int3 to allow for call emulation > > Peter Zijlstra (2): > x86_64: Allow breakpoints to emulate call instructions > ftrace/x86_64: Emulate call function while updating in breakpoint handler Reviewed-and-tested-by: Nicolai Stange <nstange@suse.de> for the whole series. Many, many thanks to everybody involved! I'll resend that live patching selftest once this fix here has been merged. Nicolai
On Wed, 8 May 2019 13:30:22 +0900 Masami Hiramatsu <mhiramat@kernel.org> wrote: > > To solve this, an int3_emulate_call() is created for x86_64 to allow > > ftrace on x86_64 to emulate the call to ftrace_regs_caller() which will > > make sure all the registered handlers to that function are still called. > > And this keeps live kernel patching happy! > > Out of curiosity, would you have any idea to re-use these function for > other use-case? Maybe kprobes can reuse it, but very limited use-case. Yes, but only for x86_64. > > > To mimimize the changes, and to avoid controversial patches, this > > only changes x86_64. Due to the way x86_32 implements the regs->sp > > the complexity of emulating calls on that platform is too much for > > stable patches, and live kernel patching does not support x86_32 anyway. > > This series looks good to me. > > Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org> Thanks Masami! -- Steve