* [PATCH 1/2] ftrace/kprobes: Fix not to delete probes if in use
2011-10-10 19:17 [PATCH 0/2] [GIT PULL][v3.1] kprobes: can cause perf to crash the system Steven Rostedt
@ 2011-10-10 19:17 ` Steven Rostedt
2011-10-10 19:17 ` [PATCH 2/2] perf probe: Fix to show correct error string Steven Rostedt
2011-10-12 7:08 ` [PATCH 0/2] [GIT PULL][v3.1] kprobes: can cause perf to crash the system Ingo Molnar
2 siblings, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2011-10-10 19:17 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Frederic Weisbecker,
Masami Hiramatsu, Arnaldo Carvalho de Melo, Frank Ch. Eigler,
stable
[-- Attachment #1: Type: text/plain, Size: 5047 bytes --]
From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Fix kprobe-tracer not to delete a probe if the probe is in use.
In that case, delete operation will return -EBUSY.
This bug can cause a kernel panic if enabled probes are deleted
during perf record.
(Add some probes on functions)
sh-4.2# perf probe --del probe:\*
sh-4.2# exit
(kernel panic)
This is originally reported on the fedora bugzilla:
https://bugzilla.redhat.com/show_bug.cgi?id=742383
I've also checked that this problem doesn't happen on
tracepoints when module removing because perf event
locks target module.
$ sudo ./perf record -e xfs:\* -aR sh
sh-4.2# rmmod xfs
ERROR: Module xfs is in use
sh-4.2# exit
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.203 MB perf.data (~8862 samples) ]
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Frank Ch. Eigler <fche@redhat.com>
Cc: stable@kernel.org
Link: http://lkml.kernel.org/r/20111004104438.14591.6553.stgit@fedora15
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
kernel/trace/trace_kprobe.c | 58 ++++++++++++++++++++++++++++++++++++------
1 files changed, 49 insertions(+), 9 deletions(-)
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 5fb3697..00d527c 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -836,11 +836,17 @@ static void __unregister_trace_probe(struct trace_probe *tp)
}
/* Unregister a trace_probe and probe_event: call with locking probe_lock */
-static void unregister_trace_probe(struct trace_probe *tp)
+static int unregister_trace_probe(struct trace_probe *tp)
{
+ /* Enabled event can not be unregistered */
+ if (trace_probe_is_enabled(tp))
+ return -EBUSY;
+
__unregister_trace_probe(tp);
list_del(&tp->list);
unregister_probe_event(tp);
+
+ return 0;
}
/* Register a trace_probe and probe_event */
@@ -854,7 +860,9 @@ static int register_trace_probe(struct trace_probe *tp)
/* Delete old (same name) event if exist */
old_tp = find_trace_probe(tp->call.name, tp->call.class->system);
if (old_tp) {
- unregister_trace_probe(old_tp);
+ ret = unregister_trace_probe(old_tp);
+ if (ret < 0)
+ goto end;
free_trace_probe(old_tp);
}
@@ -892,6 +900,7 @@ static int trace_probe_module_callback(struct notifier_block *nb,
mutex_lock(&probe_lock);
list_for_each_entry(tp, &probe_list, list) {
if (trace_probe_within_module(tp, mod)) {
+ /* Don't need to check busy - this should have gone. */
__unregister_trace_probe(tp);
ret = __register_trace_probe(tp);
if (ret)
@@ -1205,10 +1214,11 @@ static int create_trace_probe(int argc, char **argv)
return -ENOENT;
}
/* delete an event */
- unregister_trace_probe(tp);
- free_trace_probe(tp);
+ ret = unregister_trace_probe(tp);
+ if (ret == 0)
+ free_trace_probe(tp);
mutex_unlock(&probe_lock);
- return 0;
+ return ret;
}
if (argc < 2) {
@@ -1317,18 +1327,29 @@ error:
return ret;
}
-static void release_all_trace_probes(void)
+static int release_all_trace_probes(void)
{
struct trace_probe *tp;
+ int ret = 0;
mutex_lock(&probe_lock);
+ /* Ensure no probe is in use. */
+ list_for_each_entry(tp, &probe_list, list)
+ if (trace_probe_is_enabled(tp)) {
+ ret = -EBUSY;
+ goto end;
+ }
/* TODO: Use batch unregistration */
while (!list_empty(&probe_list)) {
tp = list_entry(probe_list.next, struct trace_probe, list);
unregister_trace_probe(tp);
free_trace_probe(tp);
}
+
+end:
mutex_unlock(&probe_lock);
+
+ return ret;
}
/* Probes listing interfaces */
@@ -1380,9 +1401,13 @@ static const struct seq_operations probes_seq_op = {
static int probes_open(struct inode *inode, struct file *file)
{
- if ((file->f_mode & FMODE_WRITE) &&
- (file->f_flags & O_TRUNC))
- release_all_trace_probes();
+ int ret;
+
+ if ((file->f_mode & FMODE_WRITE) && (file->f_flags & O_TRUNC)) {
+ ret = release_all_trace_probes();
+ if (ret < 0)
+ return ret;
+ }
return seq_open(file, &probes_seq_op);
}
@@ -2055,6 +2080,21 @@ static __init int kprobe_trace_self_tests_init(void)
ret = target(1, 2, 3, 4, 5, 6);
+ /* Disable trace points before removing it */
+ tp = find_trace_probe("testprobe", KPROBE_EVENT_SYSTEM);
+ if (WARN_ON_ONCE(tp == NULL)) {
+ pr_warning("error on getting test probe.\n");
+ warn++;
+ } else
+ disable_trace_probe(tp, TP_FLAG_TRACE);
+
+ tp = find_trace_probe("testprobe2", KPROBE_EVENT_SYSTEM);
+ if (WARN_ON_ONCE(tp == NULL)) {
+ pr_warning("error on getting 2nd test probe.\n");
+ warn++;
+ } else
+ disable_trace_probe(tp, TP_FLAG_TRACE);
+
ret = command_trace_probe("-:testprobe");
if (WARN_ON_ONCE(ret)) {
pr_warning("error on deleting a probe.\n");
--
1.7.6.3
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 2/2] perf probe: Fix to show correct error string
2011-10-10 19:17 [PATCH 0/2] [GIT PULL][v3.1] kprobes: can cause perf to crash the system Steven Rostedt
2011-10-10 19:17 ` [PATCH 1/2] ftrace/kprobes: Fix not to delete probes if in use Steven Rostedt
@ 2011-10-10 19:17 ` Steven Rostedt
2011-10-12 7:08 ` [PATCH 0/2] [GIT PULL][v3.1] kprobes: can cause perf to crash the system Ingo Molnar
2 siblings, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2011-10-10 19:17 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Frederic Weisbecker,
Masami Hiramatsu, Peter Zijlstra, Arnaldo Carvalho de Melo,
Paul Mackerras, stable
[-- Attachment #1: Type: text/plain, Size: 1198 bytes --]
From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Fix perf probe to show correct error string when it
fails to delete an event. The write(2) returns -1
if failed, and errno stores real error number.
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: stable@kernel.org
Link: http://lkml.kernel.org/r/20111004104504.14591.41266.stgit@fedora15
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
tools/perf/util/probe-event.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 1c7bfa5..eb25900 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1956,8 +1956,10 @@ static int __del_trace_probe_event(int fd, struct str_node *ent)
pr_debug("Writing event: %s\n", buf);
ret = write(fd, buf, strlen(buf));
- if (ret < 0)
+ if (ret < 0) {
+ ret = -errno;
goto error;
+ }
printf("Remove event: %s\n", ent->s);
return 0;
--
1.7.6.3
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply related [flat|nested] 4+ messages in thread