Re: PATCH v6 6/6] livepatch: Add atomic replace

[Petr Mladek]

On Thu 2018-01-25 23:27:57, Jason Baron wrote:
> On 01/25/2018 11:02 AM, Petr Mladek wrote:
> > From: Jason Baron <jbaron@xxxxxxxxxx>
> > A better solution would be to create cumulative patch and say that
> > it replaces all older ones.
> > 
> > Signed-off-by: Jason Baron <jbaron@xxxxxxxxxx>
> > [pmladek@xxxxxxxx: Split, reuse existing code, simplified]
> 
> Hi Petr,
> 
> Thanks for cleaning this up - it looks good.

Uff, I feel relief :-)

> > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> > index 6ad3195d995a..c606b291dfcd 100644
> > --- a/kernel/livepatch/core.c
> > +++ b/kernel/livepatch/core.c
> > +/*
> > + * This function removes replaced patches from both func_stack
> > + * and klp_patches stack.
> > + *
> > + * We could be pretty aggressive here. It is called in situation
> > + * when these structures are not longer accessible. All functions
> > + * are redirected using the klp_transition_patch. They use either
> > + * a new code or they in the original code because of the special
> > + * nop function patches.
> > + */
> > +void klp_throw_away_replaced_patches(struct klp_patch *new_patch,
> > +				     bool keep_module)
> > +{
> > +	struct klp_patch *old_patch, *tmp_patch;
> > +
> > +	list_for_each_entry_safe(old_patch, tmp_patch, &klp_patches, list) {
> > +		if (old_patch == new_patch)
> > +			return;
> > +
> > +		klp_unpatch_objects(old_patch, KLP_FUNC_ANY);
> > +		old_patch->enabled = false;
> > +
> > +		/*
> > +		 * Replaced patches could not get re-enabled to keep
> > +		 * the code sane.
> > +		 */
> > +		list_del_init(&old_patch->list);
> 
> I'm wondering if this should be:
> 
> list_move(&old_patch->list, &klp_replaced_patches);

Yes, great catch!

The list_del() comes from one iteration where I got rid of the extra
list. I though that it might be enough to check
patch->kobj.state_initialized. But then I realized that this
kobject state was modified outside klp_mutex.

To be honest, I did not only minimal testing with my changes.
Mirek promised to port a battery of his kGraft-based tests and
run it.

Thanks a lot for review.

Best Regards,
Petr