linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] tracing/kprobe: Release kprobe print_fmt properly
@ 2018-07-09 14:19 Jiri Olsa
  2018-07-09 16:40 ` Song Liu
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Jiri Olsa @ 2018-07-09 14:19 UTC (permalink / raw)
  To: Steven Rostedt
  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

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>
---
 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 related	[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: 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
end of thread, other threads:[~2018-07-11 13:29 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
2018-07-11 13:26     ` Masami Hiramatsu
2018-07-11 13:29       ` Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).