linux-kernel - Re: [PATCH v4 4/8] KVM: VMX: Handle MMIO Stale Data in VM-Enter assembly via ALTERNATIVES

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: <20251112183836.GBaRTULLaMWA5hkfT9@fat_crate.local>
Date: Wed, 12 Nov 2025 19:38:36 +0100
From: Borislav Petkov <bp@...en8.de>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Peter Zijlstra <peterz@...radead.org>,
	Josh Poimboeuf <jpoimboe@...nel.org>, 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 v4 4/8] KVM: VMX: Handle MMIO Stale Data in VM-Enter
 assembly via ALTERNATIVES_2

On Wed, Nov 12, 2025 at 09:15:00AM -0800, Sean Christopherson wrote:
> On Wed, Nov 12, 2025, Borislav Petkov wrote:
> > On Thu, Oct 30, 2025 at 05:30:36PM -0700, Sean Christopherson wrote:
> > > @@ -137,6 +138,12 @@ SYM_FUNC_START(__vmx_vcpu_run)
> > >  	/* Load @regs to RAX. */
> > >  	mov (%_ASM_SP), %_ASM_AX
> > >  
> > > +	/* Stash "clear for MMIO" in EFLAGS.ZF (used below). */
> > 
> > Oh wow. Alternatives interdependence. What can go wrong. :)
> 
> Nothing, it's perfect. :-D

Yeah. :-P

> 
> > > +	ALTERNATIVE_2 "",								\
> > > +		      __stringify(test $VMX_RUN_CLEAR_CPU_BUFFERS_FOR_MMIO, %ebx), 	\
> > 
> > So this VMX_RUN_CLEAR_CPU_BUFFERS_FOR_MMIO bit gets set here:
> > 
> >         if (cpu_feature_enabled(X86_FEATURE_CLEAR_CPU_BUF_MMIO) &&
> >             kvm_vcpu_can_access_host_mmio(&vmx->vcpu))
> >                 flags |= VMX_RUN_CLEAR_CPU_BUFFERS_FOR_MMIO;
> > 
> > So how static and/or dynamic is this?
> 
> kvm_vcpu_can_access_host_mmio() is very dynamic.  It can be different between
> vCPUs in a VM, and can even change on back-to-back runs of the same vCPU.

Hmm, strange. Because looking at those things there:

root->has_mapped_host_mmio and vcpu->kvm->arch.has_mapped_host_mmio

they both read like something that a guest would set up once and that's it.
But what do I know...

> > IOW, can you stick this into a simple variable which is unconditionally
> > updated and you can use it in X86_FEATURE_CLEAR_CPU_BUF_MMIO case and
> > otherwise it simply remains unused?
> 
> Can you elaborate?  I don't think I follow what you're suggesting.

So I was thinking if you could set a per-guest variable in
C - vmx_per_guest_clear_per_mmio or so and then test it in asm:

		testb $1,vmx_per_guest_clear_per_mmio(%rip)
		jz .Lskip_clear_cpu_buffers;
		CLEAR_CPU_BUFFERS_SEQ;

.Lskip_clear_cpu_buffers:

gcc -O3 suggests also

		cmpb   $0x0,vmx_per_guest_clear_per_mmio(%rip)

which is the same insn size...

The idea is to get rid of this first asm stashing things and it'll be a bit
more robust, I'd say.

And you don't rely on registers...

and when I say that, I now realize this is 32-bit too and you don't want to
touch regs - that's why you're stashing it - and there's no rip-relative on
32-bit...

I dunno - it might get hairy but I would still opt for a different solution
instead of this fragile stashing in ZF. You could do a function which pushes
and pops a scratch register where you put the value, i.e., you could do

	push %reg
	mov var, %reg
	test or cmp ...
	...
	jz skip...
skip:
	pop %reg

It is still all together in one place instead of spreading it around like
that.

Oh well.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette