tracing/kprobe: Release kprobe print_fmt properly

PATCH In review Jiri Olsa 4 participants 7 messages Jul 9, 2018 – Jul 11, 2018 Tracing

Share & tools

Jiri Olsa

author

2018-07-09 14:19 UTC #0 lore raw

PATCH 1/1 tracing/kprobe: Release kprobe print_fmt properly

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 <[email protected]>

---
 kernel/trace/trace_kprobe.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

kernel/trace/trace_kprobe.c +5 -1

index daa81571b22a..21f718472942 100644
@@ -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 */

Signature · 1 line

2.17.1

Song Liu

Acked-by on v1 1/1

2018-07-09 16:41 UTC #1 lore raw

Quoted 10 lines · Jiri Olsa

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 <[email protected]>

Thanks for the fix!

Acked-by Song Liu <[email protected]>

Quoted 32 lines

---
kernel/trace/trace_kprobe.c | 6 +++++-

1 file changed, 5 insertions(+), 1 deletion(-)

kernel/trace/trace_kprobe.c +3 -1

index daa81571b22a..21f718472942 100644
@@ -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:

kernel/trace/trace_kprobe.c +2 -0

@@ -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 */

Signature · 1 line

2.17.1

Steven Rostedt

maintainer

2018-07-09 16:44 UTC #2 lore raw

In reply to Jiri Olsa

Quoted 6 lines · On Mon, 9 Jul 2018 16:19:06 +0200 Jiri Olsa <[email protected]> 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 <[email protected]>

Thanks for the patch, I'm applying it now (and testing it).

Quoted 21 lines

---
 kernel/trace/trace_kprobe.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

kernel/trace/trace_kprobe.c +3 -1

index daa81571b22a..21f718472942 100644
@@ -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:

kernel/trace/trace_kprobe.c void destroy_local_trace_kprobe(struct trace_event_call *event_call) · line 1507

Quoted 5 lines

kernel/trace/trace_kprobe.c +2 -0

@@ -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

Quoted 3 lines

kernel/trace/trace_kprobe.c +0 -0

 	free_trace_kprobe(tk);
 }
 #endif /* CONFIG_PERF_EVENTS */

Steven Rostedt

maintainer

2018-07-10 13:04 UTC #3 lore raw

In reply to Jiri Olsa

Quoted 3 lines · On Mon, 9 Jul 2018 16:19:06 +0200 Jiri Olsa <[email protected]> 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

Quoted 32 lines

Fixes e12f03d7031a ("perf/core: Implement the 'perf_kprobe' PMU")

Signed-off-by Jiri Olsa <[email protected]>

---
 kernel/trace/trace_kprobe.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

kernel/trace/trace_kprobe.c +5 -1

index daa81571b22a..21f718472942 100644
@@ -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 */

Steven Rostedt

maintainer

2018-07-11 12:54 UTC #4 lore raw

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

Quoted 46 lines · On Tue, 10 Jul 2018 09:04:17 -0400 Steven Rostedt <[email protected]> wrote:

Quoted 3 lines · On Mon, 9 Jul 2018 16:19:06 +0200 Jiri Olsa <[email protected]> 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
Quoted 32 lines
Fixes e12f03d7031a ("perf/core: Implement the 'perf_kprobe' PMU")
Signed-off-by Jiri Olsa <[email protected]>
---
 kernel/trace/trace_kprobe.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
kernel/trace/trace_kprobe.c +5 -1
index daa81571b22a..21f718472942 100644
@@ -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

maintainer Acked-by

2018-07-11 13:26 UTC #5 lore raw

Quoted 6 lines · On Wed, 11 Jul 2018 08:54:29 -0400 Steven Rostedt <[email protected]> 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 <[email protected]>

Thanks!

Quoted 55 lines

Thanks!
-- Steve
Quoted 46 lines · On Tue, 10 Jul 2018 09:04:17 -0400 Steven Rostedt <[email protected]> wrote:
Quoted 3 lines · On Mon, 9 Jul 2018 16:19:06 +0200 Jiri Olsa <[email protected]> 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
Quoted 32 lines
Fixes e12f03d7031a ("perf/core: Implement the 'perf_kprobe' PMU")
Signed-off-by Jiri Olsa <[email protected]>
---
 kernel/trace/trace_kprobe.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
kernel/trace/trace_kprobe.c +5 -1
index daa81571b22a..21f718472942 100644
@@ -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 */  

Signature · 1 line

Masami Hiramatsu <[email protected]>

Steven Rostedt

maintainer

2018-07-11 13:29 UTC #6 lore raw

Quoted 6 lines · On Wed, 11 Jul 2018 22:26:37 +0900 Masami Hiramatsu <[email protected]> wrote:

Quoted 3 lines · On Wed, 11 Jul 2018 08:54:29 -0400 Steven Rostedt <[email protected]> wrote:
Masami,

Quoted 8 lines

Quoted 3 lines
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 <[email protected]>

Thanks Masami!

-- Steve