Could someone help to review this patch? Thanks Minfei
On 07/15/15 at 04:55pm, Minfei Huang wrote: > From: Minfei Huang <[email protected]> > > Livepatch will obey the stacking rule to enable/disable the patch. It > only allows to enable the patch, when it is the fist disabled patch, > disable the patch, when it is the last enabled patch. > > In the livepatch code, it uses list to gather the all of the patches. > And we do not know whether the previous/next patch is patched to the > same modules or vmlinux in that way. > > According to above rule, livepatch will make incorrect decision to > enable/disable the patch. Following is an example to show how livepatch > does. > > - install the livepatch example module which is in samples/livepatch. > - install the third part kernel module > - install the livepatch module which is patched to the third part module > - disable the livepatch example module > > We can find that we can not disable livepatch example module, although > it is the last enabled patch. > > To fix this issue, we will find the corresponding patch which is patched > to the same modules or vmlinux, when we enable/disable the patch. > > Signed-off-by: Minfei Huang <[email protected]> > --- > kernel/livepatch/core.c | 55 > +++++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 51 insertions(+), 4 deletions(-) > > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c > index 6e53441..d59aec7 100644 > --- a/kernel/livepatch/core.c > +++ b/kernel/livepatch/core.c > @@ -429,6 +429,27 @@ err: > return ret; > } > > +static bool is_patched_to_same(struct klp_patch *p1, struct klp_patch *p2) > +{ > + struct klp_object *obj1, *obj2; > + bool is_mod1, is_mod2; > + > + klp_for_each_object(p1, obj1) { > + klp_for_each_object(p2, obj2) { > + is_mod1 = !!klp_is_module(obj1); > + is_mod2 = !!klp_is_module(obj2); > + > + if (is_mod1 && is_mod2) { > + if (!strcmp(obj1->name, obj2->name)) > + return true; > + } else if (!is_mod1 && !is_mod2) > + return true; > + } > + } > + > + return false; > +} > + > static void klp_disable_object(struct klp_object *obj) > { > struct klp_func *func; > @@ -463,13 +484,26 @@ static int klp_enable_object(struct klp_object *obj) > return 0; > } > > +struct klp_patch *get_next_patch(struct klp_patch *patch) > +{ > + struct klp_patch *p = patch; > + > + list_for_each_entry_continue(p, &klp_patches, list) { > + if (is_patched_to_same(p, patch)) > + return p; > + } > + > + return NULL; > +} > + > static int __klp_disable_patch(struct klp_patch *patch) > { > + struct klp_patch *p; > struct klp_object *obj; > > /* enforce stacking: only the last enabled patch can be disabled */ > - if (!list_is_last(&patch->list, &klp_patches) && > - list_next_entry(patch, list)->state == KLP_ENABLED) > + p = get_next_patch(patch); > + if (p && (p->state == KLP_ENABLED)) > return -EBUSY; > > pr_notice("disabling patch '%s'\n", patch->mod->name); > @@ -516,8 +550,21 @@ err: > } > EXPORT_SYMBOL_GPL(klp_disable_patch); > > +struct klp_patch *get_prev_patch(struct klp_patch *patch) > +{ > + struct klp_patch *p = patch; > + > + list_for_each_entry_continue_reverse(p, &klp_patches, list) { > + if (is_patched_to_same(p, patch)) > + return p; > + } > + > + return NULL; > +} > + > static int __klp_enable_patch(struct klp_patch *patch) > { > + struct klp_patch *p; > struct klp_object *obj; > int ret; > > @@ -525,8 +572,8 @@ static int __klp_enable_patch(struct klp_patch *patch) > return -EINVAL; > > /* enforce stacking: only the first disabled patch can be enabled */ > - if (patch->list.prev != &klp_patches && > - list_prev_entry(patch, list)->state == KLP_DISABLED) > + p = get_prev_patch(patch); > + if (p && (p->state == KLP_DISABLED)) > return -EBUSY; > > pr_notice_once("tainting kernel with TAINT_LIVEPATCH\n"); > -- > 2.2.2 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
