Re: [PATCH] Fix SMP shadow instantiation race

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

Marcelo Tosatti wrote:
> There is a race where VCPU0 is shadowing a pagetable entry while VCPU1
> is updating it, which results in a stale shadow copy.
>
> Fix that by comparing the contents of the cached guest pte with the
> current guest pte after write-protecting the guest pagetable.
>
> Attached program kvm_shadow_race.c demonstrates the problem.
>
>   

Where is it?

> Signed-off-by: Marcelo Tosatti <mtosatti-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>
>
> diff --git a/drivers/kvm/paging_tmpl.h b/drivers/kvm/paging_tmpl.h
> index 72d4816..4fece01 100644
> --- a/drivers/kvm/paging_tmpl.h
> +++ b/drivers/kvm/paging_tmpl.h
> @@ -66,6 +66,7 @@ struct guest_walker {
>  	int level;
>  	gfn_t table_gfn[PT_MAX_FULL_LEVELS];
>  	pt_element_t pte;
> +	gpa_t pte_gpa;
>   

I think this needs to be an array like table_gfn[].  The guest may play 
with the pde (and upper entries) as well as the pte.

>  
> +	kvm_read_guest(vcpu->kvm, walker->pte_gpa, &curr_pte, sizeof(curr_pte));
> +
> +	if (curr_pte != walker->pte)
> +		return 0;
> +
>   

'return NULL'

It would also be preferable to read the pte only if we shadowed the page 
just now.  Perhaps pass the pte and the index to kvm_mmu_get_page() 
which would use them as a guard when the page is being shadowed:

    if (lookup page succeeds)
         return it
    shadow page
    write protect it
    if (guard check succeeds)
        return it
    else
        return NULL

or perhaps have kvm_mmu_get_page() return an additional bool signifying 
it is a new page.  but this is ugly.

>  
> -	++vcpu->stat.pf_fixed;
> +	if (shadow_pte)
> +		++vcpu->stat.pf_fixed;
>   

This is a very rare case; it isn't worth being so accurate maintaining 
the statistics.

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


-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php