* KVM: x86: verify MTRR/PAT validity
@ 2009-06-16 12:05 Marcelo Tosatti
2009-06-18 2:39 ` Yang, Sheng
2009-06-18 9:00 ` Avi Kivity
0 siblings, 2 replies; 5+ messages in thread
From: Marcelo Tosatti @ 2009-06-16 12:05 UTC (permalink / raw)
To: Avi Kivity, Yang, Sheng; +Cc: kvm
Do not allow invalid MTRR/PAT values in set_msr_mtrr.
Please review carefully.
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
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) {
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: KVM: x86: verify MTRR/PAT validity
2009-06-16 12:05 KVM: x86: verify MTRR/PAT validity Marcelo Tosatti
@ 2009-06-18 2:39 ` Yang, Sheng
2009-06-22 18:27 ` Marcelo Tosatti
2009-06-18 9:00 ` Avi Kivity
1 sibling, 1 reply; 5+ messages in thread
From: Yang, Sheng @ 2009-06-18 2:39 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: Avi Kivity, kvm
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?
--
regards
Yang, Sheng
>
> 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) {
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: KVM: x86: verify MTRR/PAT validity
2009-06-18 2:39 ` Yang, Sheng
@ 2009-06-22 18:27 ` Marcelo Tosatti
2009-06-24 10:00 ` Avi Kivity
0 siblings, 1 reply; 5+ messages in thread
From: Marcelo Tosatti @ 2009-06-22 18:27 UTC (permalink / raw)
To: Yang, Sheng; +Cc: Avi Kivity, kvm
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 <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?
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 <mtosatti@redhat.com>
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) {
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: KVM: x86: verify MTRR/PAT validity
2009-06-22 18:27 ` Marcelo Tosatti
@ 2009-06-24 10:00 ` Avi Kivity
0 siblings, 0 replies; 5+ messages in thread
From: Avi Kivity @ 2009-06-24 10:00 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: Yang, Sheng, kvm
On 06/22/2009 09:27 PM, Marcelo Tosatti wrote:
>
> 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:
>
Applied, thanks.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: KVM: x86: verify MTRR/PAT validity
2009-06-16 12:05 KVM: x86: verify MTRR/PAT validity Marcelo Tosatti
2009-06-18 2:39 ` Yang, Sheng
@ 2009-06-18 9:00 ` Avi Kivity
1 sibling, 0 replies; 5+ messages in thread
From: Avi Kivity @ 2009-06-18 9:00 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: Yang, Sheng, kvm
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 */
}
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-06-24 9:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-06-16 12:05 KVM: x86: verify MTRR/PAT validity Marcelo Tosatti
2009-06-18 2:39 ` Yang, Sheng
2009-06-22 18:27 ` Marcelo Tosatti
2009-06-24 10:00 ` Avi Kivity
2009-06-18 9:00 ` Avi Kivity
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox