Re: [patch] kvm: make cr3 loading more robust

[Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]

Ingo Molnar wrote:
> From: Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>
> Subject: [patch] kvm: make cr3 loading more robust
>
> rmap_write_protect() has a BUG_ON() if a physical address is not found 
> the the memslot. But this is a possible scenario if a buggy guest OS 
> loads an invalid or corrupted cr3 value. So exit more gracefully.
>
>   

I think a better solution is to detect the invalid cr3 in new_cr3(). 
That way the entire chain of error returns is avoided.

> [ Avi: this patch needs kvm_mmu_get_page() NULL return to not be ignored 
>   by mmu_alloc_roots()/nonpaging_init_context()/paging_new_cr3()/etc. 
>   and passed further down - i assume you are working on those
>   issues already, or should i fix those too? ]
>   

kvm_mmu_get_page() cannot return NULL (before this patch), as we ensure 
there are free pages before entry to page_fault.

> Signed-off-by: Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>
>
> Index: linux/drivers/kvm/mmu.c
> ===================================================================
> --- linux.orig/drivers/kvm/mmu.c
> +++ linux/drivers/kvm/mmu.c
> @@ -378,7 +378,7 @@ static void rmap_remove(struct kvm_vcpu 
>  	}
>  }
>  
> -static void rmap_write_protect(struct kvm_vcpu *vcpu, u64 gfn)
> +static int rmap_write_protect(struct kvm_vcpu *vcpu, u64 gfn)
>  {
>  	struct kvm *kvm = vcpu->kvm;
>  	struct page *page;
> @@ -387,7 +387,8 @@ static void rmap_write_protect(struct kv
>  	u64 *spte;
>  
>  	slot = gfn_to_memslot(kvm, gfn);
> -	BUG_ON(!slot);
> +	if (!slot)
> +		return -1;
>  	page = gfn_to_page(slot, gfn);
>  
>  	while (page->private) {
> @@ -407,6 +408,7 @@ static void rmap_write_protect(struct kv
>  		kvm_arch_ops->tlb_flush(vcpu);
>  		*spte &= ~(u64)PT_WRITABLE_MASK;
>  	}
> +	return 0;
>  }
>  
>  static int is_empty_shadow_page(hpa_t page_hpa)
> @@ -592,12 +594,17 @@ static struct kvm_mmu_page *kvm_mmu_get_
>  	page = kvm_mmu_alloc_page(vcpu, parent_pte);
>  	if (!page)
>  		return page;
> +	if (!metaphysical) {
> +		if (rmap_write_protect(vcpu, gfn) < 0) {
> +			kvm_mmu_free_page(vcpu, page->page_hpa);
> +			return NULL;
> +		}
> +	}
>  	pgprintk("%s: adding gfn %lx role %x\n", __FUNCTION__, gfn, role.word);
>  	page->gfn = gfn;
>  	page->role = role;
>  	hlist_add_head(&page->hash_link, bucket);
> -	if (!metaphysical)
> -		rmap_write_protect(vcpu, gfn);
> +
>  	return page;
>  }
>  
>   


-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV