| Message ID | 20180117122224.19860-1-naveen.n.rao@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | e6e133c47e6bd4d5dac05b35d06634a8e5648615 |
| Headers | show |
| Series | powerpc/kprobes: Fix call trace due to incorrect preempt count | expand |
On Wed, Jan 17, 2018 at 05:52:24PM +0530, Naveen N. Rao wrote: > Michael Ellerman reported the following call trace when running > ftracetest: > > BUG: using __this_cpu_write() in preemptible [00000000] code: ftracetest/6178 > caller is opt_pre_handler+0xc4/0x110 > CPU: 1 PID: 6178 Comm: ftracetest Not tainted 4.15.0-rc7-gcc6x-gb2cd1df #1 > Call Trace: > [c0000000f9ec39c0] [c000000000ac4304] dump_stack+0xb4/0x100 (unreliable) > [c0000000f9ec3a00] [c00000000061159c] check_preemption_disabled+0x15c/0x170 > [c0000000f9ec3a90] [c000000000217e84] opt_pre_handler+0xc4/0x110 > [c0000000f9ec3af0] [c00000000004cf68] optimized_callback+0x148/0x170 > [c0000000f9ec3b40] [c00000000004d954] optinsn_slot+0xec/0x10000 > [c0000000f9ec3e30] [c00000000004bae0] kretprobe_trampoline+0x0/0x10 > > This is showing up since OPTPROBES is now enabled with CONFIG_PREEMPT. > > trampoline_probe_handler() considers itself to be a special kprobe > handler for kretprobes. In doing so, it expects to be called from > kprobe_handler() on a trap, and re-enables preemption before returning a > non-zero return value so as to suppress any subsequent processing of the > trap by the kprobe_handler(). > > However, with optprobes, we don't deal with special handlers (we ignore > the return code) and just try to re-enable preemption causing the above > trace. > > To address this, modify trampoline_probe_handler() to not be special. > The only additional processing done in kprobe_handler() is to emulate > the instruction (in this case, a 'nop'). We adjust the value of > regs->nip for the purpose and delegate the job of re-enabling > preemption and resetting current kprobe to the probe handlers > (kprobe_handler() or optimized_callback()). > > Reported-by: Michael Ellerman <mpe@ellerman.id.au> > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> Acked-by: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com> writes: > On Wed, Jan 17, 2018 at 05:52:24PM +0530, Naveen N. Rao wrote: >> Michael Ellerman reported the following call trace when running >> ftracetest: >> >> BUG: using __this_cpu_write() in preemptible [00000000] code: ftracetest/6178 >> caller is opt_pre_handler+0xc4/0x110 >> CPU: 1 PID: 6178 Comm: ftracetest Not tainted 4.15.0-rc7-gcc6x-gb2cd1df #1 >> Call Trace: >> [c0000000f9ec39c0] [c000000000ac4304] dump_stack+0xb4/0x100 (unreliable) >> [c0000000f9ec3a00] [c00000000061159c] check_preemption_disabled+0x15c/0x170 >> [c0000000f9ec3a90] [c000000000217e84] opt_pre_handler+0xc4/0x110 >> [c0000000f9ec3af0] [c00000000004cf68] optimized_callback+0x148/0x170 >> [c0000000f9ec3b40] [c00000000004d954] optinsn_slot+0xec/0x10000 >> [c0000000f9ec3e30] [c00000000004bae0] kretprobe_trampoline+0x0/0x10 >> >> This is showing up since OPTPROBES is now enabled with CONFIG_PREEMPT. >> >> trampoline_probe_handler() considers itself to be a special kprobe >> handler for kretprobes. In doing so, it expects to be called from >> kprobe_handler() on a trap, and re-enables preemption before returning a >> non-zero return value so as to suppress any subsequent processing of the >> trap by the kprobe_handler(). >> >> However, with optprobes, we don't deal with special handlers (we ignore >> the return code) and just try to re-enable preemption causing the above >> trace. >> >> To address this, modify trampoline_probe_handler() to not be special. >> The only additional processing done in kprobe_handler() is to emulate >> the instruction (in this case, a 'nop'). We adjust the value of >> regs->nip for the purpose and delegate the job of re-enabling >> preemption and resetting current kprobe to the probe handlers >> (kprobe_handler() or optimized_callback()). >> >> Reported-by: Michael Ellerman <mpe@ellerman.id.au> >> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> > > Acked-by: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com> Fixes: 51c9c0843993 ("powerpc/kprobes: Implement Optprobes") ?? cheers
Michael Ellerman wrote: > Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com> writes: > >> On Wed, Jan 17, 2018 at 05:52:24PM +0530, Naveen N. Rao wrote: >>> Michael Ellerman reported the following call trace when running >>> ftracetest: >>> >>> BUG: using __this_cpu_write() in preemptible [00000000] code: ftracetest/6178 >>> caller is opt_pre_handler+0xc4/0x110 >>> CPU: 1 PID: 6178 Comm: ftracetest Not tainted 4.15.0-rc7-gcc6x-gb2cd1df #1 >>> Call Trace: >>> [c0000000f9ec39c0] [c000000000ac4304] dump_stack+0xb4/0x100 (unreliable) >>> [c0000000f9ec3a00] [c00000000061159c] check_preemption_disabled+0x15c/0x170 >>> [c0000000f9ec3a90] [c000000000217e84] opt_pre_handler+0xc4/0x110 >>> [c0000000f9ec3af0] [c00000000004cf68] optimized_callback+0x148/0x170 >>> [c0000000f9ec3b40] [c00000000004d954] optinsn_slot+0xec/0x10000 >>> [c0000000f9ec3e30] [c00000000004bae0] kretprobe_trampoline+0x0/0x10 >>> >>> This is showing up since OPTPROBES is now enabled with CONFIG_PREEMPT. >>> >>> trampoline_probe_handler() considers itself to be a special kprobe >>> handler for kretprobes. In doing so, it expects to be called from >>> kprobe_handler() on a trap, and re-enables preemption before returning a >>> non-zero return value so as to suppress any subsequent processing of the >>> trap by the kprobe_handler(). >>> >>> However, with optprobes, we don't deal with special handlers (we ignore >>> the return code) and just try to re-enable preemption causing the above >>> trace. >>> >>> To address this, modify trampoline_probe_handler() to not be special. >>> The only additional processing done in kprobe_handler() is to emulate >>> the instruction (in this case, a 'nop'). We adjust the value of >>> regs->nip for the purpose and delegate the job of re-enabling >>> preemption and resetting current kprobe to the probe handlers >>> (kprobe_handler() or optimized_callback()). >>> >>> Reported-by: Michael Ellerman <mpe@ellerman.id.au> >>> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> >> >> Acked-by: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com> > > Fixes: 51c9c0843993 ("powerpc/kprobes: Implement Optprobes") Fixes: 8a2d71a3f2737e ("powerpc/kprobes: Disable preemption before invoking probe handler for optprobes") I think this is more appropriate. I should have caught this issue with kretprobes, but I am fairly certain that I ran ftracetest at that point, but didn't see any call traces. Regards, Naveen
On Wed, 2018-01-17 at 12:22:24 UTC, "Naveen N. Rao" wrote: > Michael Ellerman reported the following call trace when running > ftracetest: > > BUG: using __this_cpu_write() in preemptible [00000000] code: ftracetest/6178 > caller is opt_pre_handler+0xc4/0x110 > CPU: 1 PID: 6178 Comm: ftracetest Not tainted 4.15.0-rc7-gcc6x-gb2cd1df #1 > Call Trace: > [c0000000f9ec39c0] [c000000000ac4304] dump_stack+0xb4/0x100 (unreliable) > [c0000000f9ec3a00] [c00000000061159c] check_preemption_disabled+0x15c/0x170 > [c0000000f9ec3a90] [c000000000217e84] opt_pre_handler+0xc4/0x110 > [c0000000f9ec3af0] [c00000000004cf68] optimized_callback+0x148/0x170 > [c0000000f9ec3b40] [c00000000004d954] optinsn_slot+0xec/0x10000 > [c0000000f9ec3e30] [c00000000004bae0] kretprobe_trampoline+0x0/0x10 > > This is showing up since OPTPROBES is now enabled with CONFIG_PREEMPT. > > trampoline_probe_handler() considers itself to be a special kprobe > handler for kretprobes. In doing so, it expects to be called from > kprobe_handler() on a trap, and re-enables preemption before returning a > non-zero return value so as to suppress any subsequent processing of the > trap by the kprobe_handler(). > > However, with optprobes, we don't deal with special handlers (we ignore > the return code) and just try to re-enable preemption causing the above > trace. > > To address this, modify trampoline_probe_handler() to not be special. > The only additional processing done in kprobe_handler() is to emulate > the instruction (in this case, a 'nop'). We adjust the value of > regs->nip for the purpose and delegate the job of re-enabling > preemption and resetting current kprobe to the probe handlers > (kprobe_handler() or optimized_callback()). > > Reported-by: Michael Ellerman <mpe@ellerman.id.au> > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> > Acked-by: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com> Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/e6e133c47e6bd4d5dac05b35d06634 cheers
diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c index ca5d5a081e75..e4c5bf33970b 100644 --- a/arch/powerpc/kernel/kprobes.c +++ b/arch/powerpc/kernel/kprobes.c @@ -455,29 +455,33 @@ static int trampoline_probe_handler(struct kprobe *p, struct pt_regs *regs) } kretprobe_assert(ri, orig_ret_address, trampoline_address); - regs->nip = orig_ret_address; + /* - * Make LR point to the orig_ret_address. - * When the 'nop' inside the kretprobe_trampoline - * is optimized, we can do a 'blr' after executing the - * detour buffer code. + * We get here through one of two paths: + * 1. by taking a trap -> kprobe_handler() -> here + * 2. by optprobe branch -> optimized_callback() -> opt_pre_handler() -> here + * + * When going back through (1), we need regs->nip to be setup properly + * as it is used to determine the return address from the trap. + * For (2), since nip is not honoured with optprobes, we instead setup + * the link register properly so that the subsequent 'blr' in + * kretprobe_trampoline jumps back to the right instruction. + * + * For nip, we should set the address to the previous instruction since + * we end up emulating it in kprobe_handler(), which increments the nip + * again. */ + regs->nip = orig_ret_address - 4; regs->link = orig_ret_address; - reset_current_kprobe(); kretprobe_hash_unlock(current, &flags); - preempt_enable_no_resched(); hlist_for_each_entry_safe(ri, tmp, &empty_rp, hlist) { hlist_del(&ri->hlist); kfree(ri); } - /* - * By returning a non-zero value, we are telling - * kprobe_handler() that we don't want the post_handler - * to run (and have re-enabled preemption) - */ - return 1; + + return 0; } NOKPROBE_SYMBOL(trampoline_probe_handler);
Michael Ellerman reported the following call trace when running ftracetest: BUG: using __this_cpu_write() in preemptible [00000000] code: ftracetest/6178 caller is opt_pre_handler+0xc4/0x110 CPU: 1 PID: 6178 Comm: ftracetest Not tainted 4.15.0-rc7-gcc6x-gb2cd1df #1 Call Trace: [c0000000f9ec39c0] [c000000000ac4304] dump_stack+0xb4/0x100 (unreliable) [c0000000f9ec3a00] [c00000000061159c] check_preemption_disabled+0x15c/0x170 [c0000000f9ec3a90] [c000000000217e84] opt_pre_handler+0xc4/0x110 [c0000000f9ec3af0] [c00000000004cf68] optimized_callback+0x148/0x170 [c0000000f9ec3b40] [c00000000004d954] optinsn_slot+0xec/0x10000 [c0000000f9ec3e30] [c00000000004bae0] kretprobe_trampoline+0x0/0x10 This is showing up since OPTPROBES is now enabled with CONFIG_PREEMPT. trampoline_probe_handler() considers itself to be a special kprobe handler for kretprobes. In doing so, it expects to be called from kprobe_handler() on a trap, and re-enables preemption before returning a non-zero return value so as to suppress any subsequent processing of the trap by the kprobe_handler(). However, with optprobes, we don't deal with special handlers (we ignore the return code) and just try to re-enable preemption causing the above trace. To address this, modify trampoline_probe_handler() to not be special. The only additional processing done in kprobe_handler() is to emulate the instruction (in this case, a 'nop'). We adjust the value of regs->nip for the purpose and delegate the job of re-enabling preemption and resetting current kprobe to the probe handlers (kprobe_handler() or optimized_callback()). Reported-by: Michael Ellerman <mpe@ellerman.id.au> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> --- arch/powerpc/kernel/kprobes.c | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-)