fix ftrace initialization issue when a module is loaded
cover fix ftrace initialization issue when a module is loaded
When a module is loaded, current ftrace initialization around the new module has some issues.
1) ftrace might race with insmod: Just after load_module calls ftrace_module_init to add ftrace records of the module, ftrace_run_update_code might jump in to change module codes. But load_module calls complete_formation=>set_section_ro_nx to put the module TEXT attribute to RO. Then, ftrace_run_update_code triggers ftrace_bug and fails.
2) complete_formation might fail and the module's ftrace records are not cleaned up.
This patchset fixes above issues.
PATCH 1/2 ftrace: fix the race between ftrace and insmod
From: Qiu Peiyang <[email protected]>
We hit ftrace_bug report when booting Android on a 64bit ATOM SOC chip. Basically, there is a race between insmod and ftrace_run_update_code.
After load_module=>ftrace_module_init, another thread jumps in to call ftrace_run_update_code=>ftrace_arch_code_modify_prepare
=>set_all_modules_text_rw, to change all modules
as RW. Since the new module is at MODULE_STATE_UNFORMED, the text attribute is not changed. Then, the 2nd thread goes ahead to change codes. However, load_module continues to call complete_formation=>set_section_ro_nx, then 2nd thread would fail when probing the module's TEXT.
The patch fixes it by using notifier to delay the enabling of ftrace records to the time when module is at state MODULE_STATE_COMING.
--- kernel/trace/ftrace.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)
kernel/trace/ftrace.c +14 -7
@@ -4993,7 +4993,6 @@ static void ftrace_init_module(struct module *mod,if (ftrace_disabled || start == end)return ;ftrace_process_locs (mod, start, end); -ftrace_module_enable (mod); }void ftrace_module_init (struct module *mod)
@@ -5003,26 +5002,34 @@ void ftrace_module_init(struct module *mod) mod->num_ftrace_callsites); } -static int ftrace_module_notify_exit (struct notifier_block *self, +static int ftrace_module_notify (struct notifier_block *self,unsigned long val,void *data) {struct module *mod = data; -if (val ==MODULE_STATE_GOING ) +switch (val) { +case MODULE_STATE_COMING : +ftrace_module_enable (mod); +break ; +case MODULE_STATE_GOING :ftrace_release_mod (mod); +break ; +default : +break ; + }return 0 ; }#else -static int ftrace_module_notify_exit (struct notifier_block *self, +static int ftrace_module_notify (struct notifier_block *self,unsigned long val,void *data) {return 0 ; }#endif /* CONFIG_MODULES */ -struct notifier_block ftrace_module_exit_nb = { - .notifier_call = ftrace_module_notify_exit, +struct notifier_block ftrace_module_nb = { + .notifier_call = ftrace_module_notify, .priority =INT_MIN ,/* Run after anything that can remove kprobes */ };
@@ -5054,7 +5061,7 @@ void __init ftrace_init(void) __start_mcount_loc, __stop_mcount_loc); - ret =register_module_notifier (&ftrace_module_exit_nb); + ret =register_module_notifier (&ftrace_module_nb);if (ret)pr_warning ("Failed to register trace ftrace module exit notifier\n" );
Signature · 1 line
1.9.1
PATCH 1/2 ftrace: fix the race between ftrace and insmod
In reply to Qiu, PeiyangXFrom: Qiu Peiyang <[email protected]>
We hit ftrace_bug report when booting Android on a 64bit ATOM SOC chip. Basically, there is a race between insmod and ftrace_run_update_code.
After load_module=>ftrace_module_init, another thread jumps in to call ftrace_run_update_code=>ftrace_arch_code_modify_prepare
=>set_all_modules_text_rw, to change all modules
as RW. Since the new module is at MODULE_STATE_UNFORMED, the text attribute is not changed. Then, the 2nd thread goes ahead to change codes. However, load_module continues to call complete_formation=>set_section_ro_nx, then 2nd thread would fail when probing the module's TEXT.
The patch fixes it by using notifier to delay the enabling of ftrace records to the time when module is at state MODULE_STATE_COMING.
--- kernel/trace/ftrace.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)
kernel/trace/ftrace.c +13 -3
@@ -4993,7 +4993,6 @@ static void ftrace_init_module(struct module *mod,if (ftrace_disabled || start == end)return ;ftrace_process_locs (mod, start, end); -ftrace_module_enable (mod); }void ftrace_module_init (struct module *mod)
@@ -5003,26 +5002,34 @@ void ftrace_module_init(struct module *mod) mod->num_ftrace_callsites); } -static int ftrace_module_notify_exit(struct notifier_block *self, +static int ftrace_module_notify (struct notifier_block *self, unsigned long val, void *data) {struct module *mod = data; - if (val == MODULE_STATE_GOING) +switch (val) { +case MODULE_STATE_COMING : +ftrace_module_enable (mod); +break ; +case MODULE_STATE_GOING :ftrace_release_mod (mod); +break ; +default : +break ; + }return 0 ; }#else -static int ftrace_module_notify_exit (struct notifier_block *self, +static int ftrace_module_notify (struct notifier_block *self,unsigned long val,void *data) {return 0 ; }#endif /* CONFIG_MODULES */ -struct notifier_block ftrace_module_exit_nb = { - .notifier_call = ftrace_module_notify_exit, +struct notifier_block ftrace_module_nb = { + .notifier_call = ftrace_module_notify, .priority = INT_MIN, /* Run after anything that can remove kprobes */ }; @@ -5054,7 +5061,7 @@ void __init ftrace_init(void) __start_mcount_loc,
__stop_mcount_loc);
- ret = register_module_notifier(&ftrace_module_exit_nb);
+ ret = register_module_notifier(&ftrace_module_nb);
if (ret)
pr_warning("Failed to register trace ftrace module exit notifier\n");
-- 1.9.1
PATCH 2/2 module: deal with the failure of complete_formation
In reply to Qiu, PeiyangXFrom: 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.
--- kernel/module.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
kernel/module.c +5 -4
@@ -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); + +/* Ftrace init must be called in the MODULE_STATE_UNFORMED state */ +ftrace_module_init (mod); +/* Set RO and NX regions for core */ set_section_ro_nx(mod->module_core, mod->core_text_size,
@@ -3388,7 +3393,6 @@ static int complete_formation(struct module *mod, struct load_info *info)/* Mark state as coming so strong_try_module_get() ignores us, * but kallsyms etc. can see us. */ mod->state =MODULE_STATE_COMING ; -mutex_unlock (&module_mutex);blocking_notifier_call_chain (&module_notify_list,MODULE_STATE_COMING , mod);
@@ -3505,9 +3509,6 @@ static int load_module(struct load_info *info, const char __user *uargs,dynamic_debug_setup (info->debug, info->num_debug); -/* Ftrace init must be called in the MODULE_STATE_UNFORMED state */ -ftrace_module_init (mod); -/* Finally it's fully formed, ready to start executing. */ err =complete_formation (mod, info);if (err)
Signature · 1 line
1.9.1
Steven Rostedt
maintainerRe: [PATCH 2/2] module: deal with the failure of complete_formation
Quoted 21 lines · PeiyangX
From: Qiu Peiyang <[email protected]>
complete_formationmight fail. kernel need clean up ftrace records of the module.The patch fixes it by tuning the operation sequence in
complete_formation. Aftercomplete_formationchecksverify_export_symbols, callftrace_module_initto 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(-)kernel/module.c +0
-0
kernel/module.c static int complete_formation(struct module *mod, struct load_info *info) · line 3377
Quoted 5 lines
kernel/module.c +2
-0@@ -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(+)
include/linux/ftrace.h +2 -0
@@ -602,6 +602,7 @@ extern int ftrace_arch_read_dyn_info(chaextern 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_daemonstatic 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 ;
kernel/module.c +1 -0
@@ -3571,6 +3571,7 @@ static int load_module(struct load_infosynchronize_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);
kernel/trace/ftrace.c +5 -0
@@ -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) {
Re: [PATCH 2/2] module: deal with the failure of complete_formation
Quoted 32 lines · Steven Rostedt
Quoted 21 lines · PeiyangX
From: Qiu Peiyang <[email protected]>
complete_formationmight fail. kernel need clean up ftrace records of the module.The patch fixes it by tuning the operation sequence in
complete_formation. Aftercomplete_formationchecksverify_export_symbols, callftrace_module_initto 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(-)kernel/module.c +0
-0kernel/module.c static int complete_formation(struct module *mod, struct load_info *info) · line 3377
Quoted 5 lines
kernel/module.c +2
-0@@ -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 modifymod->state.
Quoted 4 lines
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 callingdo_init_module(), as if that happens it does not call the module going notifiers.
It's a good idea although ftrace_module_init_fail might be complicated.
Thanks,
Yanmin
Steven Rostedt
maintainerRe: [PATCH 2/2] module: deal with the failure of complete_formation
Quoted 1 line · Yanmin
It's a good idea although
ftrace_module_init_failmight be complicated.
How so? I posted the patch. All it does is to call ftrace_release_mod(), which will only remove pages that are included with the module. If the pages were not added yet, nothing is done.
-- Steve
Re: [PATCH 2/2] module: deal with the failure of complete_formation
Quoted 8 lines · Steven Rostedt
Quoted 1 line · Yanmin
It's a good idea although
ftrace_module_init_failmight be complicated.How so? I posted the patch. All it does is to call
ftrace_release_mod(), which will only remove pages that are included with the module. If the pages were not added yet, nothing is done.
You are right. ftrace_release_mod can be used.
Steven Rostedt
maintainerRe: [PATCH 2/2] module: deal with the failure of complete_formation
Quoted 11 lines · Yanmin
Quoted 8 lines · Steven Rostedt
Quoted 1 line · Yanmin
It's a good idea although
ftrace_module_init_failmight be complicated.How so? I posted the patch. All it does is to call
ftrace_release_mod(), which will only remove pages that are included with the module. If the pages were not added yet, nothing is done.You are right.
ftrace_release_modcan be used.
I was going to call that directly, but for clarity, documentation and robustness I added the new call. Hmm, but on second thought, I noticed that it is already shown globally. I think I'll change that back to call ftrace_release_mod() directly.
-- Steve