Re: [PATCH v3 2/2] livepatch: add atomic replace

[Josh Poimboeuf]

On Wed, Oct 18, 2017 at 03:36:42PM +0200, Jiri Kosina wrote:
> On Wed, 18 Oct 2017, Miroslav Benes wrote:
> 
> > 3. Drop immediate. It causes problems only and its advantages on x86_64 
> > are theoretical. You would still need to solve the interaction with atomic 
> > replace on other architecture with immediate preserved, but that may be 
> > easier. Or we can be aggressive and drop immediate completely. The force 
> > transition I proposed earlier could achieve the same.
> 
> After brief off-thread discussion, I've been thinking about this a bit 
> more and I also think that we should claim immediate "an experiment that 
> failed", especially as the force functionality (which provides equal 
> functionality from the userspace POV) will likely be there sonnish.

Agreed.

To clarify, we'll need the force patch before removing
klp_patch.immediate, so we don't break non-x86 arches in the meantime.

On the other hand I think we could remove klp_func.immediate
immediately.


While we're on the subject of removing code... :-)


I've mentioned this several times before, but the more features we add,
the more obvious this point becomes: if we could figure out how to get
rid of the "patching unloaded modules" feature, the code would be so
much better, and I'd actually be able to fit the code in my brain.  Then
we could get rid of all these sneaky bugs that Miroslav and Petr keep
finding, and I wouldn't get an uneasy feeling everytime somebody posts a
new feature.

Here's one vague idea for how to achieve that.  More ideas welcome.

1) Make the consistency model synchronous with respect to modules: don't
   allow any modules to load or unload until the patch transition is
   complete.

2) Instead of one big uber patch module which patches vmlinux and
   modules at the same time, make each patch module specific to a single
   object.  Then bundle the patch modules together somehow into a "patch
   bundle" so they're treated as a single atomic unit.

3) The patch bundle, when loaded, would load some of its patch modules
   immediately (for those objects which are already loaded).  For
   unloaded objects, the corresponding patch modules will be copied into
   memory but not loaded.

4) Then, when a to-be-patched module is loaded, the module loader loads
   it into memory and relocates it, but doesn't make it live.  Then it
   loads the patch module from the memory blob, makes the patch module
   live, and then makes the to-be-patched module live.

(A variation would be to create a way for user space to load a module in
the paused state.  Then user space can handle the dependencies and do
the patch juggling.  I think that would mean depmod/modprobe would need
to be involved.)

It could be a terrible idea, though it might be worth looking into.

-- 
Josh