Messages in this thread | | | Subject | Re: [PATCH v8 0/8] livepatch: Atomic replace feature | From | Evgenii Shatokhin <> | Date | Mon, 5 Mar 2018 13:56:40 +0300 |
| |
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
| |