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

[Josh Poimboeuf]

On Thu, Oct 19, 2017 at 10:30:24AM +0200, Miroslav Benes wrote:
> 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.

Good!

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

Yes, that's a very real possibility.  In order for it to be an
improvement, it would have to be self-contained.  We don't want to make
the module code worse either.

> Jessica, would that be possible?
> 
> We need to discuss it with packaging if that'd be possible to deliver it.
> 
> Anyway, interesting idea.

Yeah, just an idea to consider...  I'd like to play around with it
someday.

-- 
Josh