lkml.org 
[lkml]   [2014]   [Jul]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] KVM: x86: Assertions to check no overrun in MSR lists
Il 24/07/2014 14:06, Nadav Amit ha scritto:
> Currently there is no check whether shared MSRs list overrun the allocated size
> which can results in bugs. In addition there is no check that vmx->guest_msrs
> has sufficient space to accommodate all the VMX msrs. This patch adds the
> assertions.
>
> Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
> ---
> arch/x86/kvm/vmx.c | 2 ++
> arch/x86/kvm/x86.c | 1 +
> 2 files changed, 3 insertions(+)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 7534a9f..286a931 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -7585,6 +7585,8 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id)
> goto free_vcpu;
>
> vmx->guest_msrs = kmalloc(PAGE_SIZE, GFP_KERNEL);
> + BUILD_BUG_ON(PAGE_SIZE / sizeof(struct shared_msr_entry) < NR_VMX_MSR);
> +
> err = -ENOMEM;
> if (!vmx->guest_msrs) {
> goto uninit_vcpu;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index f750b69..f5cd7876 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -212,6 +212,7 @@ static void shared_msr_update(unsigned slot, u32 msr)
>
> void kvm_define_shared_msr(unsigned slot, u32 msr)
> {
> + BUG_ON(slot >= KVM_NR_SHARED_MSRS);
> if (slot >= shared_msrs_global.nr)
> shared_msrs_global.nr = slot + 1;
> shared_msrs_global.msrs[slot] = msr;
>

Thanks, both are good improvements. I'm adding this patch on top.

-------------------- 8< ---------------------
From: Paolo Bonzini <pbonzini@redhat.com>
Subject: [PATCH] Replace NR_VMX_MSR with its definition

Using ARRAY_SIZE directly makes it easier to read the code. While touching
the code, replace the division by a multiplication in the recently added
BUILD_BUG_ON.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 3397a88b7463..906f9e49d0e7 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -823,7 +823,6 @@ static const u32 vmx_msr_index[] = {
#endif
MSR_EFER, MSR_TSC_AUX, MSR_STAR,
};
-#define NR_VMX_MSR ARRAY_SIZE(vmx_msr_index)

static inline bool is_page_fault(u32 intr_info)
{
@@ -4441,7 +4440,7 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx)
vmx->vcpu.arch.pat = host_pat;
}

- for (i = 0; i < NR_VMX_MSR; ++i) {
+ for (i = 0; i < ARRAY_SIZE(vmx_msr_index); ++i) {
u32 index = vmx_msr_index[i];
u32 data_low, data_high;
int j = vmx->nmsrs;
@@ -7608,7 +7607,8 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id)
goto free_vcpu;

vmx->guest_msrs = kmalloc(PAGE_SIZE, GFP_KERNEL);
- BUILD_BUG_ON(PAGE_SIZE / sizeof(struct shared_msr_entry) < NR_VMX_MSR);
+ BUILD_BUG_ON(ARRAY_SIZE(vmx_msr_index) * sizeof(vmx->guest_msrs[0])
+ > PAGE_SIZE);

err = -ENOMEM;
if (!vmx->guest_msrs) {
@@ -8960,7 +8960,7 @@ static int __init vmx_init(void)

rdmsrl_safe(MSR_EFER, &host_efer);

- for (i = 0; i < NR_VMX_MSR; ++i)
+ for (i = 0; i < ARRAY_SIZE(vmx_msr_index); ++i)
kvm_define_shared_msr(i, vmx_msr_index[i]);

vmx_io_bitmap_a = (unsigned long *)__get_free_page(GFP_KERNEL);


\
 
 \ /
  Last update: 2014-07-24 15:01    [W:0.035 / U:12.140 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site