Re: [PATCH] KVM: nVMX: Rebuild MSR bitmap after eVMCS control changes

[Sean Christopherson <seanjc@google.com>]

On Sat, Sep 26, 2026, Weiming Shi wrote:
> The Enlightened MSR Bitmap shortcut reuses vmcs02's bitmap when L1 marks
> MSR_BITMAP clean, but the bitmap also depends on execution controls in
> CONTROL_GRP1 and CONTROL_PROC.  L1 can change either group while leaving
> MSR_BITMAP clean, preserving stale APIC_TASKPRI passthrough for L2.
> 
> Reusing the bitmap after a failed rebuild is unsafe too.  The failed entry
> disables hardware MSR bitmaps, but KVM subsequently marks the eVMCS clean,
> allowing the next entry to reactivate the old vmcs02 bitmap.
> 
> Require both control groups to be clean before reusing the bitmap, and keep
> force_msr_bitmap_recalc set until a rebuild succeeds.
> 
> Fixes: 502d2bf5f2fd ("KVM: nVMX: Implement Enlightened MSR Bitmap feature")
> Cc: stable@vger.kernel.org
> Reported-by: Zhong Wang <wangzhong.c0ss4ck@bytedance.com>
> Assisted-by: LLM
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>

Fix already posted[*] and applied (I cheated a bit and sent it to Paolo off-list
a while back).

[*] https://lore.kernel.org/all/20260926053253.195597-7-pbonzini@redhat.com

> ---
>  arch/x86/kvm/vmx/nested.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index 151873407abd3..0f0e677d99482 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -755,13 +755,17 @@ static inline bool nested_vmx_prepare_msr_bitmap(struct kvm_vcpu *vcpu,
>  		struct hv_enlightened_vmcs *evmcs = nested_vmx_evmcs(vmx);
>  
>  		if (evmcs && evmcs->hv_enlightenments_control.msr_bitmap &&
> -		    evmcs->hv_clean_fields & HV_VMX_ENLIGHTENED_CLEAN_FIELD_MSR_BITMAP)
> +		    evmcs->hv_clean_fields & HV_VMX_ENLIGHTENED_CLEAN_FIELD_MSR_BITMAP &&
> +		    evmcs->hv_clean_fields & HV_VMX_ENLIGHTENED_CLEAN_FIELD_CONTROL_GRP1 &&
> +		    evmcs->hv_clean_fields & HV_VMX_ENLIGHTENED_CLEAN_FIELD_CONTROL_PROC)
>  			return true;
>  	}
>  
>  	CLASS(kvm_vcpu_map_local_readonly, m)(vcpu, gpa_to_gfn(vmcs12->msr_bitmap));
> -	if (m.ret)
> +	if (m.ret) {
> +		vmx->nested.force_msr_bitmap_recalc = true;

Copy+pasting a comment I made off-list in reponse to a suggestion to fix this as
you propose here (though you obviously caught the early return issue I pointed out):

 : Hmm, I like the idea from a "what's logical", but it's wildly unsafe.  Even if we
 : fixed all of the paths that could cause problems (and there are a lot), the code
 : would be extremely brittle, i.e. we'd always be at a higher risk of reintroducing
 : the bug.
 : 
 : Specifically, if nested_vmx_prepare_msr_bitmap() doesn't run to completion and
 : actually update the bitmaps, then L1 can re-run L2 in the future *without*
 : changing the control fields, and once again consume the stale bitmap.  E.g. if
 : CPU_BASED_USE_MSR_BITMAPS is clear, if hv_enlightenments_control.msr_bitmap is
 : disabled, if mapping vmcs12->msr_bitmap fails, or most problematically, if
 : anything between updating the control fields in copy_enlightened_to_vmcs12()
 : causes KVM to bail from nested VM-Enter.  The window for that last one could be
 : shrinked by detecting control changes in prepare_vmcs02_early(), but there would
 : still be a bug.
 : 
 : In other words, the only input from L1 that can be safely consumed on-demand
 : HV_VMX_ENLIGHTENED_CLEAN_FIELD_MSR_BITMAP, because that's L1's responsibility to
 : get right.  For everything else, KVM needs to well and truly force an update
 : when *KVM* detects a need to update the bitmaps.

Thanks for the fix though!

>  		return false;
> +	}
>  
>  	msr_bitmap_l1 = (unsigned long *)m.map.hva;
>  
> -- 
> 2.55.0
> 
>