From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: KVM: MMU: bail out pagewalk on kvm_read_guest error Date: Mon, 18 Jan 2010 15:05:20 -0200 Message-ID: <20100118170520.GA3383@amt.cnet> References: <20100114194127.GA3968@amt.cnet> <4B52C909.2010909@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm To: Avi Kivity Return-path: Received: from mx1.redhat.com ([209.132.183.28]:39286 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751824Ab0ARRGB (ORCPT ); Mon, 18 Jan 2010 12:06:01 -0500 Received: from int-mx08.intmail.prod.int.phx2.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o0IH60gP026676 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 18 Jan 2010 12:06:01 -0500 Content-Disposition: inline In-Reply-To: <4B52C909.2010909@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Sun, Jan 17, 2010 at 10:23:37AM +0200, Avi Kivity wrote: > On 01/14/2010 09:41 PM, Marcelo Tosatti wrote: > >Exit the guest pagetable walk loop if reading gpte failed. Otherwise its > >possible to enter an endless loop processing the previous present pte. > > > >Cc: stable@kernel.org > >Signed-off-by: Marcelo Tosatti > > > >diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h > >index 58a0f1e..ede2131 100644 > >--- a/arch/x86/kvm/paging_tmpl.h > >+++ b/arch/x86/kvm/paging_tmpl.h > >@@ -150,7 +150,9 @@ walk: > > walker->table_gfn[walker->level - 1] = table_gfn; > > walker->pte_gpa[walker->level - 1] = pte_gpa; > > > >- kvm_read_guest(vcpu->kvm, pte_gpa,&pte, sizeof(pte)); > >+ if (kvm_read_guest(vcpu->kvm, pte_gpa,&pte, sizeof(pte))) > >+ goto not_present; > >+ > > On real hardware, if you place a pte at non-existing memory, you > aren't guaranteed to get the present bit clear, so why is this > necessary? > > We should be able to survive any garbage the pte previously contained. The problem is the content of the previous pte is processed (which is valid), but the cmpxchg fails (see the loop), without level decreasing.