Re: [PATCH v3 1/2] KVM: x86: Check EFER validity on KVM_SET_SREGS*

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the return value of `kvm_x86_call(set_efer)` is ignored in `__set_sregs_common()`, leading to a silent failure when nested state allocation fails.
--

--- Patch [1]: [PATCH v3 1/2] KVM: x86: Check EFER validity on KVM_SET_SREGS* ---
commit 6edd9e84461af3c58ac965b84f89b7c4985ddeb3
Author: Yosry Ahmed <yosry@kernel.org>

KVM: x86: Check EFER validity on KVM_SET_SREGS*

This commit adds checks to validate EFER against both KVM and guest caps
during KVM_SET_SREGS* writes, preventing userspace from writing bogus values
through the per-vendor set_efer callbacks.

> diff --git a/arch/x86/kvm/regs.c b/arch/x86/kvm/regs.c
> index bd8147798cc3e..8f66438989e47 100644
> --- a/arch/x86/kvm/regs.c
> +++ b/arch/x86/kvm/regs.c

[ ... ]

> @@ -564,7 +564,8 @@ static bool kvm_is_valid_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
>  	}
>  
>  	return kvm_is_valid_cr4(vcpu, sregs->cr4) &&
> -	       kvm_is_valid_cr0(vcpu, sregs->cr0);
> +	       kvm_is_valid_cr0(vcpu, sregs->cr0) &&
> +	       kvm_valid_efer(vcpu, sregs->efer);
>  }
>  
>  static int __set_sregs_common(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs,
>  			      int *mmu_reset_needed, bool update_pdptrs)
>  {

[ ... ]

>  	vcpu->arch.cr3 = sregs->cr3;
>  	kvm_register_mark_dirty(vcpu, VCPU_REG_CR3);
>  	kvm_x86_call(post_set_cr3)(vcpu, sregs->cr3);
>  
>  	*mmu_reset_needed |= vcpu->arch.efer != sregs->efer;
>  	kvm_x86_call(set_efer)(vcpu, sregs->efer);

[Severity: High]
This isn't a bug introduced by this patch, but should the return value
of kvm_x86_call() be checked here?

If a userspace process calls the KVM_SET_SREGS ioctl with EFER.SVME set,
kvm_x86_call(set_efer) could call into svm_set_efer(), which attempts to
allocate nested state via svm_allocate_nested().

If the system is under memory pressure, this allocation can fail and return
-ENOMEM. Because the return value is ignored in __set_sregs_common(),
doesn't this lead to a silent failure where KVM fails to set the guest's
EFER register but still returns success to userspace?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713180153.2728382-1-yosry@kernel.org?part=1