Re: [RFC PATCH v1.9 00/14] livepatch: hybrid consistency model

[Petr Mladek]

On Fri 2016-03-25 14:34:47, Josh Poimboeuf wrote:
> These patches are still a work in progress, but Jiri asked that I share
> them before I go on vacation next week.  Based on origin/master because
> it has CONFIG_STACK_VALIDATION.

I have to follow Mirek and say that it is a great work.

> There's also a func->immediate flag which allows users to specify that
> certain functions in the patch can be applied without per-task
> consistency.  This might be useful if you want to patch a common
> function like schedule(), and the function change doesn't need
> consistency but the rest of the patch does.

I like the possibility to immediately patch some functions or objects.
Just note that this is not yet completely implemented and it is not
on the TODO list.

We probably should not set func->transition flag when func->immediate
is set or when the related func->object is set. It currently happens
only when patch->immediate is set.

Also we should ignore immediate functions and objects when the stack
is checked.


> Still have a lot of TODOs, some of them are listed here.  If you see
> something objectionable, it might be a good idea to make sure it's not
> already on the TODO list :-)
> 
> TODO:
> - come up with a better name than universe?  KLP_STATE_PREV/NEXT?
>   KLP_UNPATCHED/PATCHED?  there were some objections to the name in v1.

The name "universe" has an advantage if we later allow to
enable/disable more patches in parallel. The integer might hold
an identifier of the last applied patch. I have been playing with
this for kGraft one year ago and it was really challenging.
We should avoid it if possible. It is not really needed
if we are able to complete any transition in a reasonable time.

If we support only one transition at a time, a simple boolean
or even bit should be enough. The most descriptive name would
be klp_transition_patch_applied but it is quite long.

Note that similar information is provided by TIF_KLP_NEED_UPDATE
flag. We use only this bit in kGraft. It saves some space in
task_struct but it brings other challenges. We need to prevent
migration using a global "kgr_immutable" flag until ftrace handlers
for all patched functions are in place. We need to set the flag
back when the ftrace handler is called and the global "kgr_immutable"
flag is set.

> - update documentation for sysfs, proc, livepatch

Also we should publish somewhere the information about TIF_KLP_NEED_UPDATE
flag, e.g. /proc/<pid>/klp_need_update. It is handy to see what
process blocks the transition. We have something similar in
kGraft, see in_progress_show() at
https://git.kernel.org/cgit/linux/kernel/git/jirislaby/kgraft.git/commit/?h=kgraft-4.4&id=1c82fbd7b1fe240f4ed178a6506a93033f6a4bed


I am still shaking my head around the patches.

Best Regards,
Petr