Re: [PATCH] livepatch: Add some basic LivePatch documentation

[Miroslav Benes]

On Thu, 10 Mar 2016, Jiri Kosina wrote:

> On Wed, 9 Mar 2016, Petr Mladek wrote:
> 
> > LivePatch framework deserves some documentation, definitely.
> > This is an attempt to provide some basic info. I hope that
> > it will be useful for both LivePatch producers and also
> > potential developers of the framework itself.
> 
> Thanks for starting the efforts; this is really needed if we want the 
> infrastructure to be used also by someone else than its developers :)

Indeed. Great job, Petr.

> [ ... snip ... ]
> > +7. Limitations
> > +==============
> 
> Miroslav Benes, who is working on creating the actual patches we are 
> shipping for our kernels, should already have a decent cheat-sheet 
> regarding things that the patch author should be extra careful when 
> creating a patch (inlining and other gcc optimizations such as isra, 
> functions that use switch_to(), percpu, ...).

Yes, my cheat-sheet is pretty long as of now, but the things there 
are mainly connected with the "userspace" part and they are not really 
limitations of current kernel implementation. Some of those are solvable 
with relocations and some depend on the patch development process 
(inlining, gcc optimizations and so on). I think it would be great to 
discuss it at Plumbers miniconference. It really depends on a future 
userspace tool which is on the list as well

> I am not really sure whether these should go to limitations, or maybe 
> they'd better be placed into a separate chapter, but I'd really like to 
> see it included.

Apart the limitations Petr has already described there are two more. And 
both quite serious to mention. First, we cannot patch and handle data 
structures. Sometimes it is possible by workaround (for example there 
could be a hole for a new member, because a data structure is aligned; or 
sometimes we can use an existing member for something else and so on), but 
generally we can't.

Second thing, which Jiri mentioned above, is switch_to macro and friends. 
switch_to is inlined to __schedule() (via context_switch()) with many 
other functions. The problem is that switch_to macro does not save RIP in 
x86_64 version (contrary to 32-bit version). So when one patches a 
function inlined to __schedule() (and thus they need to patch the very 
__schedule() because of inlining) it is perfectly possible to crash the 
kernel. New __schedule() function can easily have a different prologue and 
then the interesting thing happens. Let's have two different tasks. One 
calls __schedule(), its registers are stored in a defined order and it 
goes to sleep in switch_to macro. Then we have the second task which calls 
patched__schedule(), it goes to sleep there and as a next task the first 
one is picked. Its RSP is restored and now the registers should be 
restored as well. But the order is different in the new 
patched__schedule(), so... well you get the idea. As a result it is 
currently not possible to patch anything inlined to __schedule(). And 
there are some CVEs out there. For example LDT problem Andy discovered 
last year.

I leave aside that since 499d79559ffe ("sched/core: More notrace 
annotations") it is not even possible to patch __schedule() directly.

This is for sure something we should discuss as well. I have a more or 
less functional solution, but it modifies switch_to macro in a way which 
would easily be nacked by maintainers (I have to perform some measurements 
yet, but it is really ugly. Trust me.).

Miroslav