kprobes: can cause perf to crash the system
Subsystems (2)
Steven Rostedt
maintainerIngo,
I tested and verified that the commends that Masami has shown will crash the system. These patches do fix the problem.
I also verified that the same commands will crash 2.6.39 and have included stable in the Cc of the commits.
-- Steve
Please pull the latest tip/perf/urgent tree, which can be found at:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
tip/perf/urgent
Head SHA1: 44a56040a0037a845d5fa218dffde464579f0cab
Masami Hiramatsu (2):
ftrace/kprobes: Fix not to delete probes if in use perf probe: Fix to show correct error string
----
kernel/trace/trace_kprobe.c | 58 ++++++++++++++++++++++++++++++++++------ tools/perf/util/probe-event.c | 4 ++- 2 files changed, 52 insertions(+), 10 deletions(-)
Steven Rostedt
maintainerPATCH 1/2 ftrace/kprobes: Fix not to delete probes if in use
From: Masami Hiramatsu <[email protected]>
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) ]
--- kernel/trace/trace_kprobe.c | 58 ++++++++++++++++++++++++++++++++++++------ 1 files changed, 49 insertions(+), 9 deletions(-)
kernel/trace/trace_kprobe.c +49 -9
@@ -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" );
Signature · 1 line
1.7.6.3
Steven Rostedt
maintainerPATCH 2/2 perf probe: Fix to show correct error string
In reply to Steven RostedtFrom: Masami Hiramatsu <[email protected]>
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.
--- tools/perf/util/probe-event.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)
tools/perf/util/probe-event.c +3 -1
@@ -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 ;
Signature · 1 line
1.7.6.3
Quoted 19 lines · Steven Rostedt
Ingo,
I tested and verified that the commends that Masami has shown will crash the system. These patches do fix the problem.
I also verified that the same commands will crash 2.6.39 and have included stable in the Cc of the commits.
-- Steve
Please pull the latest tip/perf/urgent tree, which can be found at:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
tip/perf/urgentHead SHA1:
44a56040a0037a845d5fa218dffde464579f0cab
Quoted 8 lines
Masami Hiramatsu (2):
ftrace/kprobes: Fix not to delete probes if in use perf probe: Fix to show correct error string----
kernel/trace/trace_kprobe.c | 58 ++++++++++++++++++++++++++++++++++------ tools/perf/util/probe-event.c | 4 ++- 2 files changed, 52 insertions(+), 10 deletions(-)
Pulled, thanks a lot guys!
Ingo