Re: [RFC PATCH] KVM: x86: Fix APIC page invalidation race

[Eiichi Tsukata <eiichi.tsukata@nutanix.com>]

> On Jun 9, 2020, at 18:54, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> 
> No need to resend, the patch is good.  Here is my take on the commit message:

Thank you Paolo! Your commit message is much clearer.
I really appreciate your great job.

Best

Eiichi

> 
>    Commit b1394e745b94 ("KVM: x86: fix APIC page invalidation") tried
>    to fix inappropriate APIC page invalidation by re-introducing arch
>    specific kvm_arch_mmu_notifier_invalidate_range() and calling it from
>    kvm_mmu_notifier_invalidate_range_start. However, the patch left a
>    possible race where the VMCS APIC address cache is updated *before*
>    it is unmapped:
> 
>      (Invalidator) kvm_mmu_notifier_invalidate_range_start()
>      (Invalidator) kvm_make_all_cpus_request(kvm, KVM_REQ_APIC_PAGE_RELOAD)
>      (KVM VCPU) vcpu_enter_guest()
>      (KVM VCPU) kvm_vcpu_reload_apic_access_page()
>      (Invalidator) actually unmap page
> 
>    Because of the above race, there can be a mismatch between the
>    host physical address stored in the APIC_ACCESS_PAGE VMCS field and
>    the host physical address stored in the EPT entry for the APIC GPA
>    (0xfee0000).  When this happens, the processor will not trap APIC
>    accesses, and will instead show the raw contents of the APIC-access page.
>    Because Windows OS periodically checks for unexpected modifications to
>    the LAPIC register, this will show up as a BSOD crash with BugCheck
>    CRITICAL_STRUCTURE_CORRUPTION (109) we are currently seeing in
>    https://urldefense.proofpoint.com/v2/url?u=https-3A__bugzilla.redhat.com_show-5Fbug.cgi-3Fid-3D1751017&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=dy01Dr4Ly8mhvnUdx1pZhhT1bkq4h9z5aVWu3paoZtk&m=yo6pYK4bnrvDLz_RwGyvwtJEY6oWkg-9XGlbPBq1B2g&s=7tKkV92x4mCNLQ7miJIanMVqokYNJdjrcK4Rqqb_h7s&e= .
> 
>    The root cause of the issue is that kvm_arch_mmu_notifier_invalidate_range()
>    cannot guarantee that no additional references are taken to the pages in
>    the range before kvm_mmu_notifier_invalidate_range_end().  Fortunately,
>    this case is supported by the MMU notifier API, as documented in
>    include/linux/mmu_notifier.h:
> 
>             * If the subsystem
>             * can't guarantee that no additional references are taken to
>             * the pages in the range, it has to implement the
>             * invalidate_range() notifier to remove any references taken
>             * after invalidate_range_start().
> 
>    The fix therefore is to reload the APIC-access page field in the VMCS
>    from kvm_mmu_notifier_invalidate_range() instead of ..._range_start().
> 
> Thanks,
> 
> Paolo