[kpatch] livepatch vs kpatch hooks and stack check

[Joe Lawrence]

On 4/26/19 11:40 AM, Mario Smarduch wrote:
> 
> 
> On 04/26/2019 06:32 AM, Joe Lawrence wrote:
>> On 4/26/19 3:51 AM, Mario Smarduch wrote:
>>> Hello,
>>>     I noticed that with livepatch the old and new functions can
>>> run in parallel, but with kpatch the kpatch module load fails on
>>> "activeness safety check".
>>
>> Hi Mario,
>>
>> I think you are referring to kpatch's use of stop_machine() to quiesce
>> the system before patching vs. livepatch's consistency model[1] and its
>> convergence on a patched state?
>>
>>> Am I missing something with livepatch? Is there another way to
>>> build and run it?
>>
>> AFAIK there is no way around the consistency model with livepatch.  What
>> you might be able to do is hack the kpatch-build to utilize the kpatch
>> support module instead of the livepatch API.  (Look for the
>> $KPATCH_MODULE code in the kpatch-build script.)  I don't think this use
>> case is very well tested and in fact, the kpatch.ko will be deprecated
>> eventually.
> 
> 
>>
>> Do you have a particular case in mind which the livepatch consistency
>> model does not support?
> The use case is replacing a function that's sleeping somewhere in
> uninterruptible mode. With livepatch the function is patched anyway
> with new and old functions sharing data. Eventually the old function
> wakes up and can result in problems as two have different view of the
> shared data (i.e. data field interpretation).
> 
> With kpatch it works fine.

Ah yes, I can see how kpatch stop_machine() model makes this 10x easier 
to handle.

You may be able to continue using kpatch.ko (with the mentioned 
kpatch-build changes), or to make a livepatch version safer:

- Shadow variables:  check out the second sample "In-flight parent 
objects" in Documentation/livepatch/shadow-vars.txt.  It shows how to 
translate 1d147bfa6429 ("mac80211: fix AP powersave TX vs. wakeup race") 
into a livepatch that will safely operate on "new" and "old" 
ieee80211_tx_latency_stat structures.

In your case, if it is only the interpretation of existing data that 
needs to change, I think you may be able to get away with an empty 
shadow variable -- this idea is mentioned in the 'Other use-cases' 
section of shadow-vars.txt.


Alternatively, if you want a giant throw-switch of sorts:

- Post-patch callbacks:  See Documentation/livepatch/callbacks.txt for 
details on livepatching callbacks.  Of particular interest would be the 
'post-patch' callback, which only runs once *all* tasks have been 
migrated to running new code.  You may be able to employ a post-patch 
callback that sets an indicator to the new code that patching is complete.


If you can share the source .patch or upstream commit id, that would be 
helpful, but I can understand if that needs to remain confidential for 
security reasons.

Hope this helps,

-- Joe