Messages in this thread Patch in this message | | | Date | Tue, 15 Oct 2019 18:09:13 +0200 | From | Jessica Yu <> | Subject | Re: [PATCH v3 5/6] x86/ftrace: Use text_poke() |
| |
+++ Steven Rostedt [15/10/19 09:28 -0400]: >On Fri, 11 Oct 2019 14:59:03 +0200 >Peter Zijlstra <peterz@infradead.org> wrote: > >> On Thu, Oct 10, 2019 at 07:28:19PM +0200, Peter Zijlstra wrote: >> >> > Really the best solution is to move all the poking into >> > ftrace_module_init(), before we mark it RO+X. That's what I'm going to >> > do for jump_label and static_call as well, I just need to add that extra >> > notifier callback. >> >> OK, so I started writing that patch... or rather, I wrote the patch and >> started on the Changelog when I ran into trouble describing why we need >> it. >> >> That is, I'm struggling to explain why we cannot flip >> prepare_coming_module() and complete_formation(). >> >> Yes, it breaks ftrace, but I'm thinking that is all it breaks. So let me >> see if we can cure that. > >You are mainly worried about making text that is executable into >read-write again. What if we kept my one patch that just changed the >module in ftrace_module_enable() from read-only to read-write, but >before we ever set it executable. > >Jessica, would this patch break anything? > >It moves the setting of the module execution to after calling both >ftrace_module_enable() and klp_module_coming(). > >This would make it possible to do the module code and still keep with >the no executable code becoming writable. > >-- Steve > >diff --git a/kernel/module.c b/kernel/module.c >index ff2d7359a418..6e2fd40a6ed9 100644 >--- a/kernel/module.c >+++ b/kernel/module.c >@@ -3728,7 +3728,6 @@ static int complete_formation(struct module *mod, struct load_info *info) > > module_enable_ro(mod, false); > module_enable_nx(mod); >- module_enable_x(mod); > > /* Mark state as coming so strong_try_module_get() ignores us, > * but kallsyms etc. can see us. */ >@@ -3751,6 +3750,11 @@ static int prepare_coming_module(struct module *mod) > if (err) > return err; > >+ /* Make module executable after ftrace is enabled */ >+ mutex_lock(&module_mutex); >+ module_enable_x(mod); >+ mutex_unlock(&module_mutex); >+ > blocking_notifier_call_chain(&module_notify_list, > MODULE_STATE_COMING, mod); > return 0;
As long as we enable x before parse_args(), which this patch does, then I don't think this change would break anything.
| |