Re: [PATCH 6/7] KVM: x86: Move supported EFER bits to kvm_caps

[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 EFER=0 and allows setting reserved EFER bits.
--

commit 2ebe510bc1d7884420361ec9d7613318508f1d94
Author: Yosry Ahmed <yosry@kernel.org>

KVM: x86: Move supported EFER bits to kvm_caps

This commit moves efer_supported_bits into kvm_caps as supported_efer_bits
for consistency, allowing it to be recomputed during vendor initialization.
It removes helper functions for enabling or disabling specific bits, choosing
to open-code updates directly to the new bitmask.

> diff --git a/arch/x86/kvm/msrs.c b/arch/x86/kvm/msrs.c
> --- a/arch/x86/kvm/msrs.c
> +++ b/arch/x86/kvm/msrs.c
> @@ -606,7 +603,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_supported_bits))
> +	if (!(efer & kvm_caps.supported_efer_bits))

[Severity: High]
Does this bitwise logic incorrectly reject an efer value of 0?

If userspace or a guest completely clears efer (e.g., setting it to 0 when
returning to real mode), efer & kvm_caps.supported_efer_bits evaluates to 0,
making the ! condition true. This would reject the valid state.

Additionally, does this allow writing unsupported reserved bits? If a guest
sets a supported bit alongside arbitrary reserved bits (e.g.,
EFER_SCE | BIT_ULL(63)), the bitwise AND yields a non-zero value, meaning
this check passes.

Since __kvm_valid_efer() only validates specific feature bits and ignores
unknown bits, could these reserved bits be written directly to vCPU hardware
state?

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

[Severity: High]
Does this identically flawed check also block the guest from clearing its efer
via wrmsr, and allow writing reserved bits directly to hardware?

Should this check be if (efer & ~kvm_caps.supported_efer_bits) instead to
correctly verify that no unsupported bits are set?

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

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