(2013/07/04 12:33), Steven Rostedt wrote: > 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; > }
This may cause an unexpected access violation at kprobe handler because unregister_probe_event frees event_call/event_files and it will be accessed until kprobe is *completely* disabled. Actually disable_kprobe() doesn't ensure to finish the current running kprobe handlers. Thus, even if trace_probe_is_enabled() returns false, we must do synchronize_sched() for waiting, before unregister_probe_event(). OTOH, unregister_kprobe() waits for that. That's why I do __unregister_trace_probe(tp) first here. I think there are 2 solutions, one is adding a wait after disable_k*probe at disable_trace_probe(), another is adding a wait before unregister_probe_event() at unregister_trace_probe(). Thank you, -- Masami HIRAMATSU IT Management Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: [email protected] -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
