Could someone help to review this patch?
Thanks
Minfei
On 07/15/15 at 04:55pm, Minfei Huang wrote:
> From: Minfei Huang <mnfhuang@xxxxxxxxx>
>
> 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 <mnfhuang@xxxxxxxxx>
> ---
> 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 live-patching" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
