On Mon, Feb 08, 2016 at 11:50:23PM -0500, Jessica Yu wrote:
> Remove the ftrace module notifier in favor of directly calling
> ftrace_module_enable() and ftrace_release_mod() in the module loader.
> Hard-coding the function calls directly in the module loader removes
> dependence on the module notifier call chain and provides better
> visibility and control over what gets called when, which is important
> to kernel utilities such as livepatch.
>
> This fixes a notifier ordering issue in which the ftrace module notifier
> (and hence ftrace_module_enable()) for coming modules was being called
> after klp_module_notify(), which caused livepatch modules to initialize
> incorrectly. This patch removes dependence on the module notifier call
> chain in favor of hard coding the corresponding function calls in the
> module loader. This ensures that ftrace and livepatch code get called in
> the correct order on patch module load and unload.
>
> Fixes: 5156dca34a3e ("ftrace: Fix the race between ftrace and insmod")
> Signed-off-by: Jessica Yu <[email protected]>
> Reviewed-by: Steven Rostedt <[email protected]>
> Reviewed-by: Petr Mladek <[email protected]>
> Acked-by: Rusty Russell <[email protected]>
Reviewed-by: Josh Poimboeuf <[email protected]>
> ---
> include/linux/ftrace.h | 6 ++++--
> kernel/module.c | 5 +++++
> kernel/trace/ftrace.c | 36 +-----------------------------------
> 3 files changed, 10 insertions(+), 37 deletions(-)
>
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 81de712..c2b340e 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -603,6 +603,7 @@ extern int ftrace_arch_read_dyn_info(char *buf, int size);
>
> extern int skip_trace(unsigned long ip);
> extern void ftrace_module_init(struct module *mod);
> +extern void ftrace_module_enable(struct module *mod);
> extern void ftrace_release_mod(struct module *mod);
>
> extern void ftrace_disable_daemon(void);
> @@ -612,8 +613,9 @@ static inline int skip_trace(unsigned long ip) { return
> 0; }
> static inline int ftrace_force_update(void) { return 0; }
> static inline void ftrace_disable_daemon(void) { }
> 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(struct module *mod) { }
> +static inline void ftrace_module_enable(struct module *mod) { }
> +static inline void ftrace_release_mod(struct module *mod) { }
> static inline __init int register_ftrace_command(struct ftrace_func_command
> *cmd)
> {
> return -EINVAL;
> diff --git a/kernel/module.c b/kernel/module.c
> index 77f6791..0bd0077 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -984,6 +984,8 @@ SYSCALL_DEFINE2(delete_module, const char __user *,
> name_user,
> mod->exit();
> blocking_notifier_call_chain(&module_notify_list,
> MODULE_STATE_GOING, mod);
> + ftrace_release_mod(mod);
> +
> async_synchronize_full();
>
> /* Store the name of the last unloaded module for diagnostic purposes */
> @@ -3313,6 +3315,7 @@ fail:
> module_put(mod);
> blocking_notifier_call_chain(&module_notify_list,
> MODULE_STATE_GOING, mod);
> + ftrace_release_mod(mod);
> free_module(mod);
> wake_up_all(&module_wq);
> return ret;
> @@ -3369,6 +3372,8 @@ out_unlocked:
> static int prepare_coming_module(struct module *mod)
> {
>
> + ftrace_module_enable(mod);
> +
> blocking_notifier_call_chain(&module_notify_list,
> MODULE_STATE_COMING, mod);
> return 0;
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index eca592f..57a6eea 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -4961,7 +4961,7 @@ void ftrace_release_mod(struct module *mod)
> mutex_unlock(&ftrace_lock);
> }
>
> -static void ftrace_module_enable(struct module *mod)
> +void ftrace_module_enable(struct module *mod)
> {
> struct dyn_ftrace *rec;
> struct ftrace_page *pg;
> @@ -5038,38 +5038,8 @@ void ftrace_module_init(struct module *mod)
> ftrace_process_locs(mod, mod->ftrace_callsites,
> mod->ftrace_callsites + mod->num_ftrace_callsites);
> }
> -
> -static int ftrace_module_notify(struct notifier_block *self,
> - unsigned long val, void *data)
> -{
> - struct module *mod = data;
> -
> - 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(struct notifier_block *self,
> - unsigned long val, void *data)
> -{
> - return 0;
> -}
> #endif /* CONFIG_MODULES */
>
> -struct notifier_block ftrace_module_nb = {
> - .notifier_call = ftrace_module_notify,
> - .priority = INT_MIN, /* Run after anything that can remove kprobes */
> -};
> -
> void __init ftrace_init(void)
> {
> extern unsigned long __start_mcount_loc[];
> @@ -5098,10 +5068,6 @@ void __init ftrace_init(void)
> __start_mcount_loc,
> __stop_mcount_loc);
>
> - ret = register_module_notifier(&ftrace_module_nb);
> - if (ret)
> - pr_warning("Failed to register trace ftrace module exit
> notifier\n");
> -
> set_ftrace_early_filters();
>
> return;
> --
> 2.4.3
>
--
Josh
