From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:33836) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TEbN7-0002Pd-63 for qemu-devel@nongnu.org; Thu, 20 Sep 2012 03:40:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TEbN2-0006vU-VW for qemu-devel@nongnu.org; Thu, 20 Sep 2012 03:40:29 -0400 Message-ID: <505AC84C.9060500@siemens.com> Date: Thu, 20 Sep 2012 09:39:56 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <1348124922-24263-1-git-send-email-david@gibson.dropbear.id.au> <1348124922-24263-2-git-send-email-david@gibson.dropbear.id.au> In-Reply-To: <1348124922-24263-2-git-send-email-david@gibson.dropbear.id.au> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 1/2] pseries: Synchronize qemu and KVM state on hypercalls List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: qemu-stable@nongnu.org, qemu-ppc@nongnu.org, agraf@suse.de, qemu-devel@nongnu.org On 2012-09-20 09:08, David Gibson wrote: > Currently the KVM exit path for PAPR hypercalls does not synchronize the > qemu cpu state with the KVM state. Mostly this works, because the actual > hypercall arguments and return values are explicitly passed through the > kvm_run structure. However, the hypercall path includes a privilege check, > to ensure that only the guest kernel can invoke hypercalls, not the guest > userspace. Because of the lack of sync, this privilege check will use an > out of date copy of the MSR, which could lead either to guest userspace > being able to invoke hypercalls (a security hole for the guest) or to the > guest kernel being incorrectly refused privilege leading to various other > failures. > > This patch fixes the bug by forcing a synchronization on the hypercall exit > path. This does mean we have a potentially quite expensive get and set of > the state, however performance critical hypercalls are generally already > implemented inside KVM so this probably won't matter. If it is a > performance problem we can optimize it later by having the kernel perform > the privilege check. That will need a new capability, however, since qemu > will still need the privilege check for older kernels. If it's just about reading a small subset of the state (a single MSR?) to allow the privilege check, you can also open-code only that in HCALL exit. If it really matters. Jan > > Signed-off-by: David Gibson > --- > target-ppc/kvm.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c > index 546c116..78a47fb 100644 > --- a/target-ppc/kvm.c > +++ b/target-ppc/kvm.c > @@ -813,6 +813,7 @@ int kvm_arch_handle_exit(CPUPPCState *env, struct kvm_run *run) > #ifdef CONFIG_PSERIES > case KVM_EXIT_PAPR_HCALL: > dprintf("handle PAPR hypercall\n"); > + cpu_synchronize_state(env); > run->papr_hcall.ret = spapr_hypercall(env, run->papr_hcall.nr, > run->papr_hcall.args); > ret = 0; > -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux