Commit c6c2401d authored by Steven Rostedt (Red Hat)'s avatar Steven Rostedt (Red Hat) Committed by Steven Rostedt

tracing/uprobes: Fail to unregister if probe event files are in use

Uprobes suffer the same problem that kprobes have. There's a race between
writing to the "enable" file and removing the probe. The probe checks for
it being in use and if it is not, goes about deleting the probe and the
event that represents it. But the problem with that is, after it checks
if it is in use it can be enabled, and the deletion of the event (access
to the probe) will fail, as it is in use. But the uprobe will still be
deleted. This is a problem as the event can reference the uprobe that
was deleted.

The fix is to remove the event first, and check to make sure the event
removal succeeds. Then it is safe to remove the probe.

When the event exists, either ftrace or perf can enable the probe and
prevent the event from being removed.

Link: http://lkml.kernel.org/r/20130704034038.991525256@goodmis.orgAcked-by: default avatarOleg Nesterov <oleg@redhat.com>
Signed-off-by: default avatarSteven Rostedt <rostedt@goodmis.org>
parent 40c32592
...@@ -70,7 +70,7 @@ struct trace_uprobe { ...@@ -70,7 +70,7 @@ struct trace_uprobe {
(sizeof(struct probe_arg) * (n))) (sizeof(struct probe_arg) * (n)))
static int register_uprobe_event(struct trace_uprobe *tu); static int register_uprobe_event(struct trace_uprobe *tu);
static void unregister_uprobe_event(struct trace_uprobe *tu); static int unregister_uprobe_event(struct trace_uprobe *tu);
static DEFINE_MUTEX(uprobe_lock); static DEFINE_MUTEX(uprobe_lock);
static LIST_HEAD(uprobe_list); static LIST_HEAD(uprobe_list);
...@@ -164,11 +164,17 @@ static struct trace_uprobe *find_probe_event(const char *event, const char *grou ...@@ -164,11 +164,17 @@ static struct trace_uprobe *find_probe_event(const char *event, const char *grou
} }
/* Unregister a trace_uprobe and probe_event: call with locking uprobe_lock */ /* Unregister a trace_uprobe and probe_event: call with locking uprobe_lock */
static void unregister_trace_uprobe(struct trace_uprobe *tu) static int unregister_trace_uprobe(struct trace_uprobe *tu)
{ {
int ret;
ret = unregister_uprobe_event(tu);
if (ret)
return ret;
list_del(&tu->list); list_del(&tu->list);
unregister_uprobe_event(tu);
free_trace_uprobe(tu); free_trace_uprobe(tu);
return 0;
} }
/* Register a trace_uprobe and probe_event */ /* Register a trace_uprobe and probe_event */
...@@ -181,9 +187,12 @@ static int register_trace_uprobe(struct trace_uprobe *tu) ...@@ -181,9 +187,12 @@ static int register_trace_uprobe(struct trace_uprobe *tu)
/* register as an event */ /* register as an event */
old_tp = find_probe_event(tu->call.name, tu->call.class->system); old_tp = find_probe_event(tu->call.name, tu->call.class->system);
if (old_tp) if (old_tp) {
/* delete old event */ /* delete old event */
unregister_trace_uprobe(old_tp); ret = unregister_trace_uprobe(old_tp);
if (ret)
goto end;
}
ret = register_uprobe_event(tu); ret = register_uprobe_event(tu);
if (ret) { if (ret) {
...@@ -256,6 +265,8 @@ static int create_trace_uprobe(int argc, char **argv) ...@@ -256,6 +265,8 @@ static int create_trace_uprobe(int argc, char **argv)
group = UPROBE_EVENT_SYSTEM; group = UPROBE_EVENT_SYSTEM;
if (is_delete) { if (is_delete) {
int ret;
if (!event) { if (!event) {
pr_info("Delete command needs an event name.\n"); pr_info("Delete command needs an event name.\n");
return -EINVAL; return -EINVAL;
...@@ -269,9 +280,9 @@ static int create_trace_uprobe(int argc, char **argv) ...@@ -269,9 +280,9 @@ static int create_trace_uprobe(int argc, char **argv)
return -ENOENT; return -ENOENT;
} }
/* delete an event */ /* delete an event */
unregister_trace_uprobe(tu); ret = unregister_trace_uprobe(tu);
mutex_unlock(&uprobe_lock); mutex_unlock(&uprobe_lock);
return 0; return ret;
} }
if (argc < 2) { if (argc < 2) {
...@@ -408,16 +419,20 @@ static int create_trace_uprobe(int argc, char **argv) ...@@ -408,16 +419,20 @@ static int create_trace_uprobe(int argc, char **argv)
return ret; return ret;
} }
static void cleanup_all_probes(void) static int cleanup_all_probes(void)
{ {
struct trace_uprobe *tu; struct trace_uprobe *tu;
int ret = 0;
mutex_lock(&uprobe_lock); mutex_lock(&uprobe_lock);
while (!list_empty(&uprobe_list)) { while (!list_empty(&uprobe_list)) {
tu = list_entry(uprobe_list.next, struct trace_uprobe, list); tu = list_entry(uprobe_list.next, struct trace_uprobe, list);
unregister_trace_uprobe(tu); ret = unregister_trace_uprobe(tu);
if (ret)
break;
} }
mutex_unlock(&uprobe_lock); mutex_unlock(&uprobe_lock);
return ret;
} }
/* Probes listing interfaces */ /* Probes listing interfaces */
...@@ -462,8 +477,13 @@ static const struct seq_operations probes_seq_op = { ...@@ -462,8 +477,13 @@ static const struct seq_operations probes_seq_op = {
static int probes_open(struct inode *inode, struct file *file) static int probes_open(struct inode *inode, struct file *file)
{ {
if ((file->f_mode & FMODE_WRITE) && (file->f_flags & O_TRUNC)) int ret;
cleanup_all_probes();
if ((file->f_mode & FMODE_WRITE) && (file->f_flags & O_TRUNC)) {
ret = cleanup_all_probes();
if (ret)
return ret;
}
return seq_open(file, &probes_seq_op); return seq_open(file, &probes_seq_op);
} }
...@@ -968,12 +988,17 @@ static int register_uprobe_event(struct trace_uprobe *tu) ...@@ -968,12 +988,17 @@ static int register_uprobe_event(struct trace_uprobe *tu)
return ret; return ret;
} }
static void unregister_uprobe_event(struct trace_uprobe *tu) static int unregister_uprobe_event(struct trace_uprobe *tu)
{ {
int ret;
/* tu->event is unregistered in trace_remove_event_call() */ /* tu->event is unregistered in trace_remove_event_call() */
trace_remove_event_call(&tu->call); ret = trace_remove_event_call(&tu->call);
if (ret)
return ret;
kfree(tu->call.print_fmt); kfree(tu->call.print_fmt);
tu->call.print_fmt = NULL; tu->call.print_fmt = NULL;
return 0;
} }
/* Make a trace interface for controling probe points */ /* Make a trace interface for controling probe points */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment