Re: [PATCH v2 3/3] livepatch: Cleanup module page permission changes

[Petr Mladek <pmladek@suse.com>]

On Thu 2015-11-05 15:18:05, Josh Poimboeuf wrote:
> Calling set_memory_rw() and set_memory_ro() for every iteration of the
> loop in klp_write_object_relocations() is messy, inefficient, and
> error-prone.
> 
> Change all the read-only pages to read-write before the loop and convert
> them back to read-only again afterwards.
> 
> The {un}set_module_core_ro_nx() functions are used to change the
> page permissions.  Toggling NX isn't necessary in this case, but it's
> not highly performance sensitive code so it should be fine.

Hmm, the name (un)set_module_core_ro_nx() still sounds a bit strange,
especially the "ro_nx" suffix. Alternative solution would be to create

   set_module_text_rw()
   set_module_text_ro()

There already exists

   set_all_modules_text_rw()
   set_all_modules_text_ro()

They modify only the ro/rw flags. IMHO, the name is more descriptive
They are used by ftrace for very similar purpose.

They modify also the init section. But we might want to touch it
as well. klp_module_notify() is called too late now. But once we
have a more complex consistency model, we will need to reject
the module when the patching fails. We will need to call the
livepatch init earlier, close to ftrace_module_init(mod).
Then the init section might be interesting as well.


Best Regards,
Petr