Re: [PATCH 11/13] KVM: MMU: fast path of handling guest page fault

[Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>]

On 03/29/2012 05:27 PM, Xiao Guangrong wrote:


> +static bool
> +FNAME(fast_pf_fetch_indirect_spte)(struct kvm_vcpu *vcpu, u64 *sptep,
> +				   u64 *new_spte, gfn_t gfn,
> +				   u32 expect_access, u64 spte)
> +
> +{
> +	struct kvm_mmu_page *sp = page_header(__pa(sptep));
> +	pt_element_t gpte;
> +	gpa_t pte_gpa;
> +	unsigned pte_access;
> +
> +	if (sp->role.direct)
> +		return fast_pf_fetch_direct_spte(vcpu, sptep, new_spte,
> +						 gfn, expect_access, spte);
> +
> +	pte_gpa = FNAME(get_sp_gpa)(sp);
> +	pte_gpa += (sptep - sp->spt) * sizeof(pt_element_t);
> +
> +	if (kvm_read_guest_atomic(vcpu->kvm, pte_gpa, &gpte,
> +				      sizeof(pt_element_t)))
> +		return false;
> +
> +	if (FNAME(invalid_gpte)(vcpu, gpte))
> +		return false;
> +
> +	if (gpte_to_gfn(gpte) != gfn)
> +		return false;
> +


Oh, it can not prevent the gpte has been changed, below case will be triggered:

      VCPU 0                       VCPU 1                          VCPU 2

gpte = gfn1 + RO + S + NX
spte = gfn1's pfn + RO + NX

                                modify gpte: gpte = gfn2 + W + U+ X
                                (due to unsync-sp or wirte emulation
                                 before calling kvm_mmu_pte_write())

                                                         page fault on gpte:
                                                               gfn = gfn2
                                                         fast page fault:
                                                               spte = gfn1's pfn + W + U + X
                                                 (It also can break shadow page table write-protect)
                                                          OOPS!!!

The issue is that gfn does not match with pfn in spte.

Maybe we can properly using sp->gfns[] to avoid it:
- sp->gfns is freed in the RCU context
- sp->gfns[] is initiated to INVALID_GFN
- while spte is dropped, set sp->gfns[] to INVALID_GFN

On fast page fault path, we can check sp->gfns[] with the gfn which is read from
gpte, then do cmpxchg if they are the same.

Then, the thing becomes safe since:
- we have set the identification in the spte before the check, that means we can
  perceive the spte change in the later cmpxchg.
- check sp->gfns[] can ensure spte is pointing to gfn's pfn.