lkml.org 
[lkml]   [2018]   [Mar]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v8 0/8] livepatch: Atomic replace feature
From
Date
Hi,

Petr, Jason - thanks a lot for working on this series, first of all! And
especially for your patience.

On 21.02.2018 16:29, Petr Mladek wrote:
> The atomic replace allows to create cumulative patches. They
> are useful when you maintain many livepatches and want to remove
> one that is lower on the stack. In addition it is very useful when
> more patches touch the same function and there are dependencies
> between them.

I have experimented with this updated patchset a bit.
It looks like replace operation fails if there is a loaded but disabled
patch.

Suppose there are 2 binary patches changing the same kernel functions.
Load patch1, then disable it (echo 0 >
/sys/kernel/livepatch/patch1/enabled), then try load patch2 with atomic
replace. I get "Device or resource busy" error at this point.

It seems, __klp_enable_patch() returns -EBUSY for some reason. I haven't
dug deeper into that code yet.

A workaround is simple: just unload all disabled patches before trying
to load patch2 with replace. Still, the behavior is quite strange.

>
> I have found one bug in v7. We were not able to initialize NOP
> struct klp_func when the patches module is not loaded. It was
> because func->new_func was NULL. I have fixed it in separate patch
> for an easier review.
>
> Note that the original Jason's patch did not have this problem
> because func->new_func was always NULL there. But this required
> NOP-specific handling also on other locations, namely
> klp_ftrace_handler() and klp_check_stack_func().
>
> Changes against v7:
>
> + Fixed handling of NOPs for not-yet-loaded modules
> + Made klp_replaced_patches list static [Mirek]
> + Made klp_free_object() public later [Mirek]
> + Fixed several reported typos [Mirek, Joe]
> + Updated documentation according to the feedback [Joe]
> + Added some Acks [Mirek]
>
> Changes against v6:
>
> + used list_move when disabling replaced patches [Jason]
> + renamed KLP_FUNC_ORIGINAL -> KLP_FUNC_STATIC [Mirek]
> + used klp_is_func_type() in klp_unpatch_object() [Mirek]
> + moved static definition of klp_get_or_add_object() [Mirek]
> + updated comment about synchronization in forced mode [Mirek]
> + added user documentation
> + fixed several typos
>
> Jason Baron (5):
> livepatch: Use lists to manage patches, objects and functions
> livepatch: Initial support for dynamic structures
> livepatch: Allow to unpatch only functions of the given type
> livepatch: Support separate list for replaced patches.
> livepatch: Add atomic replace
>
> Petr Mladek (3):
> livepatch: Free only structures with initialized kobject
> livepatch: Correctly handle atomic replace for not yet loaded modules
> livepatch: Atomic replace and cumulative patches documentation
>
> Documentation/livepatch/cumulative-patches.txt | 83 ++++++
> include/linux/livepatch.h | 59 +++-
> kernel/livepatch/core.c | 394 ++++++++++++++++++++++---
> kernel/livepatch/core.h | 4 +
> kernel/livepatch/patch.c | 31 +-
> kernel/livepatch/patch.h | 4 +-
> kernel/livepatch/transition.c | 41 ++-
> 7 files changed, 566 insertions(+), 50 deletions(-)
> create mode 100644 Documentation/livepatch/cumulative-patches.txt
>

Regards,
Evgenii

\
 
 \ /
  Last update: 2018-03-05 11:58    [W:0.162 / U:0.908 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site