Messages in this thread | | | Date | Tue, 30 Jul 2013 17:15:42 +0900 | From | Masami Hiramatsu <> | Subject | Re: [RFC][PATCH 3/4] tracing/kprobes: Fail to unregister if probe event files are open |
| |
(2013/07/04 12:33), Steven Rostedt wrote: > From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org> > > When one of the event files is opened, we need to prevent them from > being removed. Modules do with with the module owner set (automated > from the VFS layer). The ftrace buffer instances have a ref count added > to the trace_array when the enabled file is opened (the others are not > that big of a deal, as they only reference the event calls which > still exist when an instance disappears). But kprobes and uprobes > do not have any protection. > > # cd /sys/kernel/debug/tracing > # echo 'p:sigprocmask sigprocmask' > kprobe_events || exit -1 > # enable_probe() { > sleep 10 > echo 1 > } > # file=events/kprobes/sigprocmask/enable > # enable_probe > $file & > > kprobe_events > > The above will corrupt the kprobe system, as the write to the enable > file will happen after the kprobe was deleted. > > Trying to create the probe again fails: > > # echo 'p:sigprocmask sigprocmask' > kprobe_events > # cat kprobe_events > p:kprobes/sigprocmask sigprocmask > # ls events/kprobes/ > enable filter > > Have the unregister probe fail when the event files are open, in use > are used by perf. >
Now, since the commit a232e270d makes sure that disable_trace_probe() waits until all running handlers are done, this patch has no problem.
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Thank you!
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org> > --- > kernel/trace/trace_kprobe.c | 21 +++++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-) > > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c > index 7ed6976..ffcaf42 100644 > --- a/kernel/trace/trace_kprobe.c > +++ b/kernel/trace/trace_kprobe.c > @@ -95,7 +95,7 @@ static __kprobes bool trace_probe_is_on_module(struct trace_probe *tp) > } > > static int register_probe_event(struct trace_probe *tp); > -static void unregister_probe_event(struct trace_probe *tp); > +static int unregister_probe_event(struct trace_probe *tp); > > static DEFINE_MUTEX(probe_lock); > static LIST_HEAD(probe_list); > @@ -340,9 +340,12 @@ static int unregister_trace_probe(struct trace_probe *tp) > if (trace_probe_is_enabled(tp)) > return -EBUSY; > > + /* Will fail if probe is being used by ftrace or perf */ > + if (unregister_probe_event(tp)) > + return -EBUSY; > + > __unregister_trace_probe(tp); > list_del(&tp->list); > - unregister_probe_event(tp); > > return 0; > } > @@ -621,7 +624,9 @@ static int release_all_trace_probes(void) > /* TODO: Use batch unregistration */ > while (!list_empty(&probe_list)) { > tp = list_entry(probe_list.next, struct trace_probe, list); > - unregister_trace_probe(tp); > + ret = unregister_trace_probe(tp); > + if (ret) > + goto end; > free_trace_probe(tp); > } > > @@ -1242,11 +1247,15 @@ static int register_probe_event(struct trace_probe *tp) > return ret; > } > > -static void unregister_probe_event(struct trace_probe *tp) > +static int unregister_probe_event(struct trace_probe *tp) > { > + int ret; > + > /* tp->event is unregistered in trace_remove_event_call() */ > - trace_remove_event_call(&tp->call); > - kfree(tp->call.print_fmt); > + ret = trace_remove_event_call(&tp->call); > + if (!ret) > + kfree(tp->call.print_fmt); > + return ret; > } > > /* Make a debugfs interface for controlling probe points */ >
-- Masami HIRAMATSU IT Management Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu.pt@hitachi.com
| |