linux-kernel - Re: [PATCH 07/54] KVM: x86: Alert userspace that KVM_SET_CPUID{,2} after KVM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 23 Jun 2021 19:11:40 +0200
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Jim Mattson <jmattson@...gle.com>
Cc:     Sean Christopherson <seanjc@...gle.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Joerg Roedel <joro@...tes.org>, kvm list <kvm@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Yu Zhang <yu.c.zhang@...ux.intel.com>,
        Maxim Levitsky <mlevitsk@...hat.com>
Subject: Re: [PATCH 07/54] KVM: x86: Alert userspace that KVM_SET_CPUID{,2}
 after KVM_RUN is broken

On 23/06/21 19:00, Jim Mattson wrote:
> On Wed, Jun 23, 2021 at 7:16 AM Paolo Bonzini <pbonzini@...hat.com> wrote:
>>
>> On 22/06/21 19:56, Sean Christopherson wrote:
>>> +     /*
>>> +      * KVM does not correctly handle changing guest CPUID after KVM_RUN, as
>>> +      * MAXPHYADDR, GBPAGES support, AMD reserved bit behavior, etc.. aren't
>>> +      * tracked in kvm_mmu_page_role.  As a result, KVM may miss guest page
>>> +      * faults due to reusing SPs/SPTEs.  Alert userspace, but otherwise
>>> +      * sweep the problem under the rug.
>>> +      *
>>> +      * KVM's horrific CPUID ABI makes the problem all but impossible to
>>> +      * solve, as correctly handling multiple vCPU models (with respect to
>>> +      * paging and physical address properties) in a single VM would require
>>> +      * tracking all relevant CPUID information in kvm_mmu_page_role.  That
>>> +      * is very undesirable as it would double the memory requirements for
>>> +      * gfn_track (see struct kvm_mmu_page_role comments), and in practice
>>> +      * no sane VMM mucks with the core vCPU model on the fly.
>>> +      */
>>> +     if (vcpu->arch.last_vmentry_cpu != -1)
>>> +             pr_warn_ratelimited("KVM: KVM_SET_CPUID{,2} after KVM_RUN may cause guest instability\n");
>>
>> Let's make this even stronger and promise to break it in 5.16.
>>
>> Paolo
> 
> Doesn't this fall squarely into kvm's philosophy of "we should let
> userspace shoot itself in the foot wherever possible"? I thought we
> only stepped in when host stability was an issue.
> 
> I'm actually delighted if this is a sign that we're rethinking that
> philosophy. I'd just like to hear someone say it.

Nah, that's not the philosophy.  The philosophy is that covering all 
possible ways for userspace to shoot itself in the foot is impossible.

However, here we're talking about 2 lines of code (thanks also to your 
patches that add last_vmentry_cpu for completely unrelated reasons) to 
remove a whole set of bullet/foot encounters.

Paolo