From patchwork Mon Jun 22 18:27:56 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marcelo Tosatti X-Patchwork-Id: 31833 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n5MIShPT019345 for ; Mon, 22 Jun 2009 18:28:43 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754387AbZFVS2i (ORCPT ); Mon, 22 Jun 2009 14:28:38 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754446AbZFVS2i (ORCPT ); Mon, 22 Jun 2009 14:28:38 -0400 Received: from mx2.redhat.com ([66.187.237.31]:34805 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754289AbZFVS2h (ORCPT ); Mon, 22 Jun 2009 14:28:37 -0400 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n5MISeO9013552; Mon, 22 Jun 2009 14:28:40 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id n5MISdOg006444; Mon, 22 Jun 2009 14:28:39 -0400 Received: from amt.cnet (vpn-51-15.sfbay.redhat.com [10.14.51.15]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id n5MISag9031055; Mon, 22 Jun 2009 14:28:38 -0400 Received: from amt.cnet (amt.cnet [127.0.0.1]) by amt.cnet (Postfix) with ESMTP id 7EF3768012F; Mon, 22 Jun 2009 15:28:05 -0300 (BRT) Received: (from marcelo@localhost) by amt.cnet (8.14.3/8.14.3/Submit) id n5MIRuA7008615; Mon, 22 Jun 2009 15:27:56 -0300 Date: Mon, 22 Jun 2009 15:27:56 -0300 From: Marcelo Tosatti To: "Yang, Sheng" Cc: Avi Kivity , kvm Subject: Re: KVM: x86: verify MTRR/PAT validity Message-ID: <20090622182756.GA8582@amt.cnet> References: <20090616120529.GA529@amt.cnet> <200906181039.33205.sheng.yang@intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <200906181039.33205.sheng.yang@intel.com> User-Agent: Mutt/1.5.19 (2009-01-05) X-Scanned-By: MIMEDefang 2.58 on 172.16.27.26 Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Thu, Jun 18, 2009 at 10:39:32AM +0800, Yang, Sheng wrote: > On Tuesday 16 June 2009 20:05:29 Marcelo Tosatti wrote: > > Do not allow invalid MTRR/PAT values in set_msr_mtrr. > > > > Please review carefully. > > > > Signed-off-by: Marcelo Tosatti > > > Looks fine to me. > > Is it necessary to check reserved bit of MSR_MTRRdefType and variable MTRRs as > well? Maybe like this: > > if (msr == MSR_MTRRdefType) { > return valid_mtrr_type(data & ~0xc00ull); > } > > And variable ones can be: > > #define MTRR_VALID_MASK(v, msr) (~(rsvd_bits(cpuid_max_physaddr(v)) | ((msr % > 2) << 11))) > > return valid_mtrr_type(data & MTRR_VALID_MASK(vcpu, msr))) > > > (rsvd_bits() is in mmu.c, both untested) > > Maybe we can put cpuid_max_physaddr as a field in vcpu struct? Sheng, This in the BIOS is writing 1's into the reserved address bits into variable MTRR: wrmsr_smp(MTRRphysMask_MSR(0), ~(0x20000000ull - 1) | 0x800); So i'll leave just memory type validity checking and MSR_MTRRdefType valid bit checks in for now: KVM: x86: verify MTRR/PAT validity Do not allow invalid memory types in MTRR/PAT (generating a #GP otherwise). Signed-off-by: Marcelo Tosatti --- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Index: kvm/arch/x86/kvm/x86.c =================================================================== --- kvm.orig/arch/x86/kvm/x86.c +++ kvm/arch/x86/kvm/x86.c @@ -721,11 +721,48 @@ static bool msr_mtrr_valid(unsigned msr) return false; } +static bool valid_pat_type(unsigned t) +{ + return t < 8 && (1 << t) & 0xf3; /* 0, 1, 4, 5, 6, 7 */ +} + +static bool valid_mtrr_type(unsigned t) +{ + return t < 8 && (1 << t) & 0x73; /* 0, 1, 4, 5, 6 */ +} + +static bool mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data) +{ + int i; + + if (!msr_mtrr_valid(msr)) + return false; + + if (msr == MSR_IA32_CR_PAT) { + for (i = 0; i < 8; i++) + if (!valid_pat_type((data >> (i * 8)) & 0xff)) + return false; + return true; + } else if (msr == MSR_MTRRdefType) { + if (data & ~0xcff) + return false; + return valid_mtrr_type(data & 0xff); + } else if (msr >= MSR_MTRRfix64K_00000 && msr <= MSR_MTRRfix4K_F8000) { + for (i = 0; i < 8 ; i++) + if (!valid_mtrr_type((data >> (i * 8)) & 0xff)) + return false; + return true; + } + + /* variable MTRRs */ + return valid_mtrr_type(data & 0xff); +} + static int set_msr_mtrr(struct kvm_vcpu *vcpu, u32 msr, u64 data) { u64 *p = (u64 *)&vcpu->arch.mtrr_state.fixed_ranges; - if (!msr_mtrr_valid(msr)) + if (!mtrr_valid(vcpu, msr, data)) return 1; if (msr == MSR_MTRRdefType) {