From: Miroslav Benes <mbenes@suse.cz>
To: Chris J Arges <chris.j.arges@canonical.com>
Cc: jeyu@redhat.com, jpoimboe@redhat.com,
eugene.shatokhin@rosalab.ru, live-patching@vger.kernel.org,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
jikos@kernel.org, pmladek@suse.cz
Subject: Re: Bug with paravirt ops and livepatches
Date: Tue, 29 Mar 2016 15:01:22 +0200 (CEST) [thread overview]
Message-ID: <alpine.LNX.2.00.1603291450090.26309@pobox.suse.cz> (raw)
In-Reply-To: <20160329120518.GA21252@canonical.com>
[ 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:
Hi,
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 :/
Miroslav
> It may be good to weigh in here and get more eyes on this.
> Thanks,
> --chris
>
> [1]: https://github.com/dynup/kpatch/issues/580
> [2]: http://people.canonical.com/~arges/livepatch_issue/livepatch_kvm_arch_vm_ioctl/livepatch.c
> --
> To unsubscribe from this list: send the line "unsubscribe live-patching" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
next prev parent reply other threads:[~2016-03-29 13:01 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-29 12:05 Bug with paravirt ops and livepatches Chris J Arges
2016-03-29 13:01 ` Miroslav Benes [this message]
2016-03-29 13:05 ` Jiri Kosina
2016-04-01 15:01 ` Jiri Kosina
2016-04-01 15:46 ` Miroslav Benes
2016-04-01 16:01 ` Chris J Arges
2016-04-01 19:07 ` Chris J Arges
2016-04-01 19:35 ` Jiri Kosina
2016-04-04 16:14 ` Josh Poimboeuf
2016-04-04 17:58 ` Jessica Yu
2016-04-05 13:07 ` Miroslav Benes
2016-04-05 13:53 ` Evgenii Shatokhin
2016-04-05 14:24 ` Josh Poimboeuf
2016-04-05 19:19 ` Jessica Yu
2016-04-06 8:30 ` Miroslav Benes
2016-04-06 8:43 ` Miroslav Benes
2016-04-06 9:09 ` Miroslav Benes
2016-04-06 17:23 ` Jessica Yu
2016-04-06 16:55 ` Jessica Yu
2016-04-05 23:27 ` Chris J Arges
2016-04-06 9:09 ` Miroslav Benes
2016-04-06 10:38 ` Chris J Arges
2016-04-06 12:09 ` Miroslav Benes
2016-04-06 13:48 ` Chris J Arges
2016-04-06 14:17 ` Miroslav Benes
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.LNX.2.00.1603291450090.26309@pobox.suse.cz \
--to=mbenes@suse.cz \
--cc=chris.j.arges@canonical.com \
--cc=eugene.shatokhin@rosalab.ru \
--cc=jeyu@redhat.com \
--cc=jikos@kernel.org \
--cc=jpoimboe@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=live-patching@vger.kernel.org \
--cc=pmladek@suse.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.