| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20160217023501.GF3018@treble.redhat.com>
Date: Tue, 16 Feb 2016 20:35:01 -0600
From: Josh Poimboeuf <jpoimboe@...hat.com>
To: Jessica Yu <jeyu@...hat.com>
Cc: Seth Jennings <sjenning@...hat.com>,
Jiri Kosina <jikos@...nel.org>,
Vojtech Pavlik <vojtech@...e.com>,
Miroslav Benes <mbenes@...e.cz>,
Petr Mladek <pmladek@...e.com>,
Rusty Russell <rusty@...tcorp.com.au>,
Steven Rostedt <rostedt@...dmis.org>,
Ingo Molnar <mingo@...hat.com>, live-patching@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5] ftrace/module: remove ftrace module notifier
On Tue, Feb 16, 2016 at 05:32:33PM -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 <jeyu@...hat.com>
> Reviewed-by: Steven Rostedt <rostedt@...dmis.org>
> Reviewed-by: Petr Mladek <pmladek@...e.cz>
> Acked-by: Rusty Russell <rusty@...tcorp.com.au>
Reviewed-by: Josh Poimboeuf <jpoimboe@...hat.com>
> ---
> Patch based on linux-next-20160216.
>
> v5:
> - Make the ftrace notifier patch standalone, without the module.c
> cleanups. This is basically reverting back to patch 1/2 from v2, found
> here: http://article.gmane.org/gmane.linux.kernel/2141648
>
> The original patchset (v4) this patch was a part of can be found here:
> https://lkml.kernel.org/g/1454993424-31031-1-git-send-email-jeyu@redhat.com
>
> include/linux/ftrace.h | 6 ++++--
> kernel/module.c | 4 ++++
> kernel/trace/ftrace.c | 36 +-----------------------------------
> 3 files changed, 9 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 9537da3..794ebe8 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;
> @@ -3389,6 +3392,7 @@ static int complete_formation(struct module *mod, struct load_info *info)
> mod->state = MODULE_STATE_COMING;
> mutex_unlock(&module_mutex);
>
> + 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
Powered by blists - more mailing lists