mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] KVM: nVMX: Rebuild MSR bitmap after eVMCS control changes
@ 2026-09-26  6:37 Weiming Shi
  2026-09-28 16:03 ` Sean Christopherson
  0 siblings, 1 reply; 2+ messages in thread
From: Weiming Shi @ 2026-09-26  6:37 UTC (permalink / raw)
  To: Sean Christopherson, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, x86, H . Peter Anvin
  Cc: Vitaly Kuznetsov, kvm, linux-kernel, Zhong Wang, Weiming Shi, stable

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>
---
 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;
 		return false;
+	}
 
 	msr_bitmap_l1 = (unsigned long *)m.map.hva;
 
-- 
2.55.0


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] KVM: nVMX: Rebuild MSR bitmap after eVMCS control changes
  2026-09-26  6:37 [PATCH] KVM: nVMX: Rebuild MSR bitmap after eVMCS control changes Weiming Shi
@ 2026-09-28 16:03 ` Sean Christopherson
  0 siblings, 0 replies; 2+ messages in thread
From: Sean Christopherson @ 2026-09-28 16:03 UTC (permalink / raw)
  To: Weiming Shi
  Cc: Paolo Bonzini, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H . Peter Anvin, Vitaly Kuznetsov, kvm,
	linux-kernel, Zhong Wang, stable

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
> 
> 

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-09-28 16:03 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-26  6:37 [PATCH] KVM: nVMX: Rebuild MSR bitmap after eVMCS control changes Weiming Shi
2026-09-28 16:03 ` Sean Christopherson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®