linux-kernel - Re: [PATCH v3 1/4] KVM: VMX: Flush CPU buffers as needed if L1D cache flush is skipped

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <DDO1FFOJKSTK.3LSOUFU5RM6PD@google.com>
Date: Tue, 21 Oct 2025 13:34:39 +0000
From: Brendan Jackman <jackmanb@...gle.com>
To: Sean Christopherson <seanjc@...gle.com>, Paolo Bonzini <pbonzini@...hat.com>
Cc: <kvm@...r.kernel.org>, <linux-kernel@...r.kernel.org>, 
	Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>, Brendan Jackman <jackmanb@...gle.com>
Subject: Re: [PATCH v3 1/4] KVM: VMX: Flush CPU buffers as needed if L1D cache
 flush is skipped

On Thu Oct 16, 2025 at 8:04 PM UTC, Sean Christopherson wrote:
> If the L1D flush for L1TF is conditionally enabled, flush CPU buffers to
> mitigate MMIO Stale Data as needed if KVM skips the L1D flush, e.g.
> because none of the "heavy" paths that trigger an L1D flush were tripped
> since the last VM-Enter.

Presumably the assumption here was that the L1TF conditionality is good
enough for the MMIO stale data vuln too? I'm not qualified to assess if
that assumption is true, but also even if it's a good one it's
definitely not obvious to users that the mitigation you pick for L1TF
has this side-effect. So I think I'm on board with calling this a bug.
If anyone turns out to be depending on the current behaviour for
performance I think they should probably add it back as a separate flag.

> MDS mitigation was inadvertently fixed by commit 43fb862de8f6 ("KVM/VMX:
> Move VERW closer to VMentry for MDS mitigation"), but previous kernels
> that flush CPU buffers in vmx_vcpu_enter_exit() are affected.
>
> Fixes: 650b68a0622f ("x86/kvm/vmx: Add MDS protection when L1D Flush is not active")
> Cc: Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>
> Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> ---
>  arch/x86/kvm/vmx/vmx.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index f87c216d976d..ce556d5dc39b 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -6663,7 +6663,7 @@ int vmx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath)
>   * information but as all relevant affected CPUs have 32KiB L1D cache size
>   * there is no point in doing so.
>   */
> -static noinstr void vmx_l1d_flush(struct kvm_vcpu *vcpu)
> +static noinstr bool vmx_l1d_flush(struct kvm_vcpu *vcpu)
>  {
>  	int size = PAGE_SIZE << L1D_CACHE_ORDER;
>  
> @@ -6691,14 +6691,14 @@ static noinstr void vmx_l1d_flush(struct kvm_vcpu *vcpu)
>  		kvm_clear_cpu_l1tf_flush_l1d();
>  
>  		if (!flush_l1d)
> -			return;
> +			return false;
>  	}
>  
>  	vcpu->stat.l1d_flush++;
>  
>  	if (static_cpu_has(X86_FEATURE_FLUSH_L1D)) {
>  		native_wrmsrq(MSR_IA32_FLUSH_CMD, L1D_FLUSH);
> -		return;
> +		return true;
>  	}
>  
>  	asm volatile(
> @@ -6722,6 +6722,7 @@ static noinstr void vmx_l1d_flush(struct kvm_vcpu *vcpu)
>  		:: [flush_pages] "r" (vmx_l1d_flush_pages),
>  		    [size] "r" (size)
>  		: "eax", "ebx", "ecx", "edx");
> +	return true;

The comment in the caller says the L1D flush "includes CPU buffer clear
to mitigate MDS" - do we actually know that this software sequence
mitigates the MMIO stale data vuln like the verw does? (Do we even know if
it mitigates MDS?)

Anyway, if this is an issue, it's orthogonal to this patch.

Reviewed-by: Brendan Jackman <jackmanb@...gle.com>

>  }
>  
>  void vmx_update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr)
> @@ -7330,8 +7331,9 @@ static noinstr void vmx_vcpu_enter_exit(struct kvm_vcpu *vcpu,
>  	 * and is affected by MMIO Stale Data. In such cases mitigation in only
>  	 * needed against an MMIO capable guest.
>  	 */
> -	if (static_branch_unlikely(&vmx_l1d_should_flush))
> -		vmx_l1d_flush(vcpu);
> +	if (static_branch_unlikely(&vmx_l1d_should_flush) &&
> +	    vmx_l1d_flush(vcpu))
> +		;
>  	else if (static_branch_unlikely(&cpu_buf_vm_clear) &&
>  		 (flags & VMX_RUN_CLEAR_CPU_BUFFERS_FOR_MMIO))
>  		x86_clear_cpu_buffers();