Messages in this thread Patch in this message | | | Date | Wed, 26 Aug 2009 10:37:11 -0400 (EDT) | From | Steven Rostedt <> | Subject | Re: [PATCH] tracing/profile: Fix profile_disable vs module_unload |
| |
On Wed, 26 Aug 2009, Li Zefan wrote:
> Peter Zijlstra wrote: > > On Wed, 2009-08-26 at 08:46 +0200, Peter Zijlstra wrote: > > > >> Aahh, I see the bug, its only ftrace that knows about the module, not > >> tracepoints themselves, _that_ needs fixing. > > > > You could possibly do something like: > > > > struct module *tp_mod = __module_address(&some_tp_symbol); > > struct module *cb_mod = __module_text_address(func); > > > > if (tp_mod && tp_mod != cb_mod) { > > ret = try_get_module(tp_mod); > > if (ret) > > goto fail; > > } > > > > in register_trace_##name() or thereabout. > > > > Actually I tried it, but it didn't work. As I said, You can't find > any tp symbol when registering tp callback. The same example again: > > In module bar, we have register_trace_foo() > In module foo, we have DEFINE_TRACE(foo) and trace_foo(). > > bar doesn't know any symbol of foo, so it can't bump foo's refcnt, > > *Note: you can load module bar without loading module foo*
WTF!!!!
We can register a trace point that is defined in another module without having that module?? How is that possible? That looks totally busted, and that is not a case that I think we need to worry about, except to prevent it from ever happening.
As for ref counts, would something like this work?
(untested)
-- Steve
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h index 0341f2e..055275b 100644 --- a/include/linux/tracepoint.h +++ b/include/linux/tracepoint.h @@ -109,8 +109,9 @@ struct tracepoint { #define EXPORT_TRACEPOINT_SYMBOL(name) \ EXPORT_SYMBOL(__tracepoint_##name) -extern void tracepoint_update_probe_range(struct tracepoint *begin, - struct tracepoint *end); +extern void tracepoint_update_probe_range(struct module *, + struct tracepoint *begin, + struct tracepoint *end); #else /* !CONFIG_TRACEPOINTS */ #define DECLARE_TRACE_WITH_CALLBACK(name, proto, args, reg, unreg) \ diff --git a/kernel/module.c b/kernel/module.c index b182143..a8e69fa 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -2974,7 +2974,7 @@ void module_update_tracepoints(void) mutex_lock(&module_mutex); list_for_each_entry(mod, &modules, list) if (!mod->taints) - tracepoint_update_probe_range(mod->tracepoints, + tracepoint_update_probe_range(mod, mod->tracepoints, mod->tracepoints + mod->num_tracepoints); mutex_unlock(&module_mutex); } diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c index 06f165a..b150255 100644 --- a/kernel/tracepoint.c +++ b/kernel/tracepoint.c @@ -274,7 +274,8 @@ static void disable_tracepoint(struct tracepoint *elem) * Updates the probe callback corresponding to a range of tracepoints. */ void -tracepoint_update_probe_range(struct tracepoint *begin, struct tracepoint *end) +tracepoint_update_probe_range(struct module *mod, + struct tracepoint *begin, struct tracepoint *end) { struct tracepoint *iter; struct tracepoint_entry *mark_entry; @@ -286,9 +287,15 @@ tracepoint_update_probe_range(struct tracepoint *begin, struct tracepoint *end) for (iter = begin; iter < end; iter++) { mark_entry = get_tracepoint(iter->name); if (mark_entry) { + if (mod) { + if (!try_module_get(mod)) + continue; + } set_tracepoint(&mark_entry, iter, !!mark_entry->refcount); } else { + if (mod) + module_put(mod); disable_tracepoint(iter); } } @@ -301,7 +308,7 @@ tracepoint_update_probe_range(struct tracepoint *begin, struct tracepoint *end) static void tracepoint_update_probes(void) { /* Core kernel tracepoints */ - tracepoint_update_probe_range(__start___tracepoints, + tracepoint_update_probe_range(NULL, __start___tracepoints, __stop___tracepoints); /* tracepoints in modules. */ module_update_tracepoints(); @@ -556,7 +563,7 @@ int tracepoint_module_notify(struct notifier_block *self, switch (val) { case MODULE_STATE_COMING: case MODULE_STATE_GOING: - tracepoint_update_probe_range(mod->tracepoints, + tracepoint_update_probe_range(mod, mod->tracepoints, mod->tracepoints + mod->num_tracepoints); break; }
| |