On Fri, 25 Dec 2015 15:03:13 +0800 "Qiu, PeiyangX" <[email protected]> wrote:
> From: Qiu Peiyang <[email protected]> > > complete_formation might fail. kernel need clean up > ftrace records of the module. > > The patch fixes it by tuning the operation sequence in > complete_formation. After complete_formation checks > verify_export_symbols, call ftrace_module_init to init > ftrace records. > > Signed-off-by: Qiu Peiyang <[email protected]> > Signed-off-by: Zhang Yanmin <[email protected]> > --- > kernel/module.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/kernel/module.c b/kernel/module.c > index 8f051a1..0a67c4e 100644 > --- a/kernel/module.c > +++ b/kernel/module.c > @@ -3373,6 +3373,11 @@ static int complete_formation(struct module *mod, > struct load_info *info) > /* This relies on module_mutex for list integrity. */ > module_bug_finalize(info->hdr, info->sechdrs, mod); > > + mutex_unlock(&module_mutex); > + First of all, this is buggy. You can't just move the locking of module_mutex. That is needed to modify mod->state. Second of all, you are solving this wrong. I guess we should add ftrace_module_init_fail() function to cover the cases where the module can fail before calling do_init_module(), as if that happens it does not call the module going notifiers. I'll be sending this to stable too. -- Steve --- include/linux/ftrace.h | 2 ++ kernel/module.c | 1 + kernel/trace/ftrace.c | 5 +++++ 3 files changed, 8 insertions(+) Index: linux-trace.git/include/linux/ftrace.h =================================================================== --- linux-trace.git.orig/include/linux/ftrace.h 2016-01-05 18:00:11.686173290 -0500 +++ linux-trace.git/include/linux/ftrace.h 2016-01-05 18:00:47.815631131 -0500 @@ -602,6 +602,7 @@ extern int ftrace_arch_read_dyn_info(cha extern int skip_trace(unsigned long ip); extern void ftrace_module_init(struct module *mod); +extern void ftrace_module_init_fail(struct module *mod); extern void ftrace_disable_daemon(void); extern void ftrace_enable_daemon(void); @@ -612,6 +613,7 @@ static inline void ftrace_disable_daemon static inline void ftrace_enable_daemon(void) { } static inline void ftrace_release_mod(struct module *mod) {} static inline void ftrace_module_init(struct module *mod) {} +static inline void ftrace_module_init_fail(struct module *mod) {} static inline __init int register_ftrace_command(struct ftrace_func_command *cmd) { return -EINVAL; Index: linux-trace.git/kernel/module.c =================================================================== --- linux-trace.git.orig/kernel/module.c 2016-01-05 18:00:11.687173275 -0500 +++ linux-trace.git/kernel/module.c 2016-01-05 18:00:47.816631117 -0500 @@ -3571,6 +3571,7 @@ static int load_module(struct load_info synchronize_sched(); mutex_unlock(&module_mutex); free_module: + ftrace_module_init_fail(mod); /* Free lock-classes; relies on the preceding sync_rcu() */ lockdep_free_key_range(mod->module_core, mod->core_size); Index: linux-trace.git/kernel/trace/ftrace.c =================================================================== --- linux-trace.git.orig/kernel/trace/ftrace.c 2016-01-05 18:00:47.817631101 -0500 +++ linux-trace.git/kernel/trace/ftrace.c 2016-01-05 18:01:10.418291635 -0500 @@ -4989,6 +4989,11 @@ void ftrace_module_init(struct module *m mod->ftrace_callsites + mod->num_ftrace_callsites); } +void ftrace_module_init_fail(struct module *mod) +{ + ftrace_release_mod(mod); +} + static int ftrace_module_notify_exit(struct notifier_block *self, unsigned long val, void *data) { -- 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/

