On Fri, Oct 18, 2019 at 03:40:58PM +0200, Petr Mladek wrote:

On Fri 2019-10-18 15:03:42, Jessica Yu wrote:

quoted

+++ Miroslav Benes [16/10/19 15:29 +0200]:

quoted

On Wed, 16 Oct 2019, Miroslav Benes wrote:
Thinking about it more... crazy idea. I think we could leverage these new
ELF .text per vmlinux/module sections for the reinvention I was talking
about. If we teach module loader to relocate (and apply alternatives and
so on, everything in arch-specific module_finalize()) not the whole module
in case of live patch modules, but separate ELF .text sections, it could
solve the issue with late module patching we have. It is a variation on
Steven's idea. When live patch module is loaded, only its section for
present modules would be processed. Then whenever a to-be-patched module
is loaded, its .text section in all present patch module would be
processed.

The upside is that almost no work would be required on patch modules
creation side. The downside is that klp_modinfo must stay. Module loader
needs to be hacked a lot in both cases. So it remains to be seen which
idea is easier to implement.

Jessica, do you think it would be feasible?

I think that does sound feasible. I'm trying to visualize how that
would look. I guess there would need to be various livepatching hooks
called during the different stages (apply_relocate_add(),
module_finalize(), module_enable_ro/x()).

So maybe something like the following?

When a livepatch module loads:
   apply_relocate_add()
       klp hook: apply .klp.rela.$objname relocations *only* for
       already loaded modules
   module_finalize()
       klp hook: apply .klp.arch.$objname changes for already loaded modules
   module_enable_ro()
       klp hook: only enable ro/x for .klp.text.$objname for already
       loaded modules

Just for record. We should also set ro for the not-yet used
.klp.text.$objname at this stage so that it can't be modified
easily "by accident".

quoted

When a to-be-patched module loads:
   apply_relocate_add()
       klp hook: for each patch module that patches the coming
       module, apply .klp.rela.$objname relocations for this object
   module_finalize()
       klp hook: for each patch module that patches the coming
       module, apply .klp.arch.$objname changes for this object
   module_enable_ro()
       klp hook: for each patch module, apply ro/x permissions for
       .klp.text.$objname for this object

Then, in klp_module_coming, we only need to do the callbacks and
enable the patch, and get rid of the module_disable_ro->apply
relocs->module_enable_ro block.

Does that sound like what you had in mind or am I totally off?

Makes sense to me.

Well, I wonder if it is really any better from what we have now.

AFAICT, this would still have a lot of the same problems we have today.
It has a lot of complexity.  It needs arch-specific livepatch code and
sections, and introduces special cases in the module code.

I'd much prefer the proposal from LPC to have per-module live patches.
It's simpler and has less things that can go wrong IMO.

-- 
Josh