linux-kernel - Re: [PATCH] KVM: VMX: Flush shadow VMCS on emergency reboot

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z_g-UQoZ8fQhVD_2@google.com>
Date: Thu, 10 Apr 2025 14:55:29 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Chao Gao <chao.gao@...el.com>
Cc: kvm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Paolo Bonzini <pbonzini@...hat.com>, Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, 
	Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org, 
	"H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH] KVM: VMX: Flush shadow VMCS on emergency reboot

On Mon, Mar 24, 2025, Chao Gao wrote:
> Ensure the shadow VMCS cache is evicted during an emergency reboot to
> prevent potential memory corruption if the cache is evicted after reboot.

I don't suppose Intel would want to go on record and state what CPUs would actually
be affected by this bug.  My understanding is that Intel has never shipped a CPU
that caches shadow VMCS state.

On a very related topic, doesn't SPR+ now flush the VMCS caches on VMXOFF?  If
that's going to be the architectural behavior going forward, will that behavior
be enumerated to software?  Regardless of whether there's software enumeration,
I would like to have the emergency disable path depend on that behavior.  In part
to gain confidence that SEAM VMCSes won't screw over kdump, but also in light of
this bug.

If all past CPUs never cache shadow VMCS state, and all future CPUs flush the
caches on VMXOFF, then this is a glorified NOP, and thus probably shouldn't be
tagged for stable.

> This issue was identified through code inspection, as __loaded_vmcs_clear()
> flushes both the normal VMCS and the shadow VMCS.
> 
> Avoid checking the "launched" state during an emergency reboot, unlike the
> behavior in __loaded_vmcs_clear(). This is important because reboot NMIs
> can interfere with operations like copy_shadow_to_vmcs12(), where shadow
> VMCSes are loaded directly using VMPTRLD. In such cases, if NMIs occur
> right after the VMCS load, the shadow VMCSes will be active but the
> "launched" state may not be set.
> 
> Signed-off-by: Chao Gao <chao.gao@...el.com>
> ---
>  arch/x86/kvm/vmx/vmx.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index b70ed72c1783..dccd1c9939b8 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -769,8 +769,11 @@ void vmx_emergency_disable_virtualization_cpu(void)
>  		return;
>  
>  	list_for_each_entry(v, &per_cpu(loaded_vmcss_on_cpu, cpu),
> -			    loaded_vmcss_on_cpu_link)
> +			    loaded_vmcss_on_cpu_link) {
>  		vmcs_clear(v->vmcs);
> +		if (v->shadow_vmcs)
> +			vmcs_clear(v->shadow_vmcs);
> +	}
>  
>  	kvm_cpu_vmxoff();
>  }
> -- 
> 2.46.1
>