From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752463AbcAFBBV (ORCPT ); Tue, 5 Jan 2016 20:01:21 -0500 Received: from smtprelay0014.hostedemail.com ([216.40.44.14]:59502 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751752AbcAFBBT (ORCPT ); Tue, 5 Jan 2016 20:01:19 -0500 X-Session-Marker: 726F737465647440676F6F646D69732E6F7267 X-Spam-Summary: 2,0,0,,d41d8cd98f00b204,rostedt@goodmis.org,:::::::::,RULES_HIT:41:355:379:541:599:800:960:966:973:988:989:1260:1277:1311:1313:1314:1345:1359:1437:1515:1516:1518:1534:1543:1593:1594:1711:1730:1747:1777:1792:2196:2199:2393:2553:2559:2562:2907:3138:3139:3140:3141:3142:3354:3622:3865:3866:3867:3870:3871:3872:3873:3874:4321:4385:4605:5007:6261:7576:7875:7903:8660:9010:9121:10004:10400:10848:10967:11026:11232:11658:11914:12043:12291:12296:12438:12517:12519:12555:12679:12683:12740:13142:13148:13230:13972:14096:14097:14659:21080:30045:30054:30064:30075:30090:30091,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:fn,MSBL:0,DNSBL:none,Custom_rules:0:0:0,LFtime:1,LUA_SUMMARY:none X-HE-Tag: drop33_af9727416839 X-Filterd-Recvd-Size: 4669 Date: Tue, 5 Jan 2016 20:01:14 -0500 From: Steven Rostedt To: "Qiu, PeiyangX" Cc: Ingo Molnar , Rusty Russell , linux-kernel@vger.kernel.org, yanmin_zhang@linux.intel.com Subject: Re: [PATCH 2/2] module: deal with the failure of complete_formation Message-ID: <20160105200114.59fbf832@gandalf.local.home> In-Reply-To: <567CEA31.1070507@intel.com> References: <567CE0A7.80605@intel.com> <567CEA31.1070507@intel.com> X-Mailer: Claws Mail 3.13.0 (GTK+ 2.24.28; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 25 Dec 2015 15:03:13 +0800 "Qiu, PeiyangX" wrote: > From: Qiu Peiyang > > 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 > Signed-off-by: Zhang Yanmin > --- > 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) {