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. Miroslav
