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

From: Petr Mladek
Date: Fri Jan 26 2018 - 04:09:18 EST


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