* Re: [PATCH] tracing/kprobe: Release kprobe print_fmt properly
2018-07-09 14:19 [PATCH] tracing/kprobe: Release kprobe print_fmt properly Jiri Olsa
@ 2018-07-09 16:40 ` Song Liu
2018-07-09 16:44 ` Steven Rostedt
2018-07-10 13:04 ` Steven Rostedt
2 siblings, 0 replies; 7+ messages in thread
From: Song Liu @ 2018-07-09 16:40 UTC (permalink / raw)
To: Jiri Olsa
Cc: Steven Rostedt, lkml, Ingo Molnar, Peter Zijlstra (Intel),
Yonghong Song, Josef Bacik, Daniel Borkmann, David S. Miller,
Arnaldo Carvalho de Melo, Jiri Olsa, Namhyung Kim,
Thomas Gleixner
> On Jul 9, 2018, at 7:19 AM, Jiri Olsa <jolsa@kernel.org> wrote:
>
> We don't release tk->tp.call.print_fmt when destroying
> local uprobe. Also there's missing print_fmt kfree in
> create_local_trace_kprobe error path.
>
> Fixes: e12f03d7031a ("perf/core: Implement the 'perf_kprobe' PMU")
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Thanks for the fix!
Acked-by: Song Liu <songliubraving@fb.com>
> ---
> kernel/trace/trace_kprobe.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index daa81571b22a..21f718472942 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -1480,8 +1480,10 @@ create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
> }
>
> ret = __register_trace_kprobe(tk);
> - if (ret < 0)
> + if (ret < 0) {
> + kfree(tk->tp.call.print_fmt);
> goto error;
> + }
>
> return &tk->tp.call;
> error:
> @@ -1501,6 +1503,8 @@ void destroy_local_trace_kprobe(struct trace_event_call *event_call)
> }
>
> __unregister_trace_kprobe(tk);
> +
> + kfree(tk->tp.call.print_fmt);
> free_trace_kprobe(tk);
> }
> #endif /* CONFIG_PERF_EVENTS */
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] tracing/kprobe: Release kprobe print_fmt properly
2018-07-09 14:19 [PATCH] tracing/kprobe: Release kprobe print_fmt properly Jiri Olsa
2018-07-09 16:40 ` Song Liu
@ 2018-07-09 16:44 ` Steven Rostedt
2018-07-10 13:04 ` Steven Rostedt
2 siblings, 0 replies; 7+ messages in thread
From: Steven Rostedt @ 2018-07-09 16:44 UTC (permalink / raw)
To: Jiri Olsa
Cc: lkml, Ingo Molnar, Song Liu, Peter Zijlstra (Intel),
Yonghong Song, Josef Bacik, daniel, davem,
Arnaldo Carvalho de Melo, Jiri Olsa, Namhyung Kim,
Thomas Gleixner
On Mon, 9 Jul 2018 16:19:06 +0200
Jiri Olsa <jolsa@kernel.org> wrote:
> We don't release tk->tp.call.print_fmt when destroying
> local uprobe. Also there's missing print_fmt kfree in
> create_local_trace_kprobe error path.
>
> Fixes: e12f03d7031a ("perf/core: Implement the 'perf_kprobe' PMU")
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Thanks for the patch, I'm applying it now (and testing it).
> ---
> kernel/trace/trace_kprobe.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index daa81571b22a..21f718472942 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -1480,8 +1480,10 @@ create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
> }
>
> ret = __register_trace_kprobe(tk);
> - if (ret < 0)
> + if (ret < 0) {
> + kfree(tk->tp.call.print_fmt);
> goto error;
> + }
>
> return &tk->tp.call;
> error:
> @@ -1501,6 +1503,8 @@ void destroy_local_trace_kprobe(struct trace_event_call *event_call)
> }
>
> __unregister_trace_kprobe(tk);
> +
> + kfree(tk->tp.call.print_fmt);
Bah! The naming convention of "set_print_fmt()" is horrible, and leads
to these kinds of bugs. I'll make a patch (not for stable though) that
makes it a bit more obvious to what is happening.
-- Steve
> free_trace_kprobe(tk);
> }
> #endif /* CONFIG_PERF_EVENTS */
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] tracing/kprobe: Release kprobe print_fmt properly
2018-07-09 14:19 [PATCH] tracing/kprobe: Release kprobe print_fmt properly Jiri Olsa
2018-07-09 16:40 ` Song Liu
2018-07-09 16:44 ` Steven Rostedt
@ 2018-07-10 13:04 ` Steven Rostedt
2018-07-11 12:54 ` Steven Rostedt
2 siblings, 1 reply; 7+ messages in thread
From: Steven Rostedt @ 2018-07-10 13:04 UTC (permalink / raw)
To: Jiri Olsa, Masami Hiramatsu
Cc: lkml, Ingo Molnar, Song Liu, Peter Zijlstra (Intel),
Yonghong Song, Josef Bacik, daniel, davem,
Arnaldo Carvalho de Melo, Jiri Olsa, Namhyung Kim,
Thomas Gleixner
On Mon, 9 Jul 2018 16:19:06 +0200
Jiri Olsa <jolsa@kernel.org> wrote:
> We don't release tk->tp.call.print_fmt when destroying
> local uprobe. Also there's missing print_fmt kfree in
> create_local_trace_kprobe error path.
OK, I finished testing and I'm all ready to push, but then I noticed
that Masami wasn't Cc'd. I like to get his ack before pushing to Linus.
-- Steve
>
> Fixes: e12f03d7031a ("perf/core: Implement the 'perf_kprobe' PMU")
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> kernel/trace/trace_kprobe.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index daa81571b22a..21f718472942 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -1480,8 +1480,10 @@ create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
> }
>
> ret = __register_trace_kprobe(tk);
> - if (ret < 0)
> + if (ret < 0) {
> + kfree(tk->tp.call.print_fmt);
> goto error;
> + }
>
> return &tk->tp.call;
> error:
> @@ -1501,6 +1503,8 @@ void destroy_local_trace_kprobe(struct trace_event_call *event_call)
> }
>
> __unregister_trace_kprobe(tk);
> +
> + kfree(tk->tp.call.print_fmt);
> free_trace_kprobe(tk);
> }
> #endif /* CONFIG_PERF_EVENTS */
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] tracing/kprobe: Release kprobe print_fmt properly
2018-07-10 13:04 ` Steven Rostedt
@ 2018-07-11 12:54 ` Steven Rostedt
2018-07-11 13:26 ` Masami Hiramatsu
0 siblings, 1 reply; 7+ messages in thread
From: Steven Rostedt @ 2018-07-11 12:54 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Jiri Olsa, lkml, Ingo Molnar, Song Liu, Peter Zijlstra (Intel),
Yonghong Song, Josef Bacik, daniel, davem,
Arnaldo Carvalho de Melo, Jiri Olsa, Namhyung Kim,
Thomas Gleixner
Masami,
Can you ack this? If I don't hear from you by tomorrow, I'm going to
push it to Linus, because I'll be on PTO after that.
Thanks!
-- Steve
On Tue, 10 Jul 2018 09:04:17 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Mon, 9 Jul 2018 16:19:06 +0200
> Jiri Olsa <jolsa@kernel.org> wrote:
>
> > We don't release tk->tp.call.print_fmt when destroying
> > local uprobe. Also there's missing print_fmt kfree in
> > create_local_trace_kprobe error path.
>
> OK, I finished testing and I'm all ready to push, but then I noticed
> that Masami wasn't Cc'd. I like to get his ack before pushing to Linus.
>
> -- Steve
>
>
> >
> > Fixes: e12f03d7031a ("perf/core: Implement the 'perf_kprobe' PMU")
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> > kernel/trace/trace_kprobe.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> > index daa81571b22a..21f718472942 100644
> > --- a/kernel/trace/trace_kprobe.c
> > +++ b/kernel/trace/trace_kprobe.c
> > @@ -1480,8 +1480,10 @@ create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
> > }
> >
> > ret = __register_trace_kprobe(tk);
> > - if (ret < 0)
> > + if (ret < 0) {
> > + kfree(tk->tp.call.print_fmt);
> > goto error;
> > + }
> >
> > return &tk->tp.call;
> > error:
> > @@ -1501,6 +1503,8 @@ void destroy_local_trace_kprobe(struct trace_event_call *event_call)
> > }
> >
> > __unregister_trace_kprobe(tk);
> > +
> > + kfree(tk->tp.call.print_fmt);
> > free_trace_kprobe(tk);
> > }
> > #endif /* CONFIG_PERF_EVENTS */
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] tracing/kprobe: Release kprobe print_fmt properly
2018-07-11 12:54 ` Steven Rostedt
@ 2018-07-11 13:26 ` Masami Hiramatsu
2018-07-11 13:29 ` Steven Rostedt
0 siblings, 1 reply; 7+ messages in thread
From: Masami Hiramatsu @ 2018-07-11 13:26 UTC (permalink / raw)
To: Steven Rostedt
Cc: Jiri Olsa, lkml, Ingo Molnar, Song Liu, Peter Zijlstra (Intel),
Yonghong Song, Josef Bacik, daniel, davem,
Arnaldo Carvalho de Melo, Jiri Olsa, Namhyung Kim,
Thomas Gleixner
On Wed, 11 Jul 2018 08:54:29 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
>
> Masami,
>
> Can you ack this? If I don't hear from you by tomorrow, I'm going to
> push it to Linus, because I'll be on PTO after that.
>
Sorry for later, yes, it seems correct.
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Thanks!
> Thanks!
>
> -- Steve
>
>
> On Tue, 10 Jul 2018 09:04:17 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
> > On Mon, 9 Jul 2018 16:19:06 +0200
> > Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > > We don't release tk->tp.call.print_fmt when destroying
> > > local uprobe. Also there's missing print_fmt kfree in
> > > create_local_trace_kprobe error path.
> >
> > OK, I finished testing and I'm all ready to push, but then I noticed
> > that Masami wasn't Cc'd. I like to get his ack before pushing to Linus.
> >
> > -- Steve
> >
> >
> > >
> > > Fixes: e12f03d7031a ("perf/core: Implement the 'perf_kprobe' PMU")
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > ---
> > > kernel/trace/trace_kprobe.c | 6 +++++-
> > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> > > index daa81571b22a..21f718472942 100644
> > > --- a/kernel/trace/trace_kprobe.c
> > > +++ b/kernel/trace/trace_kprobe.c
> > > @@ -1480,8 +1480,10 @@ create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
> > > }
> > >
> > > ret = __register_trace_kprobe(tk);
> > > - if (ret < 0)
> > > + if (ret < 0) {
> > > + kfree(tk->tp.call.print_fmt);
> > > goto error;
> > > + }
> > >
> > > return &tk->tp.call;
> > > error:
> > > @@ -1501,6 +1503,8 @@ void destroy_local_trace_kprobe(struct trace_event_call *event_call)
> > > }
> > >
> > > __unregister_trace_kprobe(tk);
> > > +
> > > + kfree(tk->tp.call.print_fmt);
> > > free_trace_kprobe(tk);
> > > }
> > > #endif /* CONFIG_PERF_EVENTS */
> >
>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] tracing/kprobe: Release kprobe print_fmt properly
2018-07-11 13:26 ` Masami Hiramatsu
@ 2018-07-11 13:29 ` Steven Rostedt
0 siblings, 0 replies; 7+ messages in thread
From: Steven Rostedt @ 2018-07-11 13:29 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Jiri Olsa, lkml, Ingo Molnar, Song Liu, Peter Zijlstra (Intel),
Yonghong Song, Josef Bacik, daniel, davem,
Arnaldo Carvalho de Melo, Jiri Olsa, Namhyung Kim,
Thomas Gleixner
On Wed, 11 Jul 2018 22:26:37 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:
> On Wed, 11 Jul 2018 08:54:29 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
> >
> > Masami,
> >
> > Can you ack this? If I don't hear from you by tomorrow, I'm going to
> > push it to Linus, because I'll be on PTO after that.
> >
>
> Sorry for later, yes, it seems correct.
>
> Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
>
Thanks Masami!
-- Steve
^ permalink raw reply [flat|nested] 7+ messages in thread