Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1693907

Re: [PATCH 0/3] livepatch: introduce atomic replace

Show all headers | View raw

On 07/21/2017 09:06 AM, Miroslav Benes wrote:
> On Wed, 19 Jul 2017, Jason Baron wrote:
>
>> Hi,
>>
>> In testing 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 introducd 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
>
> Hi Jason,
>
> this has been on my TODO list for a long time now, so thanks for working
> on this. We have the same feature in kGraft and we use it heavily (in fact
> we distribute our patches as cumulative and "replace_all" how we call it).
>

Hi Miroslav,

Cool - we feel like this is an important feature as well and would like 
to have an upstream solution as well.

> The forward port of the feature from kGraft is unfortunately not
> straightforward. We do not have a concept of klp_target_state there, so we
> can freely let functions to be patched or reverted in one go. We cannot do
> the same upstream. At first glance, you used nop function exactly for this
> case. Nice hack :).
>
>> Patches:
>>
>> 1) livepatch: Add klp_object and klp_func iterators
>> 	Just a prep patch for the 'atomic revert' feature
>>
>> 2) livepatch: add atomic replace
>> 	Core feature
>>
>> 3) livepatch: Add a sysctl livepatch_mode for atomic replace
>> 	Introduces a knob for enabling atomic replace. I hate knobs and perhaps
>> 	its possible to default to cumulative replace? Although I suspect there
>> 	are workflows relying on the existing behavior - I'm not sure. It may
>> 	be desirable to associate the knob with the patch itself as in the
>> 	'immediate' flag, such that we don't introduce a global sysctl that
>> 	likely would also need to built-in, if there are patches in the initrd.
>
> Yes. I think it should be associated with the patch itself. This would
> allow more flexible behaviour. You could stack more patches on top of
> "atomic replace" patch.
>

Ok - associating the "atomic replace" with the patch itself makes sense 
to me. It would also basically work, I think with the patch I proposed 
except for the case where the the "atomic replace" was on top of several 
non-"atomic replace" patches. The reason is that the "atomic replace" I 
posted looks back 1 patch to see what it needs to replace (assuming all 
patches are in atomic replace mode). So instead of just looking back 1 
patch, it could 'look back' and make sure it was replacing all 
previously loaded patches.


> Anyway, I'm on holiday next week, so I'll take a proper look the week
> after.
>

Ok - have a nice holiday!

Thanks,

-Jason


> Thanks,
> Miroslav
>

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[PATCH 0/3] livepatch: introduce atomic replace Jason Baron <jbaron@akamai.com> - 2017-07-19 19:40 +0200
  [PATCH 3/3] livepatch: Add a sysctl livepatch_mode for atomic replace Jason Baron <jbaron@akamai.com> - 2017-07-19 19:40 +0200
  [PATCH 2/3] livepatch: add atomic replace Jason Baron <jbaron@akamai.com> - 2017-07-19 19:50 +0200
  Re: [PATCH 0/3] livepatch: introduce atomic replace Miroslav Benes <mbenes@suse.cz> - 2017-07-21 15:10 +0200
    Re: [PATCH 0/3] livepatch: introduce atomic replace Jason Baron <jbaron@akamai.com> - 2017-07-21 20:00 +0200

csiph-web