From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755858AbcC2NB1 (ORCPT ); Tue, 29 Mar 2016 09:01:27 -0400 Received: from mx2.suse.de ([195.135.220.15]:37812 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751826AbcC2NB0 (ORCPT ); Tue, 29 Mar 2016 09:01:26 -0400 Date: Tue, 29 Mar 2016 15:01:22 +0200 (CEST) From: Miroslav Benes To: Chris J Arges cc: jeyu@redhat.com, jpoimboe@redhat.com, eugene.shatokhin@rosalab.ru, live-patching@vger.kernel.org, Linux Kernel Mailing List , jikos@kernel.org, pmladek@suse.cz Subject: Re: Bug with paravirt ops and livepatches In-Reply-To: <20160329120518.GA21252@canonical.com> Message-ID: References: <20160329120518.GA21252@canonical.com> User-Agent: Alpine 2.00 (LNX 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org [ 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 >