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 for Android: free password hash cracker in your pocket

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

Message-ID: <858cdb9c2c1cc1deda179fc534ad42de1275920f.camel@redhat.com>
Date:   Wed, 11 May 2022 19:26:05 +0300
From:   Maxim Levitsky <mlevitsk@...hat.com>
To:     Suravee Suthikulpanit <suravee.suthikulpanit@....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

On Wed, 2022-05-11 at 22:37 +0700, Suravee Suthikulpanit wrote:
> 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)

Nope because the above only updates L1 msr intercept bitmap, while 'merged'
msr bitmap that L2 uses still has those msrs open.

Other and better way to fix it would be to fix set_msr_interception
to update the merged bitmap as well.

I think I will post a patch series to clean up this mess soon.

Best regards,
	Maxim Levitsky

> 
> > 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
>