Re: Bug with paravirt ops and livepatches

From: Miroslav Benes
Date: Tue Mar 29 2016 - 09:01:30 EST

[ adding CCs ]

On Tue, 29 Mar 2016, Chris J Arges wrote:

> Paravirtualized ops and livepatching currently don't mix very well and can
> cause undefined behavor such as oops, invalid opcodes or corrupted stacks.
> The original discussion of this issue can be found here [1].
> I've written an example livepatch module that reproduces the issue [2].
> In order to trigger the issue you must first insert the module then trigger
> the paravirt ops by starting a VM.
> In the thread here [1] a couple of solutions have been proposed:


oh no... so this is not only about paravirt ops but also about
alternatives, jump labels and so on, isn't it?

> 1) Jessica proposed using the Arch-independent patchset ensure that livepatch
> finishes writing its relas before apply_paravirt() is called. However, this
> introduces a bit more arch-dependent code. It would be useful to see if other
> arches are affected by this as well.

I think this is the way to go. Provided we have Jessica's two patch sets
applied (arch-independent and notifiers removal) there are two options. We
either move a call to klp_coming_module() somewhere before
module_finalize(), or we move the problematic parts of module_finalize()
to the end of load_module() (on x86 it is probably module_finalize() as a
whole). The former is almost impossible because of the dependencies
(ftrace and such), the latter should be doable (with very careful check we
won't break anything).

> 2) Eugene proposed skipping application of the rela if the instruction to be
> relocated has already been changed. This passes the initial example [2];
> however its unclear if/how this will break things.

Hm, I don't like this one. It really depends on that the paravirt
instructions which are supposed to be patched do not contain the
code which needs to be relocated. This can be true for now, but we have to
think long-term... which leads me to... If the new instructions need to be
relocated... this is indeed a problem, right? You'd need to fix
kpatch-build somehow to generate appropriate dynrelas for the paravirt
patched code. But, during the livepatch module generation one does not
know if the code would be patched by alternatives. Crap :/


> It may be good to weigh in here and get more eyes on this.
> Thanks,
> --chris
> [1]:
> [2]:
> --
> To unsubscribe from this list: send the line "unsubscribe live-patching" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at