Commit 5aaf1ab5 authored by Petr Mladek's avatar Petr Mladek Committed by Jiri Kosina

livepatch: Correctly call klp_post_unpatch_callback() in error paths

The post_unpatch_enabled flag in struct klp_callbacks is set when a
pre-patch callback successfully executes, indicating that we need to
call a corresponding post-unpatch callback when the patch is reverted.
This is true for ordinary patch disable as well as the error paths of
klp_patch_object() callers.

As currently coded, we inadvertently execute the post-patch callback
twice in klp_module_coming() when klp_patch_object() fails:

  - We explicitly call klp_post_unpatch_callback() for the failed object
  - We call it again for the same object (and all the others) via
    klp_cleanup_module_patches_limited()

We should clear the flag in klp_post_unpatch_callback() to make
sure that the callback is not called twice. It makes the API
more safe.

(We could have removed the callback from the former error path as it
would be covered by the latter call, but I think that is is cleaner to
clear the post_unpatch_enabled after its invoked. For example, someone
might later decide to call the callback only when obj->patched flag is
set.)

There is another mistake in the error path of klp_coming_module() in
which it skips the post-unpatch callback for the klp_transition_patch.
However, the pre-patch callback was called even for this patch, so be
sure to make the corresponding callbacks for all patches.

Finally, I used this opportunity to make klp_pre_patch_callback() more
readable.

[jkosina@suse.cz: incorporate changelog wording changes proposed by Joe Lawrence]
Signed-off-by: default avatarPetr Mladek <pmladek@suse.com>
Acked-by: default avatarJoe Lawrence <joe.lawrence@redhat.com>
Signed-off-by: default avatarJiri Kosina <jkosina@suse.cz>
parent af026796
...@@ -894,9 +894,7 @@ int klp_module_coming(struct module *mod) ...@@ -894,9 +894,7 @@ int klp_module_coming(struct module *mod)
pr_warn("failed to apply patch '%s' to module '%s' (%d)\n", pr_warn("failed to apply patch '%s' to module '%s' (%d)\n",
patch->mod->name, obj->mod->name, ret); patch->mod->name, obj->mod->name, ret);
if (patch != klp_transition_patch) klp_post_unpatch_callback(obj);
klp_post_unpatch_callback(obj);
goto err; goto err;
} }
......
...@@ -12,10 +12,10 @@ static inline bool klp_is_object_loaded(struct klp_object *obj) ...@@ -12,10 +12,10 @@ static inline bool klp_is_object_loaded(struct klp_object *obj)
static inline int klp_pre_patch_callback(struct klp_object *obj) static inline int klp_pre_patch_callback(struct klp_object *obj)
{ {
int ret; int ret = 0;
ret = (obj->callbacks.pre_patch) ? if (obj->callbacks.pre_patch)
(*obj->callbacks.pre_patch)(obj) : 0; ret = (*obj->callbacks.pre_patch)(obj);
obj->callbacks.post_unpatch_enabled = !ret; obj->callbacks.post_unpatch_enabled = !ret;
...@@ -39,6 +39,8 @@ static inline void klp_post_unpatch_callback(struct klp_object *obj) ...@@ -39,6 +39,8 @@ static inline void klp_post_unpatch_callback(struct klp_object *obj)
if (obj->callbacks.post_unpatch_enabled && if (obj->callbacks.post_unpatch_enabled &&
obj->callbacks.post_unpatch) obj->callbacks.post_unpatch)
(*obj->callbacks.post_unpatch)(obj); (*obj->callbacks.post_unpatch)(obj);
obj->callbacks.post_unpatch_enabled = false;
} }
#endif /* _LIVEPATCH_CORE_H */ #endif /* _LIVEPATCH_CORE_H */
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment