Re: [PATCH 1/2] tracing: kprobes: Prohibit probing on notrace functions
On Thu, 26 Jul 2018 09:41:06 +0900 Masami Hiramatsu wrote: > On Fri, 13 Jul 2018 08:18:03 -0400 > Steven Rostedt wrote: > > > On Fri, 13 Jul 2018 11:53:01 +0900 > > Masami Hiramatsu wrote: > > > > > On Thu, 12 Jul 2018 13:54:12 -0400 > > > Francis Deslauriers wrote: > > > > > > > From: Masami Hiramatsu > > > > > > > > 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 infinite recursive call. > > > > > > BTW, I'm considering to add an option to allow putting > > > kprobes on notrace function - just for debugging > > > ftrace by kprobes. That is "developer only" option > > > so generally it should be disabled, but for debugging > > > the ftrace, we still need it. Or should I introduce > > > another kprobes module for debugging it? > > > > No, I think the former is better (to add an option to allow putting > > kprobes on notrace functions). By default we let people protect > > themselves. But if then provide a switch that lets you do things that > > might let you shoot yourself in the foot. > > I'm adding CONFIG_KPROBE_EVENTS_ON_NOTRACE kconfig which allows > kprobes on notrace function. I think we don't need to make it > online switchable, since it is only good for ftrace developers. > Works for me. Thanks! -- Steve
Re: [PATCH 1/2] tracing: kprobes: Prohibit probing on notrace functions
On Thu, 26 Jul 2018 09:41:06 +0900 Masami Hiramatsu wrote: > On Fri, 13 Jul 2018 08:18:03 -0400 > Steven Rostedt wrote: > > > On Fri, 13 Jul 2018 11:53:01 +0900 > > Masami Hiramatsu wrote: > > > > > On Thu, 12 Jul 2018 13:54:12 -0400 > > > Francis Deslauriers wrote: > > > > > > > From: Masami Hiramatsu > > > > > > > > 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 infinite recursive call. > > > > > > BTW, I'm considering to add an option to allow putting > > > kprobes on notrace function - just for debugging > > > ftrace by kprobes. That is "developer only" option > > > so generally it should be disabled, but for debugging > > > the ftrace, we still need it. Or should I introduce > > > another kprobes module for debugging it? > > > > No, I think the former is better (to add an option to allow putting > > kprobes on notrace functions). By default we let people protect > > themselves. But if then provide a switch that lets you do things that > > might let you shoot yourself in the foot. > > I'm adding CONFIG_KPROBE_EVENTS_ON_NOTRACE kconfig which allows > kprobes on notrace function. I think we don't need to make it > online switchable, since it is only good for ftrace developers. > Works for me. Thanks! -- Steve
Re: [PATCH 1/2] tracing: kprobes: Prohibit probing on notrace functions
On Fri, 13 Jul 2018 08:18:03 -0400 Steven Rostedt wrote: > On Fri, 13 Jul 2018 11:53:01 +0900 > Masami Hiramatsu wrote: > > > On Thu, 12 Jul 2018 13:54:12 -0400 > > Francis Deslauriers wrote: > > > > > From: Masami Hiramatsu > > > > > > 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 infinite recursive call. > > > > BTW, I'm considering to add an option to allow putting > > kprobes on notrace function - just for debugging > > ftrace by kprobes. That is "developer only" option > > so generally it should be disabled, but for debugging > > the ftrace, we still need it. Or should I introduce > > another kprobes module for debugging it? > > No, I think the former is better (to add an option to allow putting > kprobes on notrace functions). By default we let people protect > themselves. But if then provide a switch that lets you do things that > might let you shoot yourself in the foot. I'm adding CONFIG_KPROBE_EVENTS_ON_NOTRACE kconfig which allows kprobes on notrace function. I think we don't need to make it online switchable, since it is only good for ftrace developers. Thank you, > > BTW, I'm now leaving on vacation. I'll be back on the 23rd and will be > looking for patches that I should be pulling in then. > > Thanks! > > -- Steve -- Masami Hiramatsu
Re: [PATCH 1/2] tracing: kprobes: Prohibit probing on notrace functions
On Fri, 13 Jul 2018 08:18:03 -0400 Steven Rostedt wrote: > On Fri, 13 Jul 2018 11:53:01 +0900 > Masami Hiramatsu wrote: > > > On Thu, 12 Jul 2018 13:54:12 -0400 > > Francis Deslauriers wrote: > > > > > From: Masami Hiramatsu > > > > > > 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 infinite recursive call. > > > > BTW, I'm considering to add an option to allow putting > > kprobes on notrace function - just for debugging > > ftrace by kprobes. That is "developer only" option > > so generally it should be disabled, but for debugging > > the ftrace, we still need it. Or should I introduce > > another kprobes module for debugging it? > > No, I think the former is better (to add an option to allow putting > kprobes on notrace functions). By default we let people protect > themselves. But if then provide a switch that lets you do things that > might let you shoot yourself in the foot. I'm adding CONFIG_KPROBE_EVENTS_ON_NOTRACE kconfig which allows kprobes on notrace function. I think we don't need to make it online switchable, since it is only good for ftrace developers. Thank you, > > BTW, I'm now leaving on vacation. I'll be back on the 23rd and will be > looking for patches that I should be pulling in then. > > Thanks! > > -- Steve -- Masami Hiramatsu
Re: [PATCH 1/2] tracing: kprobes: Prohibit probing on notrace functions
On Fri, 13 Jul 2018 11:53:01 +0900 Masami Hiramatsu wrote: > On Thu, 12 Jul 2018 13:54:12 -0400 > Francis Deslauriers wrote: > > > From: Masami Hiramatsu > > > > 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 infinite recursive call. > > BTW, I'm considering to add an option to allow putting > kprobes on notrace function - just for debugging > ftrace by kprobes. That is "developer only" option > so generally it should be disabled, but for debugging > the ftrace, we still need it. Or should I introduce > another kprobes module for debugging it? No, I think the former is better (to add an option to allow putting kprobes on notrace functions). By default we let people protect themselves. But if then provide a switch that lets you do things that might let you shoot yourself in the foot. BTW, I'm now leaving on vacation. I'll be back on the 23rd and will be looking for patches that I should be pulling in then. Thanks! -- Steve
Re: [PATCH 1/2] tracing: kprobes: Prohibit probing on notrace functions
On Fri, 13 Jul 2018 11:53:01 +0900 Masami Hiramatsu wrote: > On Thu, 12 Jul 2018 13:54:12 -0400 > Francis Deslauriers wrote: > > > From: Masami Hiramatsu > > > > 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 infinite recursive call. > > BTW, I'm considering to add an option to allow putting > kprobes on notrace function - just for debugging > ftrace by kprobes. That is "developer only" option > so generally it should be disabled, but for debugging > the ftrace, we still need it. Or should I introduce > another kprobes module for debugging it? No, I think the former is better (to add an option to allow putting kprobes on notrace functions). By default we let people protect themselves. But if then provide a switch that lets you do things that might let you shoot yourself in the foot. BTW, I'm now leaving on vacation. I'll be back on the 23rd and will be looking for patches that I should be pulling in then. Thanks! -- Steve
Re: [PATCH 1/2] tracing: kprobes: Prohibit probing on notrace functions
On Thu, 12 Jul 2018 13:54:12 -0400 Francis Deslauriers wrote: > From: Masami Hiramatsu > > 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 infinite recursive call. BTW, I'm considering to add an option to allow putting kprobes on notrace function - just for debugging ftrace by kprobes. That is "developer only" option so generally it should be disabled, but for debugging the ftrace, we still need it. Or should I introduce another kprobes module for debugging it? Thank you, -- Masami Hiramatsu
Re: [PATCH 1/2] tracing: kprobes: Prohibit probing on notrace functions
On Thu, 12 Jul 2018 13:54:12 -0400 Francis Deslauriers wrote: > From: Masami Hiramatsu > > 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 infinite recursive call. BTW, I'm considering to add an option to allow putting kprobes on notrace function - just for debugging ftrace by kprobes. That is "developer only" option so generally it should be disabled, but for debugging the ftrace, we still need it. Or should I introduce another kprobes module for debugging it? Thank you, -- Masami Hiramatsu
Re: [PATCH 1/2] tracing: kprobes: Prohibit probing on notrace functions
On Thu, 12 Jul 2018 13:54:12 -0400
Francis Deslauriers wrote:
> From: Masami Hiramatsu
>
> 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 infinite recursive call.
>
> Signed-off-by: Masami Hiramatsu
> Tested-by: Francis Deslauriers
BTW, since you are sending the patch, you must also put in your
Signed-off-by too, after Masami's. You can keep the Tested-by.
-- Steve
> ---
> kernel/trace/trace_kprobe.c | 23 +++
> 1 file changed, 23 insertions(+)
>
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index daa8157..952dc2a 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -487,6 +487,23 @@ disable_trace_kprobe(struct trace_kprobe *tk, struct
> trace_event_file *file)
> return ret;
> }
>
> +#ifdef CONFIG_KPROBES_ON_FTRACE
> +static bool within_notrace_func(struct trace_kprobe *tk)
> +{
> + unsigned long offset, size, addr;
> +
> + addr = kallsyms_lookup_name(trace_kprobe_symbol(tk));
> + addr += trace_kprobe_offset(tk);
> +
> + if (!kallsyms_lookup_size_offset(addr, , ))
> + return true;/* Out of range. */
> +
> + return !ftrace_location_range(addr - offset, addr - offset + size);
> +}
> +#else
> +#define within_notrace_func(tk) (false)
> +#endif
> +
> /* Internal register function - just handle k*probes and flags */
> static int __register_trace_kprobe(struct trace_kprobe *tk)
> {
> @@ -495,6 +512,12 @@ static int __register_trace_kprobe(struct trace_kprobe
> *tk)
> if (trace_probe_is_registered(>tp))
> return -EINVAL;
>
> + if (within_notrace_func(tk)) {
> + pr_warn("Could not probe notrace function %s\n",
> + trace_kprobe_symbol(tk));
> + return -EINVAL;
> + }
> +
> for (i = 0; i < tk->tp.nr_args; i++)
> traceprobe_update_arg(>tp.args[i]);
>
Re: [PATCH 1/2] tracing: kprobes: Prohibit probing on notrace functions
On Thu, 12 Jul 2018 13:54:12 -0400
Francis Deslauriers wrote:
> From: Masami Hiramatsu
>
> 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 infinite recursive call.
>
> Signed-off-by: Masami Hiramatsu
> Tested-by: Francis Deslauriers
BTW, since you are sending the patch, you must also put in your
Signed-off-by too, after Masami's. You can keep the Tested-by.
-- Steve
> ---
> kernel/trace/trace_kprobe.c | 23 +++
> 1 file changed, 23 insertions(+)
>
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index daa8157..952dc2a 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -487,6 +487,23 @@ disable_trace_kprobe(struct trace_kprobe *tk, struct
> trace_event_file *file)
> return ret;
> }
>
> +#ifdef CONFIG_KPROBES_ON_FTRACE
> +static bool within_notrace_func(struct trace_kprobe *tk)
> +{
> + unsigned long offset, size, addr;
> +
> + addr = kallsyms_lookup_name(trace_kprobe_symbol(tk));
> + addr += trace_kprobe_offset(tk);
> +
> + if (!kallsyms_lookup_size_offset(addr, , ))
> + return true;/* Out of range. */
> +
> + return !ftrace_location_range(addr - offset, addr - offset + size);
> +}
> +#else
> +#define within_notrace_func(tk) (false)
> +#endif
> +
> /* Internal register function - just handle k*probes and flags */
> static int __register_trace_kprobe(struct trace_kprobe *tk)
> {
> @@ -495,6 +512,12 @@ static int __register_trace_kprobe(struct trace_kprobe
> *tk)
> if (trace_probe_is_registered(>tp))
> return -EINVAL;
>
> + if (within_notrace_func(tk)) {
> + pr_warn("Could not probe notrace function %s\n",
> + trace_kprobe_symbol(tk));
> + return -EINVAL;
> + }
> +
> for (i = 0; i < tk->tp.nr_args; i++)
> traceprobe_update_arg(>tp.args[i]);
>
