Re: [PATCH v5 1/3] tracing: kprobes: Prohibit probing on notrace function

[Masami Hiramatsu <mhiramat@kernel.org>]

On Mon, 30 Jul 2018 18:40:10 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Mon, 30 Jul 2018 19:20:14 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > Prohibit kprobe-events probing on notrace function.
> > Since probing on the notrace function can cause recursive
> > event call. In most case those are just skipped, but
> > in some case it falls into infinit recursive call.
> > 
> > This protection can be disabled by the kconfig
> > CONFIG_KPROBE_EVENTS_ON_NOTRACE=y, but it is highly
> > recommended to keep it "n" for normal kernel.
> > Note that this is only available if "kprobes on ftrace"
> > has been implemented on target arch and
> > CONFIG_KPROBES_ON_FTRACE=y.
> > 
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> >
> 
> Hi Masami,
> 
> Note, I made the following changes for grammar. For the Change log I
> have this:
> 
>     tracing: kprobes: Prohibit probing on notrace function
>     
>     Prohibit kprobe-events probing on notrace functions.  Since probing on a
>     notrace function can cause a recursive event call. In most cases those are just
>     skipped, but in some cases it falls into an infinite recursive call.
>     
>     This protection can be disabled by the kconfig
>     CONFIG_KPROBE_EVENTS_ON_NOTRACE=y, but it is highly recommended to keep it
>     "n" for normal kernel builds.  Note that this is only available if "kprobes on
>     ftrace" has been implemented on the target arch and CONFIG_KPROBES_ON_FTRACE=y.
>     
>     Link: http://lkml.kernel.org/r/153294601436.32740.10557881188933661239.stgit@devbox
>     
>     Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
>     Tested-by: Francis Deslauriers <francis.deslauriers@efficios.com>
>     [ Slight grammar and spelling fixes ]
>     Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

Looks good to me.
Thanks for fixing it! :)

> 
> 
> And this change to the patch:
> 
> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> index 4d4eb15cc7fd..23fc7c9abedb 100644
> --- a/kernel/trace/Kconfig
> +++ b/kernel/trace/Kconfig
> @@ -457,7 +457,7 @@ config KPROBE_EVENTS
>  	  If you want to use perf tools, this option is strongly recommended.
>  
>  config KPROBE_EVENTS_ON_NOTRACE
> -	bool "Do NOT protect notrace function from kprobe events"
> +	bool "Do NOT protect notrace functions from kprobe events"
>  	depends on KPROBE_EVENTS
>  	depends on KPROBES_ON_FTRACE
>  	default n
> @@ -465,13 +465,13 @@ config KPROBE_EVENTS_ON_NOTRACE
>  	  This is only for the developers who want to debug ftrace itself
>  	  using kprobe events.
>  
> -	  If kprobes can use ftrace instead of breakpoint, ftrace related
> -	  functions are protected from kprobe-events to prevent an infinit
> -	  recursion or any unexpected execution path which leads to a kernel
> -	  crash.
> +	  If kprobes is using ftrace to hook to a function instead of a
> +	  breakpoint, ftrace related functions are protected from
> +	  kprobe-events to prevent an infinite recursion or any unexpected
> +	  execution path which would lead to a kernel crash.
>  
>  	  This option disables such protection and allows you to put kprobe
> -	  events on ftrace functions for debugging ftrace by itself.
> +	  events on ftrace functions for debugging ftrace itself.
>  	  Note that this might let you shoot yourself in the foot.
>  
>  	  If unsure, say N.
> 
> 
> Are you OK with this?
> 
> -- Steve


-- 
Masami Hiramatsu <mhiramat@kernel.org>