ACK: [PATCH] KVM: SVM: load control fields from VMCB12 before checking them

[Stefan Bader]

On 08.04.21 21:36, Tim Gardner wrote:
> From: Paolo Bonzini <pbonzini at redhat.com>
> 
> CVE-2021-29657
> 
> Avoid races between check and use of the nested VMCB controls.  This
> for example ensures that the VMRUN intercept is always reflected to the
> nested hypervisor, instead of being processed by the host.  Without this
> patch, it is possible to end up with svm->nested.hsave pointing to
> the MSR permission bitmap for nested guests.
> 
> This bug is CVE-2021-29657.
> 
> Reported-by: Felix Wilhelm <fwilhelm at google.com>
> Cc: stable at vger.kernel.org
> Fixes: 2fcf4876ada ("KVM: nSVM: implement on demand allocation of the nested state")
> Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
> (cherry picked from commit a58d9166a756a0f4a6618e4f593232593d6df134)
> Signed-off-by: Tim Gardner <tim.gardner at canonical.com>
Acked-by: Stefan Bader <stefan.bader at canonical.com>
> ---
>   arch/x86/kvm/svm/nested.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 1008cc6cb66c..dd318ca6c722 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -246,7 +246,7 @@ static bool nested_vmcb_check_controls(struct vmcb_control_area *control)
>   	return true;
>   }
>   
> -static bool nested_vmcb_checks(struct vcpu_svm *svm, struct vmcb *vmcb12)
> +static bool nested_vmcb_check_save(struct vcpu_svm *svm, struct vmcb *vmcb12)
>   {
>   	struct kvm_vcpu *vcpu = &svm->vcpu;
>   	bool vmcb12_lma;
> @@ -271,7 +271,7 @@ static bool nested_vmcb_checks(struct vcpu_svm *svm, struct vmcb *vmcb12)
>   	if (kvm_valid_cr4(&svm->vcpu, vmcb12->save.cr4))
>   		return false;
>   
> -	return nested_vmcb_check_controls(&vmcb12->control);
> +	return true;
>   }
>   
>   static void load_nested_vmcb_control(struct vcpu_svm *svm,
> @@ -454,7 +454,6 @@ int enter_svm_guest_mode(struct vcpu_svm *svm, u64 vmcb12_gpa,
>   	int ret;
>   
>   	svm->nested.vmcb12_gpa = vmcb12_gpa;
> -	load_nested_vmcb_control(svm, &vmcb12->control);
>   	nested_prepare_vmcb_save(svm, vmcb12);
>   	nested_prepare_vmcb_control(svm);
>   
> @@ -501,7 +500,10 @@ int nested_svm_vmrun(struct vcpu_svm *svm)
>   	if (WARN_ON_ONCE(!svm->nested.initialized))
>   		return -EINVAL;
>   
> -	if (!nested_vmcb_checks(svm, vmcb12)) {
> +	load_nested_vmcb_control(svm, &vmcb12->control);
> +
> +	if (!nested_vmcb_check_save(svm, vmcb12) ||
> +	    !nested_vmcb_check_controls(&svm->nested.ctl)) {
>   		vmcb12->control.exit_code    = SVM_EXIT_ERR;
>   		vmcb12->control.exit_code_hi = 0;
>   		vmcb12->control.exit_info_1  = 0;
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20210409/86a2e619/attachment.sig>