lkml.org 
[lkml]   [2018]   [Feb]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: PATCH v6 6/6] livepatch: Add atomic replace

> -struct klp_object *klp_get_or_add_object(struct klp_patch *patch,
> +static struct klp_object *klp_get_or_add_object(struct klp_patch *patch,
> struct klp_object *old_obj)

A nit, but this change belongs to 3/6, doesn't it?

> {
> struct klp_object *obj;
> @@ -708,6 +764,102 @@ struct klp_object *klp_get_or_add_object(struct klp_patch *patch,
> return obj;
> }

[...]

> diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c
> index 6917100fbe79..3f6cf5b9e048 100644
> --- a/kernel/livepatch/transition.c
> +++ b/kernel/livepatch/transition.c
> @@ -87,6 +87,33 @@ static void klp_complete_transition(void)
> klp_transition_patch->mod->name,
> klp_target_state == KLP_PATCHED ? "patching" : "unpatching");
>
> + /*
> + * For replace patches, we disable all previous patches, and replace
> + * the dynamic no-op functions by removing the ftrace hook.
> + */
> + if (klp_transition_patch->replace && klp_target_state == KLP_PATCHED) {
> + /*
> + * Make sure klp_ftrace_handler() can no longer see functions
> + * from the replaced patches. Then all functions will be
> + * redirected using klp_transition_patch. They will use
> + * either a new code or they will stay in the original code
> + * because of the nop funcs' structures.
> + */
> + if (klp_forced)
> + klp_synchronize_transition();

I'm not sure why this is here. klp_forced should not be so special that it
would warrant a synchronization. However, I'm also not sure that it would
be safe to remove it from the patch.

Is this because "force" is the only one who can clear TIF_PATCH_PENDING of
other tasks? So, there could be a task running in klp_ftrace_handler(),
its TIF_PATCH_PENDING is cleared by force and we could have a problem
because of that since we're throwing away replaced patches.

Ok, that sounds viable. I'd welcome a comment on that in the code.

Thanks,
Miroslav

> +
> + klp_throw_away_replaced_patches(klp_transition_patch,
> + klp_forced);
> +
> + /*
> + * There is no need to synchronize the transition after removing
> + * nops. They must be the last on the func_stack. Ftrace
> + * gurantees that nobody will stay in the trampoline after
> + * the ftrace handler is unregistered.
> + */
> + klp_unpatch_objects(klp_transition_patch, KLP_FUNC_NOP);
> + }
> +
> if (klp_target_state == KLP_UNPATCHED) {
> /*
> * All tasks have transitioned to KLP_UNPATCHED so we can now

\
 
 \ /
  Last update: 2018-02-01 14:50    [W:0.151 / U:0.160 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site