Re: [PATCH 5/7] KVM: x86: Reverse the polarity of efer_reserved_bits

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incorrect bitwise logic in EFER validation rejects valid zero values (breaking guest boot) and allows guests to set reserved bits (triggering VMENTRY failures).
--

commit 4bd740e43933efd5aaca18b2bd83e1696dbbd7dd
Author: Yosry Ahmed <yosry@kernel.org>

KVM: x86: Reverse the polarity of efer_reserved_bits

This commit reverses the polarity of the EFER validation variable, renaming
efer_reserved_bits to efer_supported_bits to align with other fields in
kvm_caps. The stated intent is that no functional change is introduced.

> diff --git a/arch/x86/kvm/msrs.c b/arch/x86/kvm/msrs.c
> index 17d4c813a9e8a..b9a16a27f6c23 100644
> --- a/arch/x86/kvm/msrs.c
> +++ b/arch/x86/kvm/msrs.c

[ ... ]

> @@ -606,7 +606,7 @@ static bool __kvm_valid_efer(struct kvm_vcpu *vcpu, u64 efer)
>  }
>  bool kvm_valid_efer(struct kvm_vcpu *vcpu, u64 efer)
>  {
> -	if (efer & efer_reserved_bits)
> +	if (!(efer & efer_supported_bits))

[Severity: High]
Does this bitwise logic inadvertently reject valid zero values and allow
guests to set reserved bits?

If a guest writes 0 to the EFER MSR, (efer & efer_supported_bits) evaluates
to 0, and the negation makes this condition true, improperly rejecting it.

Additionally, if a guest writes an EFER value containing both a supported
bit and a reserved bit, the condition evaluates to false, allowing the
reserved bit to be set. 

Should this check instead be (efer & ~efer_supported_bits)?

>  		return false;
>  
>  	return __kvm_valid_efer(vcpu, efer);
> @@ -619,7 +619,7 @@ static int set_efer(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  	u64 efer = msr_info->data;
>  	int r;
>  
> -	if (efer & efer_reserved_bits)
> +	if (!(efer & efer_supported_bits))

[Severity: High]
As with kvm_valid_efer() above, does this logic incorrectly reject a valid
clearing of EFER and accept invalid reserved bits if at least one supported
bit is also present?

>  		return 1;
>  
>  	if (!msr_info->host_initiated) {

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