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
