Re: [PATCH v2 1/4] powerpc/kprobes: Pause function_graph tracing during jprobes handling

[Steven Rostedt]

On Thu, 8 Jun 2017 22:43:54 +0900
Masami Hiramatsu  wrote:

> On Thu,  1 Jun 2017 16:18:15 +0530
> "Naveen N. Rao"  wrote:
> 
> > This fixes a crash when function_graph and jprobes are used together.
> > This is essentially commit 237d28db036e ("ftrace/jprobes/x86: Fix
> > conflict between jprobes and function graph tracing"), but for powerpc.
> > 
> > Jprobes breaks function_graph tracing since the jprobe hook needs to use
> > jprobe_return(), which never returns back to the hook, but instead to
> > the original jprobe'd function. The solution is to momentarily pause
> > function_graph tracing before invoking the jprobe hook and re-enable it
> > when returning back to the original jprobe'd function.
> > 
> > Fixes: 6794c78243bfd ("powerpc64: port of the function graph tracer")
> > Signed-off-by: Naveen N. Rao   
> 
> Acked-by: Masami Hiramatsu 

For what it's worth...

Acked-by: Steven Rostedt (VMware) 

-- Steve

> 
> Thanks!
> 
> > ---
> > v2: No changes since v1.
> > 
> >  arch/powerpc/kernel/kprobes.c | 11 +++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> > index e1fb95385a9e..0554d6e66194 100644
> > --- a/arch/powerpc/kernel/kprobes.c
> > +++ b/arch/powerpc/kernel/kprobes.c
> > @@ -610,6 +610,15 @@ int setjmp_pre_handler(struct kprobe *p, struct 
> > pt_regs *regs)
> > regs->gpr[2] = (unsigned long)(((func_descr_t *)jp->entry)->toc);
> >  #endif
> >  
> > +   /*
> > +* jprobes use jprobe_return() which skips the normal return
> > +* path of the function, and this messes up the accounting of the
> > +* function graph tracer.
> > +*
> > +* Pause function graph tracing while performing the jprobe function.
> > +*/
> > +   pause_graph_tracing();
> > +
> > return 1;
> >  }
> >  NOKPROBE_SYMBOL(setjmp_pre_handler);
> > @@ -635,6 +644,8 @@ int longjmp_break_handler(struct kprobe *p, struct 
> > pt_regs *regs)
> >  * saved regs...
> >  */
> > memcpy(regs, >jprobe_saved_regs, sizeof(struct pt_regs));
> > +   /* It's OK to start function graph tracing again */
> > +   unpause_graph_tracing();
> > preempt_enable_no_resched();
> > return 1;
> >  }
> > -- 
> > 2.12.2
> >   
> 
> 

Re: [PATCH v2 1/4] powerpc/kprobes: Pause function_graph tracing during jprobes handling

[Masami Hiramatsu]

On Thu,  1 Jun 2017 16:18:15 +0530
"Naveen N. Rao"  wrote:

> This fixes a crash when function_graph and jprobes are used together.
> This is essentially commit 237d28db036e ("ftrace/jprobes/x86: Fix
> conflict between jprobes and function graph tracing"), but for powerpc.
> 
> Jprobes breaks function_graph tracing since the jprobe hook needs to use
> jprobe_return(), which never returns back to the hook, but instead to
> the original jprobe'd function. The solution is to momentarily pause
> function_graph tracing before invoking the jprobe hook and re-enable it
> when returning back to the original jprobe'd function.
> 
> Fixes: 6794c78243bfd ("powerpc64: port of the function graph tracer")
> Signed-off-by: Naveen N. Rao 

Acked-by: Masami Hiramatsu 

Thanks!

> ---
> v2: No changes since v1.
> 
>  arch/powerpc/kernel/kprobes.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> index e1fb95385a9e..0554d6e66194 100644
> --- a/arch/powerpc/kernel/kprobes.c
> +++ b/arch/powerpc/kernel/kprobes.c
> @@ -610,6 +610,15 @@ int setjmp_pre_handler(struct kprobe *p, struct pt_regs 
> *regs)
>   regs->gpr[2] = (unsigned long)(((func_descr_t *)jp->entry)->toc);
>  #endif
>  
> + /*
> +  * jprobes use jprobe_return() which skips the normal return
> +  * path of the function, and this messes up the accounting of the
> +  * function graph tracer.
> +  *
> +  * Pause function graph tracing while performing the jprobe function.
> +  */
> + pause_graph_tracing();
> +
>   return 1;
>  }
>  NOKPROBE_SYMBOL(setjmp_pre_handler);
> @@ -635,6 +644,8 @@ int longjmp_break_handler(struct kprobe *p, struct 
> pt_regs *regs)
>* saved regs...
>*/
>   memcpy(regs, >jprobe_saved_regs, sizeof(struct pt_regs));
> + /* It's OK to start function graph tracing again */
> + unpause_graph_tracing();
>   preempt_enable_no_resched();
>   return 1;
>  }
> -- 
> 2.12.2
> 


-- 
Masami Hiramatsu 

[PATCH v2 1/4] powerpc/kprobes: Pause function_graph tracing during jprobes handling

[Naveen N. Rao]

This fixes a crash when function_graph and jprobes are used together.
This is essentially commit 237d28db036e ("ftrace/jprobes/x86: Fix
conflict between jprobes and function graph tracing"), but for powerpc.

Jprobes breaks function_graph tracing since the jprobe hook needs to use
jprobe_return(), which never returns back to the hook, but instead to
the original jprobe'd function. The solution is to momentarily pause
function_graph tracing before invoking the jprobe hook and re-enable it
when returning back to the original jprobe'd function.

Fixes: 6794c78243bfd ("powerpc64: port of the function graph tracer")
Signed-off-by: Naveen N. Rao 
---
v2: No changes since v1.

 arch/powerpc/kernel/kprobes.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index e1fb95385a9e..0554d6e66194 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -610,6 +610,15 @@ int setjmp_pre_handler(struct kprobe *p, struct pt_regs 
*regs)
regs->gpr[2] = (unsigned long)(((func_descr_t *)jp->entry)->toc);
 #endif
 
+   /*
+* jprobes use jprobe_return() which skips the normal return
+* path of the function, and this messes up the accounting of the
+* function graph tracer.
+*
+* Pause function graph tracing while performing the jprobe function.
+*/
+   pause_graph_tracing();
+
return 1;
 }
 NOKPROBE_SYMBOL(setjmp_pre_handler);
@@ -635,6 +644,8 @@ int longjmp_break_handler(struct kprobe *p, struct pt_regs 
*regs)
 * saved regs...
 */
memcpy(regs, >jprobe_saved_regs, sizeof(struct pt_regs));
+   /* It's OK to start function graph tracing again */
+   unpause_graph_tracing();
preempt_enable_no_resched();
return 1;
 }
-- 
2.12.2