Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1372663

Re: Bug with paravirt ops and livepatches

Show all headers | View raw

+++ Jessica Yu [05/04/16 15:19 -0400]:
>+++ Miroslav Benes [05/04/16 15:07 +0200]:
>>On Mon, 4 Apr 2016, Josh Poimboeuf wrote:
>>
>>>So I think this doesn't fix the problem.  Dynamic relocations are
>>>applied to the "patch module", whereas the above code deals with the
>>>initialization order of the "patched module".  This distinction
>>>originally confused me as well, until Jessica set me straight.
>>>
>>>Let me try to illustrate the problem with an example.  Imagine you have
>>>a patch module P which applies a patch to module M.  P replaces M's
>>>function F with a new function F', which uses paravirt ops.
>>>
>>>1) Patch P is loaded before module M.  P's new function F' has an
>>>   instruction which is patched by apply_paravirt(), even though the
>>>   patch hasn't been applied yet.
>>>
>>>2) Module M is loaded.  Before applying the patch, livepatch tries to
>>>   apply a klp_reloc to the instruction in F' which was already patched
>>>   by apply_paravirt() in step 1.  This results in undefined behavior
>>>   because it tries to patch the original instruction but instead
>>>   patches the new paravirt instruction.
>>>
>>>So the above patch makes no difference because the paravirt module
>>>loading order doesn't really matter.
>>
>>Hi,
>>
>>we are trying really hard to understand the actual culprit here and as it
>>is quite confusing I have several questions/comments...
>
>I don't have a 100% clear understanding of the whole picture either,
>but I'll try to help clarify up some things..
>
>>1. can you provide dynrela sections of the patch module from
>>https://github.com/dynup/kpatch/issues/580? What is interesting is that
>>kvm_arch_vm_ioctl() function contains rela records only for trivial (==
>>exported) symbols from the first look. The problem should be there only if
>>you want to patch a function which reference some paravirt_ops unexported
>>symbol. For that symbol dynrela should be created.
>
>Just to dispel some confusion over this, kpatch isn't "smart" enough
>yet to differentiate between exported and non-exported symbols, as
>Evgenii already mentioned. Just global and local, and whether the
>symbol belongs to a module or vmlinux. So that means dynrelas are
>indeed being created for the pv_*_ops symbols, despite the fact they
>are exported.

Gah, slight mistake, I should have mentioned that the above case only
applies to patches to modules (i.e. there is no mechanism to detect
exported symbols for patches to modules), but for vmlinux patches
there is (and those stay normal relas). I'm sorry if I confused
anybody. The relevant dynrela generation code is here if anyone's
interested:
https://github.com/dynup/kpatch/blob/master/kpatch-build/create-diff-object.c#L2661

Jessica

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Re: Bug with paravirt ops and livepatches Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-04 18:20 +0200
  Re: Bug with paravirt ops and livepatches Jessica Yu <jeyu@redhat.com> - 2016-04-04 20:00 +0200
  Re: Bug with paravirt ops and livepatches Miroslav Benes <mbenes@suse.cz> - 2016-04-05 15:10 +0200
    Re: Bug with paravirt ops and livepatches Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-05 16:30 +0200
    Re: Bug with paravirt ops and livepatches Jessica Yu <jeyu@redhat.com> - 2016-04-05 21:20 +0200
      Re: Bug with paravirt ops and livepatches Miroslav Benes <mbenes@suse.cz> - 2016-04-06 10:40 +0200
        Re: Bug with paravirt ops and livepatches Miroslav Benes <mbenes@suse.cz> - 2016-04-06 10:50 +0200
          Re: Bug with paravirt ops and livepatches Miroslav Benes <mbenes@suse.cz> - 2016-04-06 11:10 +0200
          Re: Bug with paravirt ops and livepatches Jessica Yu <jeyu@redhat.com> - 2016-04-06 19:30 +0200
      Re: Bug with paravirt ops and livepatches Jessica Yu <jeyu@redhat.com> - 2016-04-06 19:00 +0200
    Re: Bug with paravirt ops and livepatches Chris J Arges <chris.j.arges@canonical.com> - 2016-04-06 01:30 +0200
      Re: Bug with paravirt ops and livepatches Miroslav Benes <mbenes@suse.cz> - 2016-04-06 11:20 +0200
        Re: Bug with paravirt ops and livepatches Chris J Arges <chris.j.arges@canonical.com> - 2016-04-06 12:40 +0200
          Re: Bug with paravirt ops and livepatches Miroslav Benes <mbenes@suse.cz> - 2016-04-06 14:10 +0200
            Re: Bug with paravirt ops and livepatches Chris J Arges <chris.j.arges@canonical.com> - 2016-04-06 15:50 +0200
              Re: Bug with paravirt ops and livepatches Miroslav Benes <mbenes@suse.cz> - 2016-04-06 16:20 +0200

csiph-web