Messages in this thread | | | Date | Tue, 15 Oct 2019 15:07:40 +0200 | From | Jessica Yu <> | Subject | Re: [PATCH v3 5/6] x86/ftrace: Use text_poke() |
| |
+++ Peter Zijlstra [11/10/19 14:59 +0200]: >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.
I'm having trouble visualizing what changes you're planning on making. I get that you want to squash ftrace_module_enable() into ftrace_module_init(), before module_enable_ro(). I'm fine with that as long as the races Steven described are addressed for affected arches. And livepatch should be OK as long as klp_module_coming() is always called *after* ftrace_module_enable(). But are you moving klp_module_coming() before the module_enable_* calls as well? And if so, why?
>The fact that it is executable; also the fact that you do it right after >we mark it ro. Flipping the memory protections back and forth is just >really poor everything. > >Once this ftrace thing is sorted, we'll change x86 to _refuse_ to make >executable (kernel) memory writable.
Not sure if relevant, but just thought I'd clarify: IIRC, klp_module_coming() is not poking the coming module, but it calls module_enable_ro() on itself (the livepatch module) so it can apply relocations and such on the new code, which lives inside the livepatch module, and it needs to possibly do this numerous times over the lifetime of the patch module for any coming module it is responsible for patching (i.e., call module_enable_ro() on the patch module, not necessarily the coming module). So I am not be sure why klp_module_coming() should be moved before complete_formation(). I hope I'm remembering the details correctly, livepatch folks feel free to chime in if I'm incorrect here.
| |