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

[Miroslav Benes]

On Wed, 18 Oct 2017, Josh Poimboeuf wrote:

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

Yes, that is correct. I've just started to work on the force patch.
 
> 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 might be easier to do at least part of it in userspace.
 
> It could be a terrible idea, though it might be worth looking into.

I'm worried that we would only move the complexity elsewhere, but maybe 
not. We'd remove the code from kernel/livepatch/, but some non-trivial 
changes would go to the module loader.

Jessica, would that be possible?

We need to discuss it with packaging if that'd be possible to deliver it.

Anyway, interesting idea.

Miroslav