Re: [PATCH v1 1/5] xsplice: Design document.

[Martin Pohlack <mpohlack@amazon.com>]

On 06.10.2015 14:57, Ross Lagerwall wrote:
> On 09/16/2015 10:01 PM, Konrad Rzeszutek Wilk wrote:
>> +### xSplice interdependencies
>> +
>> +xSplice patches interdependencies are tricky.
>> +
>> +There are the ways this can be addressed:
>> + * A single large patch that subsumes and replaces all previous ones.
>> +   Over the life-time of patching the hypervisor this large patch
>> +   grows to accumulate all the code changes.
>> + * Hotpatch stack - where an mechanism exists that loads the hotpatches
>> +   in the same order they were built in. We would need an build-id
>> +   of the hypevisor to make sure the hot-patches are build against the
>> +   correct build.
>> + * Payload containing the old code to check against that. That allows
>> +   the hotpatches to be loaded indepedently (if they don't overlap) - or
>> +   if the old code also containst previously patched code - even if they
>> +   overlap.
>> +
>> +The disadvantage of the first large patch is that it can grow over
>> +time and not provide an bisection mechanism to identify faulty patches.
>> +
>> +The hot-patch stack puts stricts requirements on the order of the patches
>> +being loaded and requires an hypervisor build-id to match against.
>> +
>> +The old code allows much more flexibility and an additional guard,
>> +but is more complex to implement.
>> +
> 
> If the single large patch mechanism is used, a new REPLACE action is 
> needed to atomically replace one patch with another to prevent a window 
> where the hypervisor is unpatched. kpatch has a "replace" command for 
> this purpose. This may be useful even for the other mechanisms listed above.
> 
>  From what I can tell:
> * kSplice uses old code checking (method [3] above), although in 
> practice the userspace tools implement dependency logic to enforce a 
> linear stack of patches.
> * kPatch and kGraft recommend using the single large patch mechanism 
> although there's nothing preventing two independent patches from being 
> loaded.

To make sure this argument is not lost: I wrote in an earlier email

> * There is a general (and mostly obscure) limitation on unloading
>   hotpatches:
> 
>   In contrast to normal kernel modules where the module code adheres
>   to specific conventions around resource allocation and locking,
>   hotpatches typically contain code from any context.  That code is
>   usually not aware that it can be unloaded.
> 
>   That code could leave behind in Xen references to itself, e.g., by
>   installing a function pointer in a global data structure, without
>   incrementing something like a usage count.  While most hotpatch code
>   will probably be very simple and small, a similar effect could even
>   be achieved by code called from the hotpatch in Xen, e.g., some code
>   patch could dynamically generate a backtrace and later decide to
>   inspect individual elements from the collected trace, later being a
>   time, where the hotpatch has been unloaded again.
> 
>   One could approach that proplem from multiple angles: code
>   inspection of generated hotpatches, testing, and by making unloading
>   a very special and exceptional operation.

If we follow that argument, we should consider patch unloading to be a
potentially unsafe operation which should not be part of standard
workflows.  Consequently, a similar argument holds for REPLACE, because
removing / unloading older code is part of replacement.

One could now either aim to have special inspection and testing
regarding unloading to lower the risk there, or:

Structure hotpatches as an ever-increasing stack of modules that are
loaded on top of each other.  This approach works around the unloading
problems as well as the temporary-unsafe problem outlined above.  Memory
usage would be similar to an ever-increasing monolithic hotpatch (+ some
fragmentation, - the temporary additional memory requirement for
replacing the monolithic hotpatch).

Martin

Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B