Messages in this thread | | | Date | Fri, 26 Jan 2018 10:09:01 +0100 | From | Petr Mladek <> | Subject | Re: PATCH v6 6/6] livepatch: Add atomic replace |
| |
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@akamai.com> > > A better solution would be to create cumulative patch and say that > > it replaces all older ones. > > > > Signed-off-by: Jason Baron <jbaron@akamai.com> > > [pmladek@suse.com: 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
| |