Messages in this thread | | | Date | Tue, 26 Sep 2017 16:49:12 +0200 | From | Petr Mladek <> | Subject | Re: [PATCH v5 1/3] livepatch: add (un)patch callbacks |
| |
On Thu 2017-08-31 10:53:51, Joe Lawrence wrote: > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c > index b9628e43c78f..aca62c4b8616 100644 > --- a/kernel/livepatch/core.c > +++ b/kernel/livepatch/core.c > @@ -54,11 +54,6 @@ static bool klp_is_module(struct klp_object *obj) > return obj->name; > } > > -static bool klp_is_object_loaded(struct klp_object *obj) > -{ > - return !obj->name || obj->mod; > -} > - > /* sets obj->mod if object is not vmlinux and module is found */ > static void klp_find_object_module(struct klp_object *obj) > { > @@ -285,6 +280,8 @@ static int klp_write_object_relocations(struct module *pmod, > > static int __klp_disable_patch(struct klp_patch *patch) > { > + struct klp_object *obj; > + > if (klp_transition_patch) > return -EBUSY; > > @@ -295,6 +292,10 @@ static int __klp_disable_patch(struct klp_patch *patch) > > klp_init_transition(patch, KLP_UNPATCHED); > > + klp_for_each_object(patch, obj) > + if (patch->enabled && obj->patched) > + klp_pre_unpatch_callback(obj); > + > /* > * Enforce the order of the func->transition writes in > * klp_init_transition() and the TIF_PATCH_PENDING writes in > @@ -388,13 +389,18 @@ static int __klp_enable_patch(struct klp_patch *patch) > if (!klp_is_object_loaded(obj)) > continue; > > - ret = klp_patch_object(obj); > + ret = klp_pre_patch_callback(obj); > if (ret) { > - pr_warn("failed to enable patch '%s'\n", > - patch->mod->name); > + pr_warn("pre-patch callback failed for object '%s'\n", > + klp_is_module(obj) ? obj->name : "vmlinux"); > + goto err; > + } > > - klp_cancel_transition(); > - return ret; > + ret = klp_patch_object(obj); > + if (ret) { > + pr_warn("failed to patch object '%s'\n", > + klp_is_module(obj) ? obj->name : "vmlinux");
We should call klp_post_unpatch_callback(obj) here to make it synchronous.
Well, what about calling:
klp_pre_patch_callback() inside klp_patch_object() and klp_post_unpatch_callback() inside klp_unpatch_object()
By other words, we would do the two operations. It would have two advantages:
+ error handling for free + no need for the strange callbacks_enabled flag
It would require the more strict consistency model if there is a dependency between the callbacks and patches from various modules. But we would probably need the consistency model in this case anyway.
> + goto err;
> } > } >
Otherwise I think that we are getting close.
Best Regards, Petr
PS: I hope that the above problem and solution has not been mentioned yet. I am sorry if it was. I am a bit lost in many mails after vacation, sickness, and conference.
| |