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