Messages in this thread | | | Subject | Re: [PATCH v5 0/3] livepatch: introduce atomic replace | From | Evgenii Shatokhin <> | Date | Fri, 19 Jan 2018 22:20:27 +0300 |
| |
On 12.01.2018 22:55, Jason Baron wrote: > Hi, > > While using livepatch, I found that when doing cumulative patches, if a patched > function is completed reverted by a subsequent patch (back to its original state) > livepatch does not revert the funtion to its original state. Specifically, if > patch A introduces a change to function 1, and patch B reverts the change to > function 1 and introduces changes to say function 2 and 3 as well, the change > that patch A introduced to function 1 is still present. This could be addressed > by first completely removing patch A (disable and then rmmod) and then inserting > patch B (insmod and enable), but this leaves an unpatched window. In discussing > this issue with Josh on the kpatch mailing list, he mentioned that we could get > 'atomic replace working properly', and that is the direction of this patchset: > https://www.redhat.com/archives/kpatch/2017-June/msg00005.html > > Thanks, > > -Jason
Thanks a lot! Atomic replace is really crucial when using cumulative patches.
There is one more thing that might need attention here. In my experiments with this patch series, I saw that unpatch callbacks are not called for the older binary patch (the one being replaced).
That is, I have prepared 2 binary patches, each has all 4 patch/unpatch callbacks.
When I load the first patch, its pre-patch and post-patch callbacks are called as expected.
Then I replace it with the second patch. Replacement is successful, the pre-patch and post-patch callbacks are called for the second patch, However, pre-unpatch and post-unpatch callbacks do not run for the first one. This makes it more difficult to clean up what its pre/post-patch callbacks have done.
It would be nice if pre-/post- unpatch callbacks were called for the first patch, perhaps, before/after the patch is actually disabled during replacement. I cannot see right now though, which way is the best to implement that. > > v4-v5 > -re-base onto remove-immediate branch (removing immediate dependencies) > -replaced modules can be re-enabled by doing rmmod and then insmod > > v3-v4: > -add static patch, objects, funcs to linked lists to simplify iterator > -break-out pure function movement as patch 2/3 > > v2-v3: > -refactor how the dynamic nops are calculated (Petr Mladek) > -move the creation of dynamic nops to enable/disable paths > -add klp_replaced_patches list to indicate patches that can be re-enabled > -dropped 'replaced' field > -renamed dynamic fields in klp_func, object and patch > -moved iterator implementation to kernel/livepatch/core.c > -'inherit' nop immediate flag > -update kobject_put free'ing logic (Petr Mladek) > > v1-v2: > -removed the func_iter and obj_iter (Petr Mladek) > -initialiing kobject structure for no_op functions using: > klp_init_object() and klp_init_func() > -added a 'replace' field to klp_patch, similar to the immediate field > -a 'replace' patch now disables all previous patches > -tried to shorten klp_init_patch_no_ops()... > -Simplified logic klp_complete_transition (Petr Mladek) > > Jason Baron (3): > livepatch: use lists to manage patches, objects and functions > livepatch: shuffle core.c function order > livepatch: add atomic replace > > include/linux/livepatch.h | 25 +- > kernel/livepatch/core.c | 626 ++++++++++++++++++++++++++++++------------ > kernel/livepatch/core.h | 6 + > kernel/livepatch/patch.c | 22 +- > kernel/livepatch/patch.h | 4 +- > kernel/livepatch/transition.c | 49 +++- > 6 files changed, 537 insertions(+), 195 deletions(-) >
Regards, Evgenii
| |