Messages in this thread | | | Date | Fri, 17 Apr 2020 10:50:50 +0200 | From | Jessica Yu <> | Subject | Re: [PATCH 0/7] livepatch,module: Remove .klp.arch and module_disable_ro() |
| |
+++ Miroslav Benes [17/04/20 10:27 +0200]: >On Thu, 16 Apr 2020, Josh Poimboeuf wrote: > >> On Thu, Apr 16, 2020 at 05:31:31PM +0200, Jessica Yu wrote: >> > > But I still not a fan of the fact that COMING has two different >> > > "states". For example, after your patch, when apply_relocate_add() is >> > > called from klp_module_coming(), it can use memcpy(), but when called >> > > from klp module init() it has to use text poke. But both are COMING so >> > > there's no way to look at the module state to know which can be used. >> > >> > This is a good observation, thanks for bringing it up. I agree that we >> > should strive to be consistent with what the module states mean. In my >> > head, I think it is easiest to assume/establish the following meanings >> > for each module state: >> > >> > MODULE_STATE_UNFORMED - no protections. relocations, alternatives, >> > ftrace module initialization, etc. any other text modifications are >> > in the process of being applied. Direct writes are permissible. >> > >> > MODULE_STATE_COMING - module fully formed, text modifications are >> > done, protections applied, module is ready to execute init or is >> > executing init. >> > >> > I wonder if we could enforce the meaning of these two states more >> > consistently without needing to add another module state. >> > >> > Regarding Peter's patches, with the set_all_modules_text_*() api gone, >> > and ftrace reliance on MODULE_STATE_COMING gone (I think?), is there >> > anything preventing ftrace_module_init+enable from being called >> > earlier (i.e., before complete_formation()) while the module is >> > unformed? Then you don't have to move module_enable_ro/nx later and we >> > keep the MODULE_STATE_COMING semantics. And if we're enforcing the >> > above module state meanings, I would also be OK with moving jump_label >> > and static_call out of the coming notifier chain and making them >> > explicit calls while the module is still writable. >> > >> > Sorry in advance if I missed anything above, I'm still trying to wrap >> > my head around which callers need what module state and what module >> > permissions :/ >> >> Sounds reasonable to me... >> >> BTW, instead of hard-coding the jump-label/static-call/ftrace calls, we >> could instead call notifiers with MODULE_STATE_UNFORMED. > >That was exactly what I was thinking about too while reading Jessica's >email. Since (hopefully all if I remember correctly. I checked only >random subset now) existing module notifiers check module state, >it should not be a problem.
Agreed, especially with the growing number of callers now that want to access the module while it is still writable, it seems reasonable. IIRC, the module notifiers I looked at too checked the module state value appropriately, so I think we are fine as well (thanks for checking!)
Jessica
| |