[RFC] GCC support for live-patching

[Nicolai Stange]

Hi Qing,

Qing Zhao <qing.zhao@oracle.com> writes:

> thanks a lot for your detailed explanation of the source based live patch creation procedure.
> really interesting and helpful information. 
>
> more questions and comments below:
>
>>> 
>>> One question here,  what’s the major benefit to prepare the patches manually? 
>> 
>> There is none. We here at SUSE prefer the source based approach (as
>> opposed to binary diff) for a number of reasons and the manual live
>> patch creation is simply a consequence of not having any tooling for
>> this yet.
>> 
>> 
>> For reference, source based live patch creation involves the following
>> steps:
>> 
>> 1. Determine the initial set of to be patched functions:
>>   a.) Inspect the upstream diff for the fix in question, add
>>       any touched functions to the initial set.
>>   b.) For each function in the initial set, check whether it has been
>>       inlined/cloned/optimized and if so, add all its callers to the
>>       initial set.  Repeat until the initial set has stabilized.
>> 
>> 2. Copy & paste the initial set over to the new live patch sources.
>> 
>> 3. Make it compile, i.e. recursively copy any needed cpp macro, type, or
>>   functions definition and add references to data objects with static
>>   storage duration.
>>   The rules are:
>>   a.) For data objects with static storage duration, a reference to the
>>       original must always be made. (If the symbol is EXPORT()ed, then
>>       fine. Otherwise, for kGraft, this involves a kallsyms lookup at
>>       patch module load time, for upstream kernel live patching, this
>>       has been solved with those '.klp....' relocations).
>>   b.) If a called function is available as a symbol from either vmlinux
>>       or some (usually the patched) module, do not copy the definition,
>>       but add a reference to it, just as in a.).
>>   c.) If a type, cpp macro or (usually inlined) function is provided by
>>       some "public" header in <linux-tree>/include/, include that
>>       rather than copying the definition.  Counterexample: Non-public
>>       header outside of include/ like
>>       e.g. <linux-tree>/fs/btrfs/qgroup.h.
>>   d.) Otherwise copy the definition to the live patch module sources.
>> 
>> Rule 3b is not strictly necessary, but it helps in reducing the live
>> patch code size which is a factor with _manual_ live patch creation.
>> 
>> For 1b.), we need help from GCC. Namely, we want to know when some
>> functions has been optimized and we want it to disable any of those IPA
>> optimization it (currently) isn't capable to report properly.
>
> Yes, this this is the place that GCC can help. and it’s also the motivation for this current proposal.
>
>> 
>> Step 3.) is a bit tedious sometimes TBH and yes, w/o any tooling in
>> place, patch size would be a valid point. However, I'm currently working
>> on that and I'm optimistic that I'll have a working prototype soon.
>
> So, currently, step 3 is done manually?

Currently yes.


> If the initial set of patched functions is too big, this work is
> really tedious and error-prone.

For the "tedious" part: yes it sometimes is. But we haven't seen any
problems or bugs so far. So it's reliable in practice.


>
> without a good tool for step3, controlling the initial set size of patched function is still meaningful.
>
>> 
>> That tool would be given the GCC command line from the original or "live
>> patch target" kernel compilation for the source file in question, the
>> set of functions as determined in 1.) and a number of user provided
>> filter scripts to make the decisions in 3.). As a result, it would
>> output a self-contained, minimal subset of the original kernel sources.
>> 
>> With that tooling in place, live patch code size would not be a real
>> concern for us.
>
> that’s good to know.
>
> however, my question here is:
>
> can this tool be easily adopted by other applications than linux kernel? i.e, if there is another application that tries to use GCC’s live patching
> feature with manually created source patch, will your tool for step 3 be readily used by this application?  Or, this application have to develop
> a similar but different tool for itself?

This tool's scope is the C99 language, more specifically the GCC dialect
and I'll try to keep the CLI agnostic to the application or build
environment anyway.

That said, my primary interest is the Linux kernel and I'm going to
make sure that it works there. It might not work out of the box for
random applications, but require some tweaking or even bug fixing.



>> 
>> So in conclusion, what we need from GCC is the information on when we
>> have to live patch callers due to optimizations. If that's not possible
>> for a particular class of optimization, it needs to be disabled.
>> 
>> OTOH, we definitely want to keep the set of these disabled optimizations
>> as small as possible in order to limit the impact of live patching on
>> kernel performance. In particular, disabling any of the "cloning"
>> optimizations, which GCC is able to report properly, would be a
>> no-no IMO.
>> 
>> IIUC, our preferred selection of allowed IPA optimizations would be
>> provided by what you are referring to as "-flive-patching=inline-clone”.
>
> Okay. thanks for the confirmation.
>
> are there any other IPA optimizations/analyzes in addition to the inlining, cloning, ipa-sra, etc that are critical to kernel run-time performance and currently 
>
> have to be disabled due to report-ability concern?

I really don't know that, but it gets discussed at
20181003090457.GJ57692@kam.mff.cuni.cz (this thread, email from Jan
Hubicka).


>
>>>>> 
>>>>> In addition to kGraft, there are other live patching tools that prefer
>>>>> creating patches by hand for the similar reason. 
>> 
>> Out of curiosity: which? Upstream kernel live patching?
>
> it’s not kernel live patching. another application. 

Aha.


>>>> 
>>>> So let me ask, what is your motivation behind this? Is there a real 
>>>> problem you're trying to solve? It may have been mentioned somewhere and I 
>>>> missed it.
>>> 
>>> the major functionality we want is:   to Only enable static inlining for live patching for one 
>>> of our internal customers.   the major purpose is to control the patch code size explosion and
>>> debugging complexity due to too much inlining of global functions for the specific application.
>>> 
>>> therefore, I proposed the multiple level of control for -flive-patching to satisfy multiple request from 
>>> different users. 
>>> 
>>> So far, from the feedback, I see that among the 4 levels of control,   none, only-inline-static, inline,
>>> and inline-clone,   “none” and “inline” are NOT needed at all.
>>> 
>>> however,  -flive-patching = [only-inline-static | inline-clone] are
>>> necessary.
>> 
>> It would be interesting to learn why that internal customer is so keen
>> about live patch code size? I mean either their live patch creation is
>> somehow automated or not.
>
> My understanding is that their patch creation is mainly by hand. as you mentioned before,  it will be very tedious and
> error-prone in step 3, therefore they want to control the impacted function list as small as possible with reasonable run-time
> performance.
>
> I am not sure how easily for them to come up with a good tool for step 3 for their application. 
>
>> In the former case, the extra .text of
>> inline-clone over only-inline-static would be handled by tooling anyway
>> and thus, wouldn't cost any additional working hours and be highly
>> unlikely to introduce (new) regressions to be debugged. In the latter
>> case, wouldn't improving the tooling also benefit the only-inline-static
>> case?
>
> if the tool you are developing for step 3 can be easily adopted by other application than kernel,  it should helpful.

That really depends on the application, I guess.

Thanks,

Nicolai

-- 
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton,
HRB 21284 (AG Nürnberg)