linux-kernel - Re: [PATCH v4 10/15] KVM: SVM: Introduce helper functions to (de)activate AVIC and x2AVIC

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]

Message-ID: <3fa12834-d144-54d8-0bf8-8a72e726db99@amd.com>
Date:   Wed, 11 May 2022 22:37:31 +0700
From:   Suravee Suthikulpanit <suravee.suthikulpanit@....com>
To:     Maxim Levitsky <mlevitsk@...hat.com>, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org
Cc:     pbonzini@...hat.com, seanjc@...gle.com, joro@...tes.org,
        jon.grimm@....com, wei.huang2@....com, terry.bowman@....com,
        kernel test robot <lkp@...el.com>
Subject: Re: [PATCH v4 10/15] KVM: SVM: Introduce helper functions to
 (de)activate AVIC and x2AVIC

Maxim,

On 5/9/22 8:42 PM, Maxim Levitsky wrote:
>...
> 
> So I did some testing, and reviewed this code again with regard to nesting,
> and now I see that it has CVE worthy bug, so have to revoke my Reviewed-By.
> 
> This is what happens:
> 
> On nested VM entry, *request to inhibit AVIC is done*, and then nested msr bitmap
> is calculated, still with all X2AVIC msrs open,
> 
> 1. nested_svm_vmrun -> enter_svm_guest_mode -> kvm_make_request(KVM_REQ_APICV_UPDATE, vcpu);
> 2. nested_svm_vmrun -> nested_svm_vmrun_msrpm
> 
> But the nested guest will be entered without AVIC active
> (since we don't yet support nested avic and it is optional anyway), thus if the nested guest
> also doesn't intercept those msrs, it will gain access to the *host* x2apic msrs. Ooops.

Shouldn't this be changed to intercept the x2APIC msrs because of the following logic?

kvm_make_request(KVM_REQ_APICV_UPDATE, vcpu)
     kvm_vcpu_update_apicv(vcpu)
         static_call(kvm_x86_refresh_apicv_exec_ctrl)(vcpu)
             avic_deactivate_vmcb()
                 svm_set_x2apic_msr_interception(true)

> I think the easist way to fix this for now, is to make nested_svm_vmrun_msrpm
> never open access to x2apic msrs regardless of the host bitmap value, but in the long
> term the whole thing needs to be refactored.

Agree.

> Another thing I noted is that avic_deactivate_vmcb should not touch avic msrs
> when avic_mode == AVIC_MODE_X1, it is just a waste of time.

We can add the check.

> Also updating these msr intercepts is pointless if the guest doesn't use x2apic.

We can also add the check.

Best Regards,
Suravee