| Message ID | 20090616120529.GA529@amt.cnet (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <mtosatti@redhat.com> > 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?
On 06/16/2009 03:05 PM, Marcelo Tosatti wrote: > Do not allow invalid MTRR/PAT values in set_msr_mtrr. > > Please review carefully. > > Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com> > > > +static unsigned mtrr_types[] = {0, 1, 4, 5, 6}; > +static unsigned pat_types[] = {0, 1, 4, 5, 6, 7}; > + > +static bool valid_mt(unsigned type, int len, unsigned array[len]) > +{ > + int i; > + > + for (i = 0; i< len; i++) > + if (type == array[i]) > + return true; > + > + return false; > +} > + > +#define valid_pat_type(a) valid_mt(a, ARRAY_SIZE(pat_types), pat_types) > +#define valid_mtrr_type(a) valid_mt(a, ARRAY_SIZE(mtrr_types), mtrr_types) > + > A little pointless since this is so performance-insensitive, but still: valid_pat_type(t) { return t < 8 && (1 << t) & 0xf3; /* 0, 1, 4, 5, 6, 7 */ } valid_mtrr_type(t) { return t < 8 && (1 << t) & 0x73; /* 0, 1, 4, 5, 6 */ }
Index: kvm/arch/x86/kvm/x86.c =================================================================== --- kvm.orig/arch/x86/kvm/x86.c +++ kvm/arch/x86/kvm/x86.c @@ -722,11 +722,53 @@ static bool msr_mtrr_valid(unsigned msr) return false; } +static unsigned mtrr_types[] = {0, 1, 4, 5, 6}; +static unsigned pat_types[] = {0, 1, 4, 5, 6, 7}; + +static bool valid_mt(unsigned type, int len, unsigned array[len]) +{ + int i; + + for (i = 0; i < len; i++) + if (type == array[i]) + return true; + + return false; +} + +#define valid_pat_type(a) valid_mt(a, ARRAY_SIZE(pat_types), pat_types) +#define valid_mtrr_type(a) valid_mt(a, ARRAY_SIZE(mtrr_types), mtrr_types) + +static bool mtrr_valid(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) { + 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, physmaskn have bits 0-10 reserved */ + 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(msr, data)) return 1; if (msr == MSR_MTRRdefType) {
Do not allow invalid MTRR/PAT values in set_msr_mtrr. Please review carefully. Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> -- 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